Bug 1553769 - Have a single way of requesting window focus and switching to a tab. r=NeilDeakin,snorp

Right now there's some duplicated code with the focus manager and the
DOMWindowFocus event.

Android didn't handle the new framefocusrequested event, so the test-cases in
bug 416771 still didn't work there.

I think using the focus manager codepath everywhere is preferable. I confirmed
manually that the stuff that sent DOMWindowFocus events still works as expected
with this patch (i.e., switching to the right tab when you click on a
notification, etc.).

This fixes it so that it works in Fennec, and it sends the focus events right in
GeckoView Example (i.e., we get here[1] properly).

The snippet that Snorp provided on IRC to implement the "bring activity to
front" stuff (`startActivity(getIntent())`) didn't actually work for me, but I
confirmed that the right message is sent when the focus is requested, and that
we get there.

[1]: https://searchfox.org/mozilla-central/rev/952521e6164ddffa3f34bc8cfa5a81afc5b859c4/mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java#503

Depends on D32353

Differential Revision: https://phabricator.services.mozilla.com/D32354

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Emilio Cobos Álvarez 2019-06-03 19:42:28 +00:00
parent 8c0163caeb
commit b6728cedb7
14 changed files with 67 additions and 93 deletions

View File

@ -68,7 +68,3 @@ ContentMetaHandler.init(this);
// This is a temporary hack to prevent regressions (bug 1471327).
void content;
addEventListener("DOMWindowFocus", function(event) {
sendAsyncMessage("DOMWindowFocus", {});
}, false);

View File

@ -50,7 +50,6 @@ window._gBrowser = {
this._outerWindowIDBrowserMap.set(this.selectedBrowser.outerWindowID,
this.selectedBrowser);
}
messageManager.addMessageListener("DOMWindowFocus", this);
messageManager.addMessageListener("RefreshBlocker:Blocked", this);
messageManager.addMessageListener("Browser:WindowCreated", this);
@ -4340,15 +4339,6 @@ window._gBrowser = {
openContextMenu(aMessage);
break;
}
case "DOMWindowFocus":
{
let tab = this.getTabForBrowser(browser);
if (!tab)
return undefined;
this.selectedTab = tab;
window.focus();
break;
}
case "Browser:Init":
{
let tab = this.getTabForBrowser(browser);

View File

@ -8924,33 +8924,29 @@ nsresult nsDocShell::PerformRetargeting(nsDocShellLoadState* aLoadState,
targetDocShell = do_QueryInterface(webNav);
}
NS_ENSURE_SUCCESS(rv, rv);
NS_ENSURE_TRUE(targetDocShell, rv);
//
// Transfer the load to the target DocShell... Pass empty string as the
// window target name from to prevent recursive retargeting!
//
if (NS_SUCCEEDED(rv) && targetDocShell) {
nsDocShell* docShell = nsDocShell::Cast(targetDocShell);
// No window target
aLoadState->SetTarget(EmptyString());
// No forced download
aLoadState->SetFileName(VoidString());
rv = docShell->InternalLoad(aLoadState, aDocShell, aRequest);
nsDocShell* docShell = nsDocShell::Cast(targetDocShell);
// No window target
aLoadState->SetTarget(EmptyString());
// No forced download
aLoadState->SetFileName(VoidString());
rv = docShell->InternalLoad(aLoadState, aDocShell, aRequest);
NS_ENSURE_SUCCESS(rv, rv);
if (NS_SUCCEEDED(rv)) {
// Switch to target tab if we're currently focused window.
// Take loadDivertedInBackground into account so the behavior would be
// the same as how the tab first opened.
bool isTargetActive = false;
targetDocShell->GetIsActive(&isTargetActive);
nsCOMPtr<nsPIDOMWindowOuter> domWin = targetDocShell->GetWindow();
if (mIsActive && !isTargetActive && domWin &&
!Preferences::GetBool("browser.tabs.loadDivertedInBackground",
false)) {
if (NS_FAILED(nsContentUtils::DispatchFocusChromeEvent(domWin))) {
return NS_ERROR_FAILURE;
}
}
}
// Switch to target tab if we're currently focused window.
// Take loadDivertedInBackground into account so the behavior would be
// the same as how the tab first opened.
bool isTargetActive = false;
targetDocShell->GetIsActive(&isTargetActive);
nsCOMPtr<nsPIDOMWindowOuter> domWin = targetDocShell->GetWindow();
if (mIsActive && !isTargetActive && domWin &&
!Preferences::GetBool("browser.tabs.loadDivertedInBackground", false)) {
nsFocusManager::FocusWindow(domWin);
}
// Else we ran out of memory, or were a popup and got blocked,

View File

@ -4163,19 +4163,6 @@ nsresult nsContentUtils::DispatchChromeEvent(
return err.StealNSResult();
}
/* static */
nsresult nsContentUtils::DispatchFocusChromeEvent(nsPIDOMWindowOuter* aWindow) {
MOZ_ASSERT(aWindow);
RefPtr<Document> doc = aWindow->GetExtantDoc();
if (!doc) {
return NS_ERROR_FAILURE;
}
return DispatchChromeEvent(doc, aWindow, NS_LITERAL_STRING("DOMWindowFocus"),
CanBubble::eYes, Cancelable::eYes);
}
void nsContentUtils::RequestFrameFocus(Element& aFrameElement, bool aCanRaise) {
RefPtr<Element> target = &aFrameElement;
bool defaultAction = true;

View File

@ -1514,14 +1514,6 @@ class nsContentUtils {
Cancelable,
bool* aDefaultAction = nullptr);
/**
* Helper function for dispatching a "DOMWindowFocus" event to
* the chrome event handler of the given DOM Window. This has the effect
* of focusing the corresponding tab and bringing the browser window
* to the foreground.
*/
static nsresult DispatchFocusChromeEvent(nsPIDOMWindowOuter* aWindow);
/**
* Helper to dispatch a "framefocusrequested" event to chrome, which will only
* bring the window to the foreground and switch tabs if aCanRaise is true.

View File

@ -373,6 +373,12 @@ nsFocusManager::GetActiveWindow(mozIDOMWindowProxy** aWindow) {
return NS_OK;
}
void nsFocusManager::FocusWindow(nsPIDOMWindowOuter* aWindow) {
if (RefPtr<nsFocusManager> fm = sInstance) {
fm->SetFocusedWindow(aWindow);
}
}
NS_IMETHODIMP
nsFocusManager::SetActiveWindow(mozIDOMWindowProxy* aWindow) {
NS_ENSURE_STATE(aWindow);

View File

@ -58,6 +58,11 @@ class nsFocusManager final : public nsIFocusManager,
static nsresult Init();
static void Shutdown();
// Simple helper to call SetFocusedWindow on the instance.
//
// This raises the window and switches to the tab as needed.
static void FocusWindow(nsPIDOMWindowOuter* aWindow);
void PrefChanged(const char* aPref);
/**

View File

@ -10,6 +10,7 @@
#include "ClientState.h"
#include "mozilla/SystemGroup.h"
#include "nsContentUtils.h"
#include "nsFocusManager.h"
#include "nsIBrowserDOMWindow.h"
#include "nsIDocShell.h"
#include "nsIDOMChromeWindow.h"
@ -276,14 +277,10 @@ void WaitForLoad(const ClientOpenWindowArgs& aArgs,
RefPtr<ClientOpPromise::Private> promise = aPromise;
nsresult rv = nsContentUtils::DispatchFocusChromeEvent(aOuterWindow);
if (NS_WARN_IF(NS_FAILED(rv))) {
promise->Reject(rv, __func__);
return;
}
nsFocusManager::FocusWindow(aOuterWindow);
nsCOMPtr<nsIURI> baseURI;
rv = NS_NewURI(getter_AddRefs(baseURI), aArgs.baseURL());
nsresult rv = NS_NewURI(getter_AddRefs(baseURI), aArgs.baseURL());
if (NS_WARN_IF(NS_FAILED(rv))) {
promise->Reject(rv, __func__);
return;

View File

@ -26,6 +26,7 @@
#include "mozilla/StorageAccess.h"
#include "nsIContentSecurityPolicy.h"
#include "nsContentUtils.h"
#include "nsFocusManager.h"
#include "nsIDocShell.h"
#include "nsPIDOMWindow.h"
@ -528,14 +529,10 @@ RefPtr<ClientOpPromise> ClientSource::Focus(const ClientFocusArgs& aArgs) {
}
MOZ_ASSERT(NS_IsMainThread());
nsresult rv = nsContentUtils::DispatchFocusChromeEvent(outer);
if (NS_FAILED(rv)) {
return ClientOpPromise::CreateAndReject(rv, __func__);
}
nsFocusManager::FocusWindow(outer);
ClientState state;
rv = SnapshotState(&state);
nsresult rv = SnapshotState(&state);
if (NS_FAILED(rv)) {
return ClientOpPromise::CreateAndReject(rv, __func__);
}

View File

@ -37,6 +37,7 @@
#include "nsContentUtils.h"
#include "nsCRTGlue.h"
#include "nsDOMJSUtils.h"
#include "nsFocusManager.h"
#include "nsGlobalWindow.h"
#include "nsIAlertsService.h"
#include "nsIContentPermissionPrompt.h"
@ -289,10 +290,7 @@ class FocusWindowRunnable final : public Runnable {
return NS_OK;
}
// Browser UI may use DOMWindowFocus to focus the tab
// from which the event was dispatched.
nsContentUtils::DispatchFocusChromeEvent(mWindow->GetOuterWindow());
nsFocusManager::FocusWindow(mWindow->GetOuterWindow());
return NS_OK;
}
};
@ -1221,9 +1219,7 @@ MainThreadNotificationObserver::Observe(nsISupports* aSubject,
bool doDefaultAction = notification->DispatchClickEvent();
if (doDefaultAction) {
// Browser UI may use DOMWindowFocus to focus the tab
// from which the event was dispatched.
nsContentUtils::DispatchFocusChromeEvent(window->GetOuterWindow());
nsFocusManager::FocusWindow(window->GetOuterWindow());
}
} else if (!strcmp("alertfinished", aTopic)) {
notification->UnpersistNotification();

View File

@ -3728,7 +3728,7 @@ Tab.prototype = {
this.browser.addEventListener("pageshow", this, true);
this.browser.addEventListener("MozApplicationManifest", this, true);
this.browser.addEventListener("TabPreZombify", this, true);
this.browser.addEventListener("DOMWindowFocus", this, true);
this.browser.addEventListener("framefocusrequested", this, true);
this.browser.addEventListener("focusin", this, true);
this.browser.addEventListener("focusout", this, true);
this.browser.addEventListener("TabSelect", this, true);
@ -3870,7 +3870,7 @@ Tab.prototype = {
this.browser.removeEventListener("pageshow", this, true);
this.browser.removeEventListener("MozApplicationManifest", this, true);
this.browser.removeEventListener("TabPreZombify", this, true);
this.browser.removeEventListener("DOMWindowFocus", this, true);
this.browser.removeEventListener("framefocusrequested", this, true);
this.browser.removeEventListener("focusin", this, true);
this.browser.removeEventListener("focusout", this, true);
this.browser.removeEventListener("TabSelect", this, true);
@ -4389,12 +4389,15 @@ Tab.prototype = {
break;
}
case "DOMWindowFocus": {
GlobalEventDispatcher.sendRequest({
type: "Tab:Select",
tabID: this.id,
foreground: true,
});
case "framefocusrequested": {
if (BrowserApp.selectedTab !== this) {
GlobalEventDispatcher.sendRequest({
type: "Tab:Select",
tabID: this.id,
foreground: true,
});
aEvent.preventDefault();
}
break;
}

View File

@ -70,7 +70,6 @@ class GeckoViewContentChild extends GeckoViewChildModule {
debug `onEnable`;
addEventListener("DOMTitleChanged", this, false);
addEventListener("DOMWindowFocus", this, false);
addEventListener("DOMWindowClose", this, false);
addEventListener("MozDOMFullscreen:Entered", this, false);
addEventListener("MozDOMFullscreen:Exit", this, false);
@ -84,7 +83,6 @@ class GeckoViewContentChild extends GeckoViewChildModule {
debug `onDisable`;
removeEventListener("DOMTitleChanged", this);
removeEventListener("DOMWindowFocus", this);
removeEventListener("DOMWindowClose", this);
removeEventListener("MozDOMFullscreen:Entered", this);
removeEventListener("MozDOMFullscreen:Exit", this);
@ -376,11 +374,6 @@ class GeckoViewContentChild extends GeckoViewChildModule {
title: content.document.title,
});
break;
case "DOMWindowFocus":
this.eventDispatcher.sendRequest({
type: "GeckoView:DOMWindowFocus",
});
break;
case "DOMWindowClose":
if (!aEvent.isTrusted) {
return;

View File

@ -427,9 +427,9 @@ public class GeckoSession implements Parcelable {
"GeckoView:ContentCrash",
"GeckoView:ContextMenu",
"GeckoView:DOMTitleChanged",
"GeckoView:DOMWindowFocus",
"GeckoView:DOMWindowClose",
"GeckoView:ExternalResponse",
"GeckoView:FocusRequest",
"GeckoView:FullScreenEnter",
"GeckoView:FullScreenExit",
"GeckoView:WebAppManifest",
@ -462,7 +462,7 @@ public class GeckoSession implements Parcelable {
} else if ("GeckoView:DOMTitleChanged".equals(event)) {
delegate.onTitleChange(GeckoSession.this,
message.getString("title"));
} else if ("GeckoView:DOMWindowFocus".equals(event)) {
} else if ("GeckoView:FocusRequest".equals(event)) {
delegate.onFocusRequest(GeckoSession.this);
} else if ("GeckoView:DOMWindowClose".equals(event)) {
delegate.onCloseRequest(GeckoSession.this);

View File

@ -34,6 +34,8 @@ class GeckoViewContent extends GeckoViewModule {
/* capture */ true, /* untrusted */ false);
this.window.addEventListener("MozDOMFullscreen:Exited", this,
/* capture */ true, /* untrusted */ false);
this.window.addEventListener("framefocusrequested", this,
/* capture */ true, /* untrusted */ false);
this.messageManager.addMessageListener("GeckoView:DOMFullscreenExit", this);
this.messageManager.addMessageListener("GeckoView:DOMFullscreenRequest", this);
@ -46,6 +48,8 @@ class GeckoViewContent extends GeckoViewModule {
/* capture */ true);
this.window.removeEventListener("MozDOMFullscreen:Exited", this,
/* capture */ true);
this.window.removeEventListener("framefocusrequested", this,
/* capture */ true);
this.messageManager.removeMessageListener("GeckoView:DOMFullscreenExit", this);
this.messageManager.removeMessageListener("GeckoView:DOMFullscreenRequest", this);
@ -114,6 +118,18 @@ class GeckoViewContent extends GeckoViewModule {
debug `handleEvent: ${aEvent.type}`;
switch (aEvent.type) {
case "framefocusrequested":
if (this.browser != aEvent.target) {
return;
}
if (this.browser.hasAttribute("primary")) {
return;
}
this.eventDispatcher.sendRequest({
type: "GeckoView:FocusRequest",
});
aEvent.preventDefault();
break;
case "MozDOMFullscreen:Entered":
if (this.browser == aEvent.target) {
// Remote browser; dispatch to content process.