Bug 1558520, rework remote controller to use JSWindowActor instead of having the browser have a controller, r=smaug

When searching for the controller for a command in nsWindowRoot::GetControllerForCommand, look for a focused browsing context instead and get the controller through the Controllers actor associated with that browsing context. When a command update occurs in a window in the child process, send the list of commands to the parent process along with the browsing context for that window. The parent will pass this information to the controllers actor. As long as we can get the right currently focused browsing context descendant, we can get the correct command state and invoke commands through the right actor.

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

--HG--
rename : toolkit/modules/RemoteController.jsm => toolkit/actors/ControllersParent.jsm
extra : moz-landing-system : lando
This commit is contained in:
Neil Deakin 2020-03-12 16:47:57 +00:00
parent e2ac4281a0
commit 8d3992adb0
19 changed files with 216 additions and 139 deletions

View File

@ -6400,18 +6400,20 @@ namespace {
class ChildCommandDispatcher : public Runnable {
public:
ChildCommandDispatcher(nsPIWindowRoot* aRoot, nsIBrowserChild* aBrowserChild,
const nsAString& aAction)
nsPIDOMWindowOuter* aWindow, const nsAString& aAction)
: mozilla::Runnable("ChildCommandDispatcher"),
mRoot(aRoot),
mBrowserChild(aBrowserChild),
mWindow(aWindow),
mAction(aAction) {}
NS_IMETHOD Run() override {
nsTArray<nsCString> enabledCommands, disabledCommands;
mRoot->GetEnabledDisabledCommands(enabledCommands, disabledCommands);
if (enabledCommands.Length() || disabledCommands.Length()) {
mBrowserChild->EnableDisableCommands(mAction, enabledCommands,
disabledCommands);
BrowserChild* bc = static_cast<BrowserChild*>(mBrowserChild.get());
bc->SendEnableDisableCommands(mWindow->GetBrowsingContext(), mAction,
enabledCommands, disabledCommands);
}
return NS_OK;
@ -6420,6 +6422,7 @@ class ChildCommandDispatcher : public Runnable {
private:
nsCOMPtr<nsPIWindowRoot> mRoot;
nsCOMPtr<nsIBrowserChild> mBrowserChild;
nsCOMPtr<nsPIDOMWindowOuter> mWindow;
nsString mAction;
};
@ -6446,7 +6449,7 @@ void nsGlobalWindowOuter::UpdateCommands(const nsAString& anAction,
nsCOMPtr<nsPIWindowRoot> root = GetTopWindowRoot();
if (root) {
nsContentUtils::AddScriptRunner(
new ChildCommandDispatcher(root, child, anAction));
new ChildCommandDispatcher(root, child, this, anAction));
}
return;
}

View File

@ -16,6 +16,9 @@
#include "nsLayoutCID.h"
#include "nsContentCID.h"
#include "nsString.h"
#include "nsFrameLoaderOwner.h"
#include "nsFrameLoader.h"
#include "nsQueryActor.h"
#include "nsGlobalWindow.h"
#include "nsFocusManager.h"
#include "nsIContent.h"
@ -165,6 +168,57 @@ nsresult nsWindowRoot::GetControllerForCommand(const char* aCommand,
NS_ENSURE_ARG_POINTER(_retval);
*_retval = nullptr;
// If this is the parent process, check if a child browsing context from
// another process is focused, and ask if it has a controller actor that
// supports the command.
if (XRE_IsParentProcess()) {
nsFocusManager* fm = nsFocusManager::GetFocusManager();
if (!fm) {
return NS_ERROR_FAILURE;
}
// Unfortunately, messages updating the active/focus state in the focus
// manager don't happen fast enough in the case when switching focus between
// processes when clicking on a chrome UI element while a child tab is
// focused, so we need to check whether the focus manager thinks a child
// frame is focused as well.
nsCOMPtr<nsPIDOMWindowOuter> focusedWindow;
nsIContent* focusedContent = nsFocusManager::GetFocusedDescendant(
mWindow, nsFocusManager::eIncludeAllDescendants,
getter_AddRefs(focusedWindow));
RefPtr<nsFrameLoaderOwner> loaderOwner = do_QueryObject(focusedContent);
if (loaderOwner) {
// Only check browsing contexts if a remote frame is focused. If chrome is
// focused, just check the controllers directly below.
RefPtr<nsFrameLoader> frameLoader = loaderOwner->GetFrameLoader();
if (frameLoader && frameLoader->IsRemoteFrame()) {
// GetActiveBrowsingContextInChrome actually returns the top-level
// browsing context if the focus is in a child process tab, or null if
// the focus is in chrome.
BrowsingContext* focusedBC =
fm->GetActiveBrowsingContextInChrome()
? fm->GetFocusedBrowsingContextInChrome()
: nullptr;
CanonicalBrowsingContext* canonicalFocusedBC =
CanonicalBrowsingContext::Cast(focusedBC);
if (canonicalFocusedBC) {
// At this point, it is known that a child process is focused, so ask
// its Controllers actor if the command is supported.
nsCOMPtr<nsIController> controller =
do_QueryActor(u"Controllers", canonicalFocusedBC);
if (controller) {
bool supported;
controller->SupportsCommand(aCommand, &supported);
if (supported) {
controller.forget(_retval);
return NS_OK;
}
}
}
}
}
}
{
nsCOMPtr<nsIControllers> controllers;
GetControllers(aForVisibleWindow, getter_AddRefs(controllers));

View File

@ -69,18 +69,6 @@ interface nsIBrowser : nsISupports
*/
readonly attribute nsIWebProgress remoteWebProgressManager;
/**
* Called by the child to inform the parent that a command update has occurred
* and the supplied set of commands are now enabled and disabled.
*
* @param action command updater action
* @param enabledCommands commands to enable
* @param disabledCommand commands to disable
*/
void enableDisableCommandsRemoteOnly(in AString action,
in Array<ACString> enabledCommands,
in Array<ACString> disabledCommands);
readonly attribute nsIPrincipal contentPrincipal;
readonly attribute nsIPrincipal contentStoragePrincipal;
readonly attribute nsIPrincipal contentBlockingAllowListPrincipal;

View File

@ -24,10 +24,6 @@ interface nsIBrowserChild : nsISupports
[notxpcom] void sendRequestFocus(in boolean canFocus, in CallerType aCallerType);
[noscript, notxpcom] void enableDisableCommands(in AString action,
in CommandsArrayRef enabledCommands,
in CommandsArrayRef disabledCommands);
[noscript] void remoteSizeShellTo(in int32_t width, in int32_t height,
in int32_t shellItemWidth, in int32_t shellItemHeight);

View File

@ -2977,13 +2977,6 @@ void BrowserChild::SendRequestFocus(bool aCanFocus, CallerType aCallerType) {
PBrowserChild::SendRequestFocus(aCanFocus, aCallerType);
}
void BrowserChild::EnableDisableCommands(
const nsAString& aAction, nsTArray<nsCString>& aEnabledCommands,
nsTArray<nsCString>& aDisabledCommands) {
PBrowserChild::SendEnableDisableCommands(PromiseFlatString(aAction),
aEnabledCommands, aDisabledCommands);
}
NS_IMETHODIMP
BrowserChild::GetTabId(uint64_t* aId) {
*aId = GetTabId();

View File

@ -61,6 +61,7 @@
#include "nsFrameManager.h"
#include "nsIBaseWindow.h"
#include "nsIBrowser.h"
#include "nsIBrowserController.h"
#include "nsIContent.h"
#include "nsIDocShell.h"
#include "nsIDocShellTreeOwner.h"
@ -2248,16 +2249,17 @@ mozilla::ipc::IPCResult BrowserParent::RecvRequestFocus(
}
mozilla::ipc::IPCResult BrowserParent::RecvEnableDisableCommands(
const nsString& aAction, nsTArray<nsCString>&& aEnabledCommands,
const MaybeDiscarded<BrowsingContext>& aContext, const nsString& aAction,
nsTArray<nsCString>&& aEnabledCommands,
nsTArray<nsCString>&& aDisabledCommands) {
nsCOMPtr<nsIBrowser> browser =
mFrameElement ? mFrameElement->AsBrowser() : nullptr;
bool isRemoteBrowser = false;
if (browser) {
browser->GetIsRemoteBrowser(&isRemoteBrowser);
if (aContext.IsNullOrDiscarded()) {
return IPC_OK();
}
if (isRemoteBrowser) {
browser->EnableDisableCommandsRemoteOnly(aAction, aEnabledCommands,
nsCOMPtr<nsIBrowserController> browserController =
do_QueryActor(u"Controllers", aContext.get_canonical());
if (browserController) {
browserController->EnableDisableCommands(aAction, aEnabledCommands,
aDisabledCommands);
}

View File

@ -429,7 +429,8 @@ class BrowserParent final : public PBrowserParent,
const bool& aIsVertical, const LayoutDeviceIntPoint& aPoint);
mozilla::ipc::IPCResult RecvEnableDisableCommands(
const nsString& aAction, nsTArray<nsCString>&& aEnabledCommands,
const MaybeDiscarded<BrowsingContext>& aContext, const nsString& aAction,
nsTArray<nsCString>&& aEnabledCommands,
nsTArray<nsCString>&& aDisabledCommands);
mozilla::ipc::IPCResult RecvSetCursor(

View File

@ -16,7 +16,7 @@
#include "mozilla/ErrorResult.h"
class nsIGlobalObject;
class nsQueryActor;
class nsQueryActorChild;
namespace mozilla {
namespace dom {

View File

@ -17,7 +17,8 @@
#include "nsWrapperCache.h"
class nsIGlobalObject;
class nsQueryActor;
class nsQueryActorChild;
class nsQueryActorParent;
namespace mozilla {
namespace dom {
@ -87,7 +88,8 @@ class JSWindowActor : public nsISupports, public nsWrapperCache {
void InvokeCallback(CallbackFunction willDestroy);
private:
friend class ::nsQueryActor; // for QueryInterfaceActor
friend class ::nsQueryActorChild; // for QueryInterfaceActor
friend class ::nsQueryActorParent; // for QueryInterfaceActor
nsresult QueryInterfaceActor(const nsIID& aIID, void** aPtr);

View File

@ -409,7 +409,8 @@ parent:
* Indicate, based on the current state, that some commands are enabled and
* some are disabled.
*/
async EnableDisableCommands(nsString action,
async EnableDisableCommands(MaybeDiscardedBrowsingContext bc,
nsString action,
nsCString[] enabledCommands,
nsCString[] disabledCommands);

View File

@ -7,12 +7,18 @@
#include "nsCOMPtr.h"
#include "mozilla/RefPtr.h"
#include "mozilla/dom/CanonicalBrowsingContext.h"
#include "mozilla/dom/JSWindowActorChild.h"
#include "mozilla/dom/JSWindowActorParent.h"
#include "mozilla/dom/WindowGlobalChild.h"
#include "mozilla/dom/WindowGlobalParent.h"
class MOZ_STACK_CLASS nsQueryActor final : public nsCOMPtr_helper {
// This type is used to get an JSWindowActorChild given a window. It
// only has any use within a child process.
class MOZ_STACK_CLASS nsQueryActorChild final : public nsCOMPtr_helper {
public:
nsQueryActor(const nsDependentString aActorName, nsPIDOMWindowOuter* aWindow)
nsQueryActorChild(const nsLiteralString aActorName,
nsPIDOMWindowOuter* aWindow)
: mActorName(aActorName), mWindow(aWindow) {}
virtual nsresult NS_FASTCALL operator()(const nsIID& aIID,
@ -37,13 +43,55 @@ class MOZ_STACK_CLASS nsQueryActor final : public nsCOMPtr_helper {
}
private:
const nsDependentString mActorName;
const nsLiteralString mActorName;
nsPIDOMWindowOuter* mWindow;
};
inline nsQueryActor do_QueryActor(const char16_t* aActorName,
nsPIDOMWindowOuter* aWindow) {
return nsQueryActor(nsDependentString(aActorName), aWindow);
template <size_t length>
inline nsQueryActorChild do_QueryActor(const char16_t (&aActorName)[length],
nsPIDOMWindowOuter* aWindow) {
return nsQueryActorChild(nsLiteralString(aActorName), aWindow);
}
// This type is used to get an actor given a CanonicalBrowsingContext. It
// is only useful in the parent process.
class MOZ_STACK_CLASS nsQueryActorParent final : public nsCOMPtr_helper {
public:
nsQueryActorParent(nsLiteralString aActorName,
mozilla::dom::CanonicalBrowsingContext* aBrowsingContext)
: mActorName(aActorName), mBrowsingContext(aBrowsingContext) {}
virtual nsresult NS_FASTCALL operator()(const nsIID& aIID,
void** aResult) const override {
if (!mBrowsingContext) {
return NS_ERROR_NO_INTERFACE;
}
RefPtr<mozilla::dom::WindowGlobalParent> wgp =
mBrowsingContext->GetCurrentWindowGlobal();
if (!wgp) {
return NS_ERROR_NO_INTERFACE;
}
RefPtr<mozilla::dom::JSWindowActorParent> actor =
wgp->GetActor(mActorName, mozilla::IgnoreErrors());
if (!actor) {
return NS_ERROR_NO_INTERFACE;
}
return actor->QueryInterfaceActor(aIID, aResult);
}
private:
const nsLiteralString mActorName;
mozilla::dom::CanonicalBrowsingContext* mBrowsingContext;
};
template <size_t length>
inline nsQueryActorParent do_QueryActor(
const char16_t (&aActorName)[length],
mozilla::dom::CanonicalBrowsingContext* aBrowsingContext) {
return nsQueryActorParent(nsLiteralString(aActorName), aBrowsingContext);
}
#endif // defined nsQueryActor_h

View File

@ -52,6 +52,7 @@ if CONFIG['MOZ_XUL']:
]
XPIDL_SOURCES += [
'nsIBrowserController.idl',
'nsIController.idl',
'nsIControllers.idl',
]

View File

@ -0,0 +1,22 @@
/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#include "nsISupports.idl"
/**
* This interface is used to accompany the nsIController for a
* <browser> element. It is used to update the commands in the
* parent process when the set of child command have changed.
*/
[scriptable, uuid(5bb3d56b-e733-4a2c-8a53-058123df65e2)]
interface nsIBrowserController : nsISupports
{
// Update the commands for a given action in the parent process.
void enableDisableCommands(in AString action,
in Array<ACString> enabledCommands,
in Array<ACString> disabledCommands);
};

View File

@ -6,22 +6,18 @@
var EXPORTED_SYMBOLS = ["ControllersChild"];
const { ActorChild } = ChromeUtils.import(
"resource://gre/modules/ActorChild.jsm"
);
class ControllersChild extends ActorChild {
class ControllersChild extends JSWindowActorChild {
receiveMessage(message) {
switch (message.name) {
case "ControllerCommands:Do":
if (this.docShell.isCommandEnabled(message.data)) {
if (this.docShell && this.docShell.isCommandEnabled(message.data)) {
this.docShell.doCommand(message.data);
}
break;
case "ControllerCommands:DoWithParams":
var data = message.data;
if (this.docShell.isCommandEnabled(data.cmd)) {
if (this.docShell && this.docShell.isCommandEnabled(data.cmd)) {
var params = Cu.createCommandParams();
for (var name in data.params) {
var value = data.params[name];

View File

@ -1,32 +1,54 @@
// -*- indent-tabs-mode: nil; js-indent-level: 2 -*-
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at http://mozilla.org/MPL/2.0/.
/* vim: set ts=2 sw=2 sts=2 et tw=80: */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
var EXPORTED_SYMBOLS = ["RemoteController"];
"use strict";
class RemoteController {
constructor(browser) {
this._browser = browser;
var EXPORTED_SYMBOLS = ["ControllersParent"];
class ControllersParent extends JSWindowActorParent {
constructor() {
super();
// A map of commands that have had their enabled/disabled state assigned. The
// value of each key will be true if enabled, and false if disabled.
this._supportedCommands = {};
this.supportedCommands = {};
}
// Update the set of enabled and disabled commands.
enableDisableCommands(aAction, aEnabledCommands, aDisabledCommands) {
// Clear the list first
this.supportedCommands = {};
for (let command of aEnabledCommands) {
this.supportedCommands[command] = true;
}
for (let command of aDisabledCommands) {
this.supportedCommands[command] = false;
}
let browser = this.browsingContext.top.embedderElement;
if (browser) {
if (browser.outerBrowser) {
browser = browser.outerBrowser; // handle RDM
}
browser.ownerGlobal.updateCommands(aAction);
}
}
isCommandEnabled(aCommand) {
return this._supportedCommands[aCommand] || false;
return this.supportedCommands[aCommand] || false;
}
supportsCommand(aCommand) {
return aCommand in this._supportedCommands;
return aCommand in this.supportedCommands;
}
doCommand(aCommand) {
this._browser.messageManager.sendAsyncMessage(
"ControllerCommands:Do",
aCommand
);
this.sendAsyncMessage("ControllerCommands:Do", aCommand);
}
getCommandStateWithParams(aCommand, aCommandParams) {
@ -55,12 +77,12 @@ class RemoteController {
},
};
} else {
throw Cr.NS_ERROR_NOT_IMPLEMENTED;
throw Components.Exception(
"Not implemented",
Cr.NS_ERROR_NOT_IMPLEMENTED
);
}
this._browser.messageManager.sendAsyncMessage(
"ControllerCommands:DoWithParams",
cmd
);
this.sendAsyncMessage("ControllerCommands:DoWithParams", cmd);
}
getSupportedCommands() {
@ -68,30 +90,10 @@ class RemoteController {
}
onEvent() {}
// This is intended to be called from the browser binding to update
// the enabled and disabled commands.
enableDisableCommands(aAction, aEnabledCommands, aDisabledCommands) {
// Clear the list first
this._supportedCommands = {};
for (let command of aEnabledCommands) {
this._supportedCommands[command] = true;
}
for (let command of aDisabledCommands) {
this._supportedCommands[command] = false;
}
// Don't update anything if we're not the active element
if (this._browser != this._browser.ownerDocument.activeElement) {
return;
}
this._browser.ownerGlobal.updateCommands(aAction);
}
}
RemoteController.prototype.QueryInterface = ChromeUtils.generateQI([
ControllersParent.prototype.QueryInterface = ChromeUtils.generateQI([
Ci.nsIBrowserController,
Ci.nsIController,
Ci.nsICommandController,
]);

View File

@ -33,6 +33,7 @@ FINAL_TARGET_FILES.actors += [
'BrowserElementChild.jsm',
'BrowserElementParent.jsm',
'ControllersChild.jsm',
'ControllersParent.jsm',
'DateTimePickerChild.jsm',
'DateTimePickerParent.jsm',
'ExtFindChild.jsm',

View File

@ -321,8 +321,6 @@
this._lastSearchString = null;
this._controller = null;
this._remoteWebNavigation = null;
this._remoteWebProgress = null;
@ -1237,12 +1235,6 @@
true
);
}
const { RemoteController } = ChromeUtils.import(
"resource://gre/modules/RemoteController.jsm"
);
this._controller = new RemoteController(this);
this.controllers.appendController(this._controller);
}
try {
@ -1318,17 +1310,6 @@
}
}
// All controllers are released upon browser element removal, but not
// when destruction is triggered before element removal in tabbrowser.
// Release the controller in the latter case.
if (this._controller && this.controllers.getControllerCount()) {
try {
this.controllers.removeController(this._controller);
} catch (ex) {
Cu.reportError(ex);
}
}
this.resetFields();
if (!this.mInitialized) {
@ -1364,20 +1345,6 @@
}
}
enableDisableCommandsRemoteOnly(
aAction,
aEnabledCommands,
aDisabledCommands
) {
if (this._controller) {
this._controller.enableDisableCommands(
aAction,
aEnabledCommands,
aDisabledCommands
);
}
}
updateSecurityUIForSecurityChange(aSecurityInfo, aState, aIsSecureContext) {
if (this.isRemoteBrowser && this.messageManager) {
// Invoking this getter triggers the generation of the underlying object,

View File

@ -132,6 +132,17 @@ let ACTORS = {
allFrames: true,
},
Controllers: {
parent: {
moduleURI: "resource://gre/actors/ControllersParent.jsm",
},
child: {
moduleURI: "resource://gre/actors/ControllersChild.jsm",
},
allFrames: true,
},
DateTimePicker: {
parent: {
moduleURI: "resource://gre/actors/DateTimePickerParent.jsm",
@ -508,13 +519,6 @@ let ACTORS = {
* sub-frames, it must use "allFrames".
*/
let LEGACY_ACTORS = {
Controllers: {
child: {
module: "resource://gre/actors/ControllersChild.jsm",
messages: ["ControllerCommands:Do", "ControllerCommands:DoWithParams"],
},
},
ManifestMessages: {
child: {
module: "resource://gre/modules/ManifestMessagesChild.jsm",

View File

@ -114,9 +114,6 @@ with Files('PrivateBrowsingUtils.jsm'):
with Files('Promise*.jsm'):
BUG_COMPONENT = ('Toolkit', 'Async Tooling')
with Files('RemoteController.jsm'):
BUG_COMPONENT = ('Core', 'Widget')
with Files('RemoteSecurityUI.jsm'):
BUG_COMPONENT = ('Firefox', 'Tabbed Browser')
@ -208,7 +205,6 @@ EXTRA_JS_MODULES += [
'Promise.jsm',
'PromiseMessage.jsm',
'PromiseUtils.jsm',
'RemoteController.jsm',
'RemoteSecurityUI.jsm',
'RemoteWebProgress.jsm',
'ResetProfile.jsm',