From 9d6d02b341b64c17545effaf05ccfa7753d14f33 Mon Sep 17 00:00:00 2001 From: Henri Sivonen Date: Mon, 22 Feb 2021 10:51:51 +0000 Subject: [PATCH] Bug 1663931 - Avoid moving focus when changing iframe remoteness. r=nika,mccr8 Differential Revision: https://phabricator.services.mozilla.com/D99215 --- dom/base/WindowDestroyedEvent.cpp | 11 +++ dom/base/nsFocusManager.cpp | 77 +++++++++++++++++-- dom/base/nsFocusManager.h | 5 +- dom/events/test/mochitest.ini | 2 +- layout/base/tests/mochitest.ini | 2 +- ...fferent-site-iframe-contentwindow.html.ini | 4 - ...ng-same-site-iframe-contentwindow.html.ini | 3 - ...s-before-iframe-loaded-different-site.html | 16 ++++ .../focus-before-iframe-loaded-same-site.html | 16 ++++ ...k-before-iframe-loaded-different-site.html | 16 ++++ ...t-tick-before-iframe-loaded-same-site.html | 16 ++++ ...re-iframe-loaded-different-site-inner.html | 17 ++++ ...frame-loaded-different-site-outer.sub.html | 42 ++++++++++ ...-before-iframe-loaded-same-site-inner.html | 17 ++++ ...-before-iframe-loaded-same-site-outer.html | 42 ++++++++++ ...re-iframe-loaded-different-site-inner.html | 17 ++++ ...frame-loaded-different-site-outer.sub.html | 50 ++++++++++++ ...-before-iframe-loaded-same-site-inner.html | 17 ++++ ...-before-iframe-loaded-same-site-outer.html | 50 ++++++++++++ ...g-different-site-iframe-contentwindow.html | 13 +--- ...cusing-same-site-iframe-contentwindow.html | 13 +--- ...erent-site-iframe-inner-contentwindow.html | 4 +- ...-same-site-iframe-inner-contentwindow.html | 4 +- .../passwordmgr/test/mochitest/mochitest.ini | 1 + 24 files changed, 412 insertions(+), 43 deletions(-) delete mode 100644 testing/web-platform/meta/focus/activeelement-after-immediately-focusing-different-site-iframe-contentwindow.html.ini delete mode 100644 testing/web-platform/meta/focus/activeelement-after-immediately-focusing-same-site-iframe-contentwindow.html.ini create mode 100644 testing/web-platform/mozilla/tests/focus/focus-before-iframe-loaded-different-site.html create mode 100644 testing/web-platform/mozilla/tests/focus/focus-before-iframe-loaded-same-site.html create mode 100644 testing/web-platform/mozilla/tests/focus/focus-next-tick-before-iframe-loaded-different-site.html create mode 100644 testing/web-platform/mozilla/tests/focus/focus-next-tick-before-iframe-loaded-same-site.html create mode 100644 testing/web-platform/mozilla/tests/focus/support/focus-before-iframe-loaded-different-site-inner.html create mode 100644 testing/web-platform/mozilla/tests/focus/support/focus-before-iframe-loaded-different-site-outer.sub.html create mode 100644 testing/web-platform/mozilla/tests/focus/support/focus-before-iframe-loaded-same-site-inner.html create mode 100644 testing/web-platform/mozilla/tests/focus/support/focus-before-iframe-loaded-same-site-outer.html create mode 100644 testing/web-platform/mozilla/tests/focus/support/focus-next-tick-before-iframe-loaded-different-site-inner.html create mode 100644 testing/web-platform/mozilla/tests/focus/support/focus-next-tick-before-iframe-loaded-different-site-outer.sub.html create mode 100644 testing/web-platform/mozilla/tests/focus/support/focus-next-tick-before-iframe-loaded-same-site-inner.html create mode 100644 testing/web-platform/mozilla/tests/focus/support/focus-next-tick-before-iframe-loaded-same-site-outer.html diff --git a/dom/base/WindowDestroyedEvent.cpp b/dom/base/WindowDestroyedEvent.cpp index d1d1c4fbdfc9..5caca8585b83 100644 --- a/dom/base/WindowDestroyedEvent.cpp +++ b/dom/base/WindowDestroyedEvent.cpp @@ -19,6 +19,7 @@ #include "mozilla/BasePrincipal.h" #include "mozilla/Components.h" #include "mozilla/ProfilerLabels.h" +#include "nsFocusManager.h" namespace mozilla { @@ -52,6 +53,8 @@ NS_IMETHODIMP WindowDestroyedEvent::Run() { AUTO_PROFILER_LABEL("WindowDestroyedEvent::Run", OTHER); + nsCOMPtr nukedOuter; + nsCOMPtr observerService = services::GetObserverService(); if (!observerService) { return NS_OK; @@ -105,6 +108,7 @@ WindowDestroyedEvent::Run() { nsGlobalWindowOuter* outer = nsGlobalWindowOuter::FromSupports(window); currentInner = outer->GetCurrentInnerWindowInternal(); + nukedOuter = outer; } NS_ENSURE_TRUE(currentInner, NS_OK); @@ -139,6 +143,13 @@ WindowDestroyedEvent::Run() { } break; } + if (nukedOuter) { + nsFocusManager* fm = nsFocusManager::GetFocusManager(); + if (fm) { + fm->WasNuked(nukedOuter); + } + } + return NS_OK; } diff --git a/dom/base/nsFocusManager.cpp b/dom/base/nsFocusManager.cpp index b3b54771d09b..f09658865266 100644 --- a/dom/base/nsFocusManager.cpp +++ b/dom/base/nsFocusManager.cpp @@ -960,8 +960,17 @@ void nsFocusManager::WindowShown(mozIDOMWindowProxy* aWindow, } } - if (mFocusedWindow != window) { - return; + if (XRE_IsParentProcess()) { + if (mFocusedWindow != window) { + return; + } + } else { + BrowsingContext* bc = window->GetBrowsingContext(); + if (!bc || mFocusedBrowsingContextInContent != bc) { + return; + } + // Sync the window for a newly-created OOP iframe + SetFocusedWindowInternal(window, false); } if (aNeedsFocus) { @@ -1056,13 +1065,51 @@ void nsFocusManager::WindowHidden(mozIDOMWindowProxy* aWindow, SetCaretVisible(presShell, false, nullptr); } + // If a window is being "hidden" because its BrowsingContext is changing + // remoteness, we don't want to handle docshell destruction by moving focus. + // Instead, the focused browsing context should stay the way it is (so that + // the newly "shown" window in the other process knows to take focus) and + // we should just null out the process-local field. + nsCOMPtr docShellBeingHidden = window->GetDocShell(); + // Check if we're currently hiding a non-remote nsDocShell due to its + // BrowsingContext navigating to become remote. Normally, when a focused + // subframe is hidden, focus is moved to the frame element, but focus should + // stay with the BrowsingContext when performing a process switch. We don't + // need to consider process switches where the hiding docshell is already + // remote (ie. GetEmbedderElement is nullptr), as shifting remoteness to the + // frame element is handled elsewhere. + if (nsDocShell::Cast(docShellBeingHidden)->WillChangeProcess() && + docShellBeingHidden->GetBrowsingContext()->GetEmbedderElement()) { + if (mFocusedWindow != window) { + // The window being hidden is an ancestor of the focused window. +#ifdef DEBUG + BrowsingContext* ancestor = window->GetBrowsingContext(); + BrowsingContext* bc = mFocusedWindow->GetBrowsingContext(); + for (;;) { + if (!bc) { + MOZ_ASSERT(false, "Should have found ancestor"); + } + bc = bc->GetParent(); + if (ancestor == bc) { + break; + } + } +#endif + // This call adjusts the focused browsing context and window. + // The latter gets nulled out immediately below. + SetFocusedWindowInternal(window); + } + mFocusedWindow = nullptr; + window->SetFocusedElement(nullptr); + return; + } + // if the docshell being hidden is being destroyed, then we want to move // focus somewhere else. Call ClearFocus on the toplevel window, which // will have the effect of clearing the focus and moving the focused window // to the toplevel window. But if the window isn't being destroyed, we are // likely just loading a new document in it, so we want to maintain the // focused window so that the new document gets properly focused. - nsCOMPtr docShellBeingHidden = window->GetDocShell(); bool beingDestroyed = !docShellBeingHidden; if (docShellBeingHidden) { docShellBeingHidden->IsBeingDestroyed(&beingDestroyed); @@ -1150,6 +1197,17 @@ void nsFocusManager::FireDelayedEvents(Document* aDocument) { } } +void nsFocusManager::WasNuked(nsPIDOMWindowOuter* aWindow) { + MOZ_ASSERT(aWindow, "Expected non-null window."); + MOZ_ASSERT(aWindow != mActiveWindow, + "How come we're nuking a window that's still active?"); + if (aWindow == mFocusedWindow) { + mFocusedWindow = nullptr; + SetFocusedBrowsingContext(nullptr); + mFocusedElement = nullptr; + } +} + nsresult nsFocusManager::FocusPlugin(Element* aPlugin) { NS_ENSURE_ARG(aPlugin); SetFocusInner(aPlugin, 0, true, false, GenerateFocusActionId()); @@ -1384,8 +1442,6 @@ void nsFocusManager::SetFocusInner(Element* aNewContent, int32_t aFlags, nsCOMPtr newWindow; nsCOMPtr subWindow = GetContentWindow(elementToFocus); if (subWindow) { - // XXX What if this is an out-of-process iframe? - // https://bugzilla.mozilla.org/show_bug.cgi?id=1613054 elementToFocus = GetFocusedDescendant(subWindow, eIncludeAllDescendants, getter_AddRefs(newWindow)); @@ -4882,7 +4938,8 @@ static bool IsInPointerLockContext(nsPIDOMWindowOuter* aWin) { : nullptr); } -void nsFocusManager::SetFocusedWindowInternal(nsPIDOMWindowOuter* aWindow) { +void nsFocusManager::SetFocusedWindowInternal(nsPIDOMWindowOuter* aWindow, + bool aSyncBrowsingContext) { if (XRE_IsParentProcess() && !PointerUnlocker::sActiveUnlocker && IsInPointerLockContext(mFocusedWindow) && !IsInPointerLockContext(aWindow)) { @@ -4900,7 +4957,13 @@ void nsFocusManager::SetFocusedWindowInternal(nsPIDOMWindowOuter* aWindow) { } mFocusedWindow = aWindow; - SetFocusedBrowsingContext(aWindow ? aWindow->GetBrowsingContext() : nullptr); + BrowsingContext* bc = aWindow ? aWindow->GetBrowsingContext() : nullptr; + if (aSyncBrowsingContext) { + SetFocusedBrowsingContext(bc); + } else if (XRE_IsContentProcess()) { + MOZ_ASSERT(mFocusedBrowsingContextInContent == bc, + "Not syncing BrowsingContext even when different."); + } } void nsFocusManager::NotifyOfReFocus(nsIContent& aContent) { diff --git a/dom/base/nsFocusManager.h b/dom/base/nsFocusManager.h index f04efb831030..dea4104ba457 100644 --- a/dom/base/nsFocusManager.h +++ b/dom/base/nsFocusManager.h @@ -258,6 +258,8 @@ class nsFocusManager final : public nsIFocusManager, */ void FireDelayedEvents(Document* aDocument); + void WasNuked(nsPIDOMWindowOuter* aWindow); + /** * Indicate that a plugin wishes to take the focus. This is similar to a * normal focus except that the widget focus is not changed. Updating the @@ -752,7 +754,8 @@ class nsFocusManager final : public nsIFocusManager, int32_t aFlags, bool aGettingFocus, bool aShouldShowFocusRing); - void SetFocusedWindowInternal(nsPIDOMWindowOuter* aWindow); + void SetFocusedWindowInternal(nsPIDOMWindowOuter* aWindow, + bool aSyncBrowsingContext = true); bool TryDocumentNavigation(nsIContent* aCurrentContent, bool* aCheckSubDocument, diff --git a/dom/events/test/mochitest.ini b/dom/events/test/mochitest.ini index 7f3251cc4465..b3f99a734c45 100644 --- a/dom/events/test/mochitest.ini +++ b/dom/events/test/mochitest.ini @@ -46,7 +46,7 @@ support-files = test_bug336682.js [test_bug402089.html] [test_bug405632.html] [test_bug409604.html] -skip-if = toolkit == 'android' #TIMED_OUT +skip-if = xorigin || toolkit == 'android' # xorigin: Bug 1682519, android: TIMED_OUT [test_bug412567.html] [test_bug418986-3.html] [test_bug422132.html] diff --git a/layout/base/tests/mochitest.ini b/layout/base/tests/mochitest.ini index 273838c1d155..eb54a2b61664 100644 --- a/layout/base/tests/mochitest.ini +++ b/layout/base/tests/mochitest.ini @@ -91,7 +91,7 @@ support-files = file_bug842853-frame.html skip-if = toolkit == 'android' # Bug 1355821 [test_bug842853-2.html] -skip-if = toolkit == 'android' # Bug 1355821 +skip-if = xorigin || toolkit == 'android' # Bug 1682493, 1355821 [test_bug849219.html] [test_bug851445.html] skip-if = toolkit == 'android' # Bug 1355821 diff --git a/testing/web-platform/meta/focus/activeelement-after-immediately-focusing-different-site-iframe-contentwindow.html.ini b/testing/web-platform/meta/focus/activeelement-after-immediately-focusing-different-site-iframe-contentwindow.html.ini deleted file mode 100644 index eb860760dfe4..000000000000 --- a/testing/web-platform/meta/focus/activeelement-after-immediately-focusing-different-site-iframe-contentwindow.html.ini +++ /dev/null @@ -1,4 +0,0 @@ -[activeelement-after-immediately-focusing-different-site-iframe-contentwindow.html] - [Check trailing events] - expected: - if not fission: FAIL diff --git a/testing/web-platform/meta/focus/activeelement-after-immediately-focusing-same-site-iframe-contentwindow.html.ini b/testing/web-platform/meta/focus/activeelement-after-immediately-focusing-same-site-iframe-contentwindow.html.ini deleted file mode 100644 index 35c48ba9f80f..000000000000 --- a/testing/web-platform/meta/focus/activeelement-after-immediately-focusing-same-site-iframe-contentwindow.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[activeelement-after-immediately-focusing-same-site-iframe-contentwindow.html] - [Check trailing events] - expected: FAIL diff --git a/testing/web-platform/mozilla/tests/focus/focus-before-iframe-loaded-different-site.html b/testing/web-platform/mozilla/tests/focus/focus-before-iframe-loaded-different-site.html new file mode 100644 index 000000000000..e76599014581 --- /dev/null +++ b/testing/web-platform/mozilla/tests/focus/focus-before-iframe-loaded-different-site.html @@ -0,0 +1,16 @@ + + +focus() before iframe loaded different site + + + diff --git a/testing/web-platform/mozilla/tests/focus/focus-before-iframe-loaded-same-site.html b/testing/web-platform/mozilla/tests/focus/focus-before-iframe-loaded-same-site.html new file mode 100644 index 000000000000..fd7c2bffa602 --- /dev/null +++ b/testing/web-platform/mozilla/tests/focus/focus-before-iframe-loaded-same-site.html @@ -0,0 +1,16 @@ + + +focus() before iframe loaded same site + + + diff --git a/testing/web-platform/mozilla/tests/focus/focus-next-tick-before-iframe-loaded-different-site.html b/testing/web-platform/mozilla/tests/focus/focus-next-tick-before-iframe-loaded-different-site.html new file mode 100644 index 000000000000..58310fd687cc --- /dev/null +++ b/testing/web-platform/mozilla/tests/focus/focus-next-tick-before-iframe-loaded-different-site.html @@ -0,0 +1,16 @@ + + +focus() from next tick before iframe loaded different site + + + diff --git a/testing/web-platform/mozilla/tests/focus/focus-next-tick-before-iframe-loaded-same-site.html b/testing/web-platform/mozilla/tests/focus/focus-next-tick-before-iframe-loaded-same-site.html new file mode 100644 index 000000000000..01b467718c2a --- /dev/null +++ b/testing/web-platform/mozilla/tests/focus/focus-next-tick-before-iframe-loaded-same-site.html @@ -0,0 +1,16 @@ + + +focus() from next tick before iframe loaded same site + + + diff --git a/testing/web-platform/mozilla/tests/focus/support/focus-before-iframe-loaded-different-site-inner.html b/testing/web-platform/mozilla/tests/focus/support/focus-before-iframe-loaded-different-site-inner.html new file mode 100644 index 000000000000..bcf23627d2e6 --- /dev/null +++ b/testing/web-platform/mozilla/tests/focus/support/focus-before-iframe-loaded-different-site-inner.html @@ -0,0 +1,17 @@ + + + + +focus() before iframe loaded different site + + + + + diff --git a/testing/web-platform/mozilla/tests/focus/support/focus-before-iframe-loaded-different-site-outer.sub.html b/testing/web-platform/mozilla/tests/focus/support/focus-before-iframe-loaded-different-site-outer.sub.html new file mode 100644 index 000000000000..e95fe7d29252 --- /dev/null +++ b/testing/web-platform/mozilla/tests/focus/support/focus-before-iframe-loaded-different-site-outer.sub.html @@ -0,0 +1,42 @@ + + + +focus() before iframe loaded different site + + + + + diff --git a/testing/web-platform/mozilla/tests/focus/support/focus-before-iframe-loaded-same-site-inner.html b/testing/web-platform/mozilla/tests/focus/support/focus-before-iframe-loaded-same-site-inner.html new file mode 100644 index 000000000000..3c277f078ff9 --- /dev/null +++ b/testing/web-platform/mozilla/tests/focus/support/focus-before-iframe-loaded-same-site-inner.html @@ -0,0 +1,17 @@ + + + + +focus() before iframe loaded same site + + + + + diff --git a/testing/web-platform/mozilla/tests/focus/support/focus-before-iframe-loaded-same-site-outer.html b/testing/web-platform/mozilla/tests/focus/support/focus-before-iframe-loaded-same-site-outer.html new file mode 100644 index 000000000000..8c829d6d47d0 --- /dev/null +++ b/testing/web-platform/mozilla/tests/focus/support/focus-before-iframe-loaded-same-site-outer.html @@ -0,0 +1,42 @@ + + + +focus() before iframe loaded same site + + + + + diff --git a/testing/web-platform/mozilla/tests/focus/support/focus-next-tick-before-iframe-loaded-different-site-inner.html b/testing/web-platform/mozilla/tests/focus/support/focus-next-tick-before-iframe-loaded-different-site-inner.html new file mode 100644 index 000000000000..2c1b35f2b237 --- /dev/null +++ b/testing/web-platform/mozilla/tests/focus/support/focus-next-tick-before-iframe-loaded-different-site-inner.html @@ -0,0 +1,17 @@ + + + + +focus() from next tick before iframe loaded different site + + + + + diff --git a/testing/web-platform/mozilla/tests/focus/support/focus-next-tick-before-iframe-loaded-different-site-outer.sub.html b/testing/web-platform/mozilla/tests/focus/support/focus-next-tick-before-iframe-loaded-different-site-outer.sub.html new file mode 100644 index 000000000000..83a48e303d19 --- /dev/null +++ b/testing/web-platform/mozilla/tests/focus/support/focus-next-tick-before-iframe-loaded-different-site-outer.sub.html @@ -0,0 +1,50 @@ + + + +focus() from next tick before iframe loaded different site + + + + + diff --git a/testing/web-platform/mozilla/tests/focus/support/focus-next-tick-before-iframe-loaded-same-site-inner.html b/testing/web-platform/mozilla/tests/focus/support/focus-next-tick-before-iframe-loaded-same-site-inner.html new file mode 100644 index 000000000000..62add75ed9fa --- /dev/null +++ b/testing/web-platform/mozilla/tests/focus/support/focus-next-tick-before-iframe-loaded-same-site-inner.html @@ -0,0 +1,17 @@ + + + + +focus() from next tick before iframe loaded same site + + + + + diff --git a/testing/web-platform/mozilla/tests/focus/support/focus-next-tick-before-iframe-loaded-same-site-outer.html b/testing/web-platform/mozilla/tests/focus/support/focus-next-tick-before-iframe-loaded-same-site-outer.html new file mode 100644 index 000000000000..a68d34039db5 --- /dev/null +++ b/testing/web-platform/mozilla/tests/focus/support/focus-next-tick-before-iframe-loaded-same-site-outer.html @@ -0,0 +1,50 @@ + + + +focus() from next tick before iframe loaded same site + + + + + diff --git a/testing/web-platform/tests/focus/activeelement-after-immediately-focusing-different-site-iframe-contentwindow.html b/testing/web-platform/tests/focus/activeelement-after-immediately-focusing-different-site-iframe-contentwindow.html index 4cb3c68db63b..8c3a9d119576 100644 --- a/testing/web-platform/tests/focus/activeelement-after-immediately-focusing-different-site-iframe-contentwindow.html +++ b/testing/web-platform/tests/focus/activeelement-after-immediately-focusing-different-site-iframe-contentwindow.html @@ -6,19 +6,8 @@