Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: cherry-pick 138b748dd0a4 from chromium #25232

Merged
merged 3 commits into from Sep 2, 2020
Merged

chore: cherry-pick 138b748dd0a4 from chromium #25232

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3c9834a
chore: cherry-pick 138b748dd0a4 from chromium
nornagon Aug 31, 2020
fd7bbe5
update patches
Aug 31, 2020
99e1aff
Merge branch '8-x-y' into cherry-pick/8-x-y/chromium/138b748dd0a4
nornagon Sep 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions patches/chromium/.patches
View file
Open in desktop
Expand Up @@ -121,3 +121,4 @@ backport_1081722.patch
backport_1073409.patch
backport_1074340.patch
cherry-pick-70579363ce7b.patch
cherry-pick-138b748dd0a4.patch
64 changes: 64 additions & 0 deletions patches/chromium/cherry-pick-138b748dd0a4.patch
View file
Open in desktop
@@ -0,0 +1,64 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Alexander Cooper <alcooper@chromium.org>
Date: Tue, 4 Aug 2020 00:31:54 +0000
Subject: Update FocusChanged notifiers to operate on a copy

These focus changed calls ultimately trigger javascript events. These
events could potentially run code that would modify the list of items
that the FocusChanged notifiers are notifying, and thus invalidate their
in-use iterators.

Fix this by having these methods iterate over a copy instead of the
member list.

(cherry picked from commit d8f526f4e25c24ed29e60b46b3416bfabd5e8f11)

Fixed: 1107815
Change-Id: I03fa08eeadc60736f3a3fae079253dbd3ee26476
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2314158
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Klaus Weidner <klausw@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Auto-Submit: Alexander Cooper <alcooper@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#791261}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2335893
Reviewed-by: Alexander Cooper <alcooper@chromium.org>
Commit-Queue: Alexander Cooper <alcooper@chromium.org>
Cr-Commit-Position: refs/branch-heads/4147@{#1015}
Cr-Branched-From: 16307825352720ae04d898f37efa5449ad68b606-refs/heads/master@{#768962}

diff --git a/third_party/blink/renderer/core/page/focus_controller.cc b/third_party/blink/renderer/core/page/focus_controller.cc
index 03eef54ac1e7b8f4cc635b5315f6ecb0d27e0019..19615ab1bcbf1d0a6e87f5dd549d014ba6826356 100644
--- a/third_party/blink/renderer/core/page/focus_controller.cc
+++ b/third_party/blink/renderer/core/page/focus_controller.cc
@@ -1326,7 +1326,12 @@ void FocusController::RegisterFocusChangedObserver(
}

void FocusController::NotifyFocusChangedObservers() const {
- for (const auto& it : focus_changed_observers_)
+ // Since this eventually dispatches an event to the page, the page could add
+ // new observer, which would invalidate our iterators; so iterate over a copy
+ // of the observer list.
+ HeapHashSet<WeakMember<FocusChangedObserver>> observers =
+ focus_changed_observers_;
+ for (const auto& it : observers)
it->FocusedFrameChanged();
}

diff --git a/third_party/blink/renderer/modules/xr/xr.cc b/third_party/blink/renderer/modules/xr/xr.cc
index 0f0a0c792c2a7479228f377488a413c7b32e3854..968a1a4dca1a680e00870e42258f4824963f6b16 100644
--- a/third_party/blink/renderer/modules/xr/xr.cc
+++ b/third_party/blink/renderer/modules/xr/xr.cc
@@ -524,7 +524,11 @@ XR::XR(LocalFrame& frame, int64_t ukm_source_id)

void XR::FocusedFrameChanged() {
// Tell all sessions that focus changed.
- for (const auto& session : sessions_) {
+ // Since this eventually dispatches an event to the page, the page could
+ // create a new session which would invalidate our iterators; so iterate over
+ // a copy of the session map.
+ HeapHashSet<WeakMember<XRSession>> processing_sessions = sessions_;
+ for (const auto& session : processing_sessions) {
session->OnFocusChanged();
}