Skip to content

Commit

Permalink
chore: [26-x-y] cherry-pick 6 changes from Release-3-M119
Browse files Browse the repository at this point in the history
* 971d6055e7b7 from openscreen
* 3f45b1af5e41 from chromium
* e13061c50998 from chromium
* 6169a1fabae1 from skia
* 6cc0d9aa5b3fb from libavif
* 922fca786b61a from libavif
  • Loading branch information
VerteDinde committed Nov 30, 2023
1 parent a0af9e0 commit 9be7bc5
Show file tree
Hide file tree
Showing 11 changed files with 493 additions and 0 deletions.
2 changes: 2 additions & 0 deletions patches/chromium/.patches
Original file line number Diff line number Diff line change
Expand Up @@ -144,3 +144,5 @@ cherry-pick-3df423a5b8de.patch
scale_rects_properly_in_syncgetfirstrectforrange.patch
cherry-pick-9384cddc7705.patch
fix_restore_original_resize_performance_on_macos.patch
cherry-pick-3f45b1af5e41.patch
cherry-pick-e13061c50998.patch
48 changes: 48 additions & 0 deletions patches/chromium/cherry-pick-3f45b1af5e41.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Alvin Ji <alvinji@chromium.org>
Date: Mon, 13 Nov 2023 20:24:24 +0000
Subject: Check context status before creating new platform destination

RealtimeAudioDestinationHandler::SetSinkDescriptor creates new
destination platofrm without validating context status. This can
reactivate the audio rendering thread when AudioContext is already in
closed state.

(cherry picked from commit 0f9bb9a1083865d4e51059e588f27f729ab32753)

Bug: 1500856
Change-Id: If1fd531324b56fcdc38d315fd84d4cec577a14bc
Test: Locally confirmed with ASAN
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5021160
Reviewed-by: Alvin Ji <alvinji@chromium.org>
Commit-Queue: Alvin Ji <alvinji@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1223168}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5026373
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Hongchan Choi <hongchan@chromium.org>
Cr-Commit-Position: refs/branch-heads/6099@{#607}
Cr-Branched-From: e6ee4500f7d6549a9ac1354f8d056da49ef406be-refs/heads/main@{#1217362}

diff --git a/third_party/blink/renderer/modules/webaudio/realtime_audio_destination_handler.cc b/third_party/blink/renderer/modules/webaudio/realtime_audio_destination_handler.cc
index 2e4757d155800700b7c6a8b7cbf2e02250cfce65..c27eb3ac07f22a4cd1ae2f86da896d255a761292 100644
--- a/third_party/blink/renderer/modules/webaudio/realtime_audio_destination_handler.cc
+++ b/third_party/blink/renderer/modules/webaudio/realtime_audio_destination_handler.cc
@@ -405,6 +405,17 @@ void RealtimeAudioDestinationHandler::SetSinkDescriptor(
GetCallbackBufferSize()));
DCHECK(IsMainThread());

+ // After the context is closed, `SetSinkDescriptor` request will be ignored
+ // because it will trigger the recreation of the platform destination. This in
+ // turn can activate the audio rendering thread.
+ AudioContext* context = static_cast<AudioContext*>(Context());
+ CHECK(context);
+ if (context->ContextState() == AudioContext::kClosed) {
+ std::move(callback).Run(
+ media::OutputDeviceStatus::OUTPUT_DEVICE_STATUS_ERROR_INTERNAL);
+ return;
+ }
+
// Create a pending AudioDestination to replace the current one.
scoped_refptr<AudioDestination> pending_platform_destination =
AudioDestination::Create(
116 changes: 116 additions & 0 deletions patches/chromium/cherry-pick-e13061c50998.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Ken Rockot <rockot@google.com>
Date: Thu, 16 Nov 2023 23:44:43 +0000
Subject: Reland: Fix IPC Channel pipe teardown

This is a reland with the new test temporarily disabled on Android
until it can run without disrupting other tests.

(cherry picked from commit cd4c1f165c16c6d8161b5372ef7f61c715e01a42)

Fixed: 1494461
Change-Id: If1d83c2dce62020f78dd50abc460973759002a1a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5015115
Commit-Queue: Ken Rockot <rockot@google.com>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1221953}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5037764
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Auto-Submit: Ken Rockot <rockot@google.com>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/branch-heads/5993@{#1618}
Cr-Branched-From: 511350718e646be62331ae9d7213d10ec320d514-refs/heads/main@{#1192594}

diff --git a/ipc/ipc_mojo_bootstrap.cc b/ipc/ipc_mojo_bootstrap.cc
index 2ab03807d102d8f4e2a22119210d5cb669338c3b..5fa17e2ff108909a8987665dd21bc490118f1147 100644
--- a/ipc/ipc_mojo_bootstrap.cc
+++ b/ipc/ipc_mojo_bootstrap.cc
@@ -787,13 +787,12 @@ class ChannelAssociatedGroupController
// handle.
DCHECK(!endpoint->client());
DCHECK(endpoint->peer_closed());
- MarkClosedAndMaybeRemove(endpoint);
+ MarkClosed(endpoint);
} else {
- MarkPeerClosedAndMaybeRemove(endpoint);
+ MarkPeerClosed(endpoint);
}
}
-
- DCHECK(endpoints_.empty());
+ endpoints_.clear();

GetMemoryDumpProvider().RemoveController(this);
}
@@ -838,15 +837,19 @@ class ChannelAssociatedGroupController
base::AutoLock locker(lock_);
encountered_error_ = true;

+ std::vector<uint32_t> endpoints_to_remove;
std::vector<scoped_refptr<Endpoint>> endpoints_to_notify;
for (auto iter = endpoints_.begin(); iter != endpoints_.end();) {
Endpoint* endpoint = iter->second.get();
++iter;

- if (endpoint->client())
+ if (endpoint->client()) {
endpoints_to_notify.push_back(endpoint);
+ }

- MarkPeerClosedAndMaybeRemove(endpoint);
+ if (MarkPeerClosed(endpoint)) {
+ endpoints_to_remove.push_back(endpoint->id());
+ }
}

for (auto& endpoint : endpoints_to_notify) {
@@ -855,6 +858,10 @@ class ChannelAssociatedGroupController
if (endpoint->client())
NotifyEndpointOfError(endpoint.get(), false /* force_async */);
}
+
+ for (uint32_t id : endpoints_to_remove) {
+ endpoints_.erase(id);
+ }
}

void NotifyEndpointOfError(Endpoint* endpoint, bool force_async) {
@@ -893,19 +900,33 @@ class ChannelAssociatedGroupController
NotifyEndpointOfError(endpoint, false /* force_async */);
}

- void MarkClosedAndMaybeRemove(Endpoint* endpoint) {
+ // Marks `endpoint` as closed and returns true if and only if its peer was
+ // also already closed.
+ bool MarkClosed(Endpoint* endpoint) {
lock_.AssertAcquired();
endpoint->set_closed();
- if (endpoint->closed() && endpoint->peer_closed())
- endpoints_.erase(endpoint->id());
+ return endpoint->peer_closed();
}

- void MarkPeerClosedAndMaybeRemove(Endpoint* endpoint) {
+ // Marks `endpoint` as having a closed peer and returns true if and only if
+ // `endpoint` itself was also already closed.
+ bool MarkPeerClosed(Endpoint* endpoint) {
lock_.AssertAcquired();
endpoint->set_peer_closed();
endpoint->SignalSyncMessageEvent();
- if (endpoint->closed() && endpoint->peer_closed())
+ return endpoint->closed();
+ }
+
+ void MarkClosedAndMaybeRemove(Endpoint* endpoint) {
+ if (MarkClosed(endpoint)) {
endpoints_.erase(endpoint->id());
+ }
+ }
+
+ void MarkPeerClosedAndMaybeRemove(Endpoint* endpoint) {
+ if (MarkPeerClosed(endpoint)) {
+ endpoints_.erase(endpoint->id());
+ }
}

Endpoint* FindOrInsertEndpoint(mojo::InterfaceId id, bool* inserted) {
4 changes: 4 additions & 0 deletions patches/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

"src/electron/patches/node": "src/third_party/electron_node",

"src/electron/patches/libavif": "src/third_party/libavif/src",

"src/electron/patches/nan": "src/third_party/nan",

"src/electron/patches/perfetto": "src/third_party/perfetto",
Expand All @@ -21,6 +23,8 @@

"src/electron/patches/ReactiveObjC": "src/third_party/squirrel.mac/vendor/ReactiveObjC",

"src/electron/patches/skia": "src/third_party/skia",

"src/electron/patches/webrtc": "src/third_party/webrtc",

"src/electron/patches/dawn": "src/third_party/dawn"
Expand Down
2 changes: 2 additions & 0 deletions patches/libavif/.patches
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
remove_potential_out_of_bound_access_to_alphaitemindices.patch
do_not_store_potentially_invalid_pointers.patch
68 changes: 68 additions & 0 deletions patches/libavif/do_not_store_potentially_invalid_pointers.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Vignesh Venkatasubramanian <vigneshv@google.com>
Date: Wed, 15 Nov 2023 15:22:49 -0800
Subject: Do not store potentially invalid pointers

Manual cherry-pick of PR #1757 into the chromium-m118 branch.

diff --git a/src/read.c b/src/read.c
index d8699bb1442645d358f13f1904d7fbb9237bb999..73aa68eb0ad377e95038280fea1523dd909b6e87 100644
--- a/src/read.c
+++ b/src/read.c
@@ -769,6 +769,8 @@ static void avifMetaDestroy(avifMeta * meta)
avifFree(meta);
}

+// CAUTION: This function could potentially resize the meta->items array thereby invalidating all existing pointers that are being
+// stored locally. So if this function is being called, exercise caution in the caller to not use invalid pointers.
static avifDecoderItem * avifMetaFindItem(avifMeta * meta, uint32_t itemID)
{
if (itemID == 0) {
@@ -3596,17 +3598,20 @@ static avifBool avifDecoderItemIsAlphaAux(avifDecoderItem * item, uint32_t color
return auxCProp && isAlphaURN(auxCProp->u.auxC.auxType);
}

-// Finds the alpha item whose parent item is colorItem and sets it in the alphaItem output parameter. Returns AVIF_RESULT_OK on
-// success. Note that *alphaItem can be NULL even if the return value is AVIF_RESULT_OK. If the colorItem is a grid and the alpha
-// item is represented as a set of auxl items to each color tile, then a fake item will be created and *isAlphaItemInInput will be
-// set to AVIF_FALSE. In this case, the alpha item merely exists to hold the locations of the alpha tile items. The data of this
-// item need not be read and the pixi property cannot be validated. Otherwise, *isAlphaItemInInput will be set to AVIF_TRUE when
-// *alphaItem is not NULL.
+// Finds the alpha item whose parent item is *colorItemPtr and sets it in the alphaItem output parameter. Returns AVIF_RESULT_OK
+// on success. Note that *alphaItem can be NULL even if the return value is AVIF_RESULT_OK. If the *colorItemPtr is a grid and the
+// alpha item is represented as a set of auxl items to each color tile, then a fake item will be created and *isAlphaItemInInput
+// will be set to AVIF_FALSE. In this case, the alpha item merely exists to hold the locations of the alpha tile items. The data
+// of this item need not be read and the pixi property cannot be validated. Otherwise, *isAlphaItemInInput will be set to
+// AVIF_TRUE when *alphaItem is not NULL. If the data->meta->items array is resized, then the value in *colorItemPtr could become
+// invalid. This function also resets *colorItemPtr to the right value if an alpha item was found and added to the data->meta->items
+// array.
static avifResult avifDecoderDataFindAlphaItem(avifDecoderData * data,
- avifDecoderItem * colorItem,
+ avifDecoderItem ** colorItemPtr,
avifDecoderItem ** alphaItem,
avifBool * isAlphaItemInInput)
{
+ const avifDecoderItem * colorItem = *colorItemPtr;
for (uint32_t itemIndex = 0; itemIndex < data->meta->items.count; ++itemIndex) {
avifDecoderItem * item = &data->meta->items.item[itemIndex];
if (avifDecoderItemShouldBeSkipped(item)) {
@@ -3682,6 +3687,10 @@ static avifResult avifDecoderDataFindAlphaItem(avifDecoderData * data,
*isAlphaItemInInput = AVIF_FALSE;
return AVIF_RESULT_OUT_OF_MEMORY;
}
+ // avifMetaFindItem() could invalidate all existing item pointers. So reset the colorItem pointers.
+ *colorItemPtr = &data->meta->items.item[colorItemIndex];
+ colorItem = *colorItemPtr;
+
memcpy((*alphaItem)->type, "grid", 4);
(*alphaItem)->width = colorItem->width;
(*alphaItem)->height = colorItem->height;
@@ -3931,7 +3940,7 @@ avifResult avifDecoderReset(avifDecoder * decoder)

avifBool isAlphaItemInInput;
avifDecoderItem * alphaItem;
- AVIF_CHECKRES(avifDecoderDataFindAlphaItem(data, colorItem, &alphaItem, &isAlphaItemInInput));
+ AVIF_CHECKRES(avifDecoderDataFindAlphaItem(data, &colorItem, &alphaItem, &isAlphaItemInInput));
avifCodecType alphaCodecType = AVIF_CODEC_TYPE_UNKNOWN;
if (alphaItem) {
if (!memcmp(alphaItem->type, "grid", 4)) {
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Vignesh Venkatasubramanian <vigneshv@google.com>
Date: Mon, 13 Nov 2023 19:39:10 -0800
Subject: Remove potential out of bound access to alphaItemIndices

It is possible to craft a file that has more alpha auxiliary items
than color items and trigger an out of bound access into
alphaItemIndices in the for loop.

Fix is to ensure that each color grid item has exactly one alpha
grid item. Also, ensure that there are exactly the same number of
color grids as informed in the grid config before trying to
find the alpha item.

Also, update a diagnostic error message to cover all cases (i.e.)
there can be more grids than necessary as well.

diff --git a/src/read.c b/src/read.c
index e4021898eb120828544afdf0f28cc0e5d4ede876..d8699bb1442645d358f13f1904d7fbb9237bb999 100644
--- a/src/read.c
+++ b/src/read.c
@@ -1417,7 +1417,7 @@ static avifBool avifDecoderGenerateImageGridTiles(avifDecoder * decoder, avifIma

if (tilesAvailable != grid->rows * grid->columns) {
avifDiagnosticsPrintf(&decoder->diag,
- "Grid image of dimensions %ux%u requires %u tiles, and only %u were found",
+ "Grid image of dimensions %ux%u requires %u tiles, but %u were found",
grid->columns,
grid->rows,
grid->rows * grid->columns,
@@ -3641,21 +3641,41 @@ static avifResult avifDecoderDataFindAlphaItem(avifDecoderData * data,
maxItemID = item->id;
}
if (item->dimgForID == colorItem->id) {
+ avifBool seenAlphaForCurrentItem = AVIF_FALSE;
for (uint32_t j = 0; j < colorItem->meta->items.count; ++j) {
avifDecoderItem * auxlItem = &colorItem->meta->items.item[j];
if (avifDecoderItemIsAlphaAux(auxlItem, item->id)) {
+ if (seenAlphaForCurrentItem || auxlItem->dimgForID != 0) {
+ // One of the following invalid cases:
+ // * Multiple items are claiming to be the alpha auxiliary of the current item.
+ // * Alpha auxiliary is dimg for another item.
+ avifFree(alphaItemIndices);
+ *isAlphaItemInInput = AVIF_FALSE;
+ return AVIF_RESULT_INVALID_IMAGE_GRID;
+ }
alphaItemIndices[alphaItemCount++] = j;
+ seenAlphaForCurrentItem = AVIF_TRUE;
}
}
+ if (!seenAlphaForCurrentItem) {
+ // No alpha auxiliary item was found for the current item. Treat this as an image without alpha.
+ avifFree(alphaItemIndices);
+ *isAlphaItemInInput = AVIF_FALSE;
+ return AVIF_RESULT_OK;
+ }
}
}
- if (alphaItemCount != colorItemCount) {
- // Not all the color items had an alpha auxiliary attached to it. Report this case as an image without alpha channel.
- avifFree(alphaItemIndices);
- *alphaItem = NULL;
- *isAlphaItemInInput = AVIF_FALSE;
- return AVIF_RESULT_OK;
+ assert(alphaItemCount == colorItemCount);
+
+ int colorItemIndex = -1;
+ for (uint32_t i = 0; i < data->meta->items.count; ++i) {
+ if (colorItem->id == data->meta->items.item[i].id) {
+ colorItemIndex = i;
+ break;
+ }
}
+ assert(colorItemIndex >= 0);
+
*alphaItem = avifMetaFindItem(colorItem->meta, maxItemID + 1);
if (*alphaItem == NULL) {
avifFree(alphaItemIndices);
1 change: 1 addition & 0 deletions patches/openscreen/.patches
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
cherry-pick-971d6055e7b7.patch

0 comments on commit 9be7bc5

Please sign in to comment.