From 3d1ba6b08c82fd535e30e456043fd384ab6f8f14 Mon Sep 17 00:00:00 2001 From: Mike Conley Date: Thu, 6 Jun 2019 20:32:30 +0000 Subject: [PATCH] Bug 1505909 - Stop sending a CPOW for the target element for context menu messages. r=MattN This introduces a new toolkit module, ContentDOMReference, which can generate identifiers for DOM elements that can be safely passed across the process boundary without having to use the CPOW infrastructure. The Password Manager code seemed to be the only thing using the original CPOW, so this has been updated to use the ContentDOMReference identifier instead. Differential Revision: https://phabricator.services.mozilla.com/D32758 --HG-- extra : moz-landing-system : lando --- browser/actors/ContextMenuChild.jsm | 10 +- browser/base/content/content.js | 2 - browser/base/content/nsContextMenu.js | 13 +- .../passwordmgr/LoginManagerContent.jsm | 19 ++- .../passwordmgr/LoginManagerContextMenu.jsm | 16 +- .../passwordmgr/LoginManagerParent.jsm | 6 +- toolkit/modules/ContentDOMReference.jsm | 141 ++++++++++++++++++ toolkit/modules/moz.build | 1 + 8 files changed, 176 insertions(+), 32 deletions(-) create mode 100644 toolkit/modules/ContentDOMReference.jsm diff --git a/browser/actors/ContextMenuChild.jsm b/browser/actors/ContextMenuChild.jsm index 24ba38824274..48d0b7b6e138 100644 --- a/browser/actors/ContextMenuChild.jsm +++ b/browser/actors/ContextMenuChild.jsm @@ -24,6 +24,7 @@ XPCOMUtils.defineLazyModuleGetters(this, { WebNavigationFrames: "resource://gre/modules/WebNavigationFrames.jsm", PrivateBrowsingUtils: "resource://gre/modules/PrivateBrowsingUtils.jsm", InlineSpellCheckerContent: "resource://gre/modules/InlineSpellCheckerContent.jsm", + ContentDOMReference: "resource://gre/modules/ContentDOMReference.jsm", }); XPCOMUtils.defineLazyGetter(this, "PageMenuChild", () => { @@ -531,8 +532,8 @@ class ContextMenuChild extends ActorChild { let referrerInfo = Cc["@mozilla.org/referrer-info;1"].createInstance(Ci.nsIReferrerInfo); referrerInfo.initWithNode(context.onLink ? context.link : aEvent.composedTarget); - let targetAsCPOW = context.target; - if (targetAsCPOW) { + let target = context.target; + if (target) { this._cleanContext(); } @@ -594,9 +595,7 @@ class ContextMenuChild extends ActorChild { aEvent.preventDefault(); aEvent.stopPropagation(); - this.mm.sendAsyncMessage("contextmenu", data, { - targetAsCPOW, - }); + this.mm.sendAsyncMessage("contextmenu", data); } /** @@ -757,6 +756,7 @@ class ContextMenuChild extends ActorChild { // Remember the node and its owner document that was clicked // This may be modifed before sending to nsContextMenu context.target = node; + context.targetIdentifier = ContentDOMReference.get(node); context.principal = context.target.ownerDocument.nodePrincipal; context.csp = E10SUtils.serializeCSP(context.target.ownerDocument.csp); diff --git a/browser/base/content/content.js b/browser/base/content/content.js index 346b7ef20955..5d8dca23f52a 100644 --- a/browser/base/content/content.js +++ b/browser/base/content/content.js @@ -18,7 +18,6 @@ XPCOMUtils.defineLazyModuleGetters(this, { ContentMetaHandler: "resource:///modules/ContentMetaHandler.jsm", LoginFormFactory: "resource://gre/modules/LoginFormFactory.jsm", InsecurePasswordUtils: "resource://gre/modules/InsecurePasswordUtils.jsm", - ContextMenuChild: "resource:///actors/ContextMenuChild.jsm", }); XPCOMUtils.defineLazyGetter(this, "LoginManagerContent", () => { @@ -31,7 +30,6 @@ XPCOMUtils.defineLazyGetter(this, "LoginManagerContent", () => { // NOTE: Much of this logic is duplicated in BrowserCLH.js for Android. addMessageListener("PasswordManager:fillForm", function(message) { // intercept if ContextMenu.jsm had sent a plain object for remote targets - message.objects.inputElement = ContextMenuChild.getTarget(global, message, "inputElement"); LoginManagerContent.receiveMessage(message, content); }); diff --git a/browser/base/content/nsContextMenu.js b/browser/base/content/nsContextMenu.js index 8c98c483bfa0..149368dac2a8 100644 --- a/browser/base/content/nsContextMenu.js +++ b/browser/base/content/nsContextMenu.js @@ -33,13 +33,6 @@ function openContextMenu(aMessage) { let spellInfo = data.spellInfo; let frameReferrerInfo = data.frameReferrerInfo; - // ContextMenu.jsm sends us the target as a CPOW only so that - // we can send that CPOW back down to the content process and - // have it resolve to a DOM node. The parent should not attempt - // to access any properties on this CPOW (in fact, doing so - // will throw an exception). - data.context.targetAsCPOW = aMessage.objects.targetAsCPOW; - if (spellInfo) { spellInfo.target = aMessage.target.messageManager; } @@ -220,7 +213,7 @@ nsContextMenu.prototype = { this.onVideo = context.onVideo; this.target = context.target; - this.targetAsCPOW = context.targetAsCPOW; + this.targetIdentifier = context.targetIdentifier; this.principal = context.principal; this.frameOuterWindowID = context.frameOuterWindowID; @@ -730,7 +723,9 @@ nsContextMenu.prototype = { return; } let documentURI = gContextMenuContentData.documentURIObject; - let fragment = LoginManagerContextMenu.addLoginsToMenu(this.targetAsCPOW, this.browser, documentURI); + let fragment = LoginManagerContextMenu.addLoginsToMenu(this.targetIdentifier, + this.browser, + documentURI); this.showItem("fill-login-no-logins", !fragment); diff --git a/toolkit/components/passwordmgr/LoginManagerContent.jsm b/toolkit/components/passwordmgr/LoginManagerContent.jsm index 1cd6c7b062bf..c7761f15c0b0 100644 --- a/toolkit/components/passwordmgr/LoginManagerContent.jsm +++ b/toolkit/components/passwordmgr/LoginManagerContent.jsm @@ -34,6 +34,8 @@ ChromeUtils.defineModuleGetter(this, "LoginHelper", "resource://gre/modules/LoginHelper.jsm"); ChromeUtils.defineModuleGetter(this, "InsecurePasswordUtils", "resource://gre/modules/InsecurePasswordUtils.jsm"); +ChromeUtils.defineModuleGetter(this, "ContentDOMReference", + "resource://gre/modules/ContentDOMReference.jsm"); XPCOMUtils.defineLazyServiceGetter(this, "gNetUtil", "@mozilla.org/network/util;1", @@ -304,7 +306,7 @@ this.LoginManagerContent = { loginFormOrigin: msg.data.loginFormOrigin, loginsFound: LoginHelper.vanillaObjectsToLogins(msg.data.logins), recipes: msg.data.recipes, - inputElement: msg.objects.inputElement, + inputElementIdentifier: msg.data.inputElementIdentifier, }); return; } @@ -684,15 +686,22 @@ this.LoginManagerContent = { * from the origin of the form used for the fill. * recipes: * Fill recipes transmitted together with the original message. - * inputElement: - * Username or password input element from the form we want to fill. + * inputElementIdentifier: + * An identifier generated for the input element via ContentDOMReference. * } */ - fillForm({ topDocument, loginFormOrigin, loginsFound, recipes, inputElement }) { - if (!inputElement) { + fillForm({ topDocument, loginFormOrigin, loginsFound, recipes, inputElementIdentifier }) { + if (!inputElementIdentifier) { log("fillForm: No input element specified"); return; } + + let inputElement = ContentDOMReference.resolve(inputElementIdentifier); + if (!inputElement) { + log("fillForm: Could not resolve inputElementIdentifier to a living element."); + return; + } + if (LoginHelper.getLoginOrigin(topDocument.documentURI) != loginFormOrigin) { if (!inputElement || LoginHelper.getLoginOrigin(inputElement.ownerDocument.documentURI) != loginFormOrigin) { diff --git a/toolkit/components/passwordmgr/LoginManagerContextMenu.jsm b/toolkit/components/passwordmgr/LoginManagerContextMenu.jsm index 97c2f035e070..24a241441f6d 100644 --- a/toolkit/components/passwordmgr/LoginManagerContextMenu.jsm +++ b/toolkit/components/passwordmgr/LoginManagerContextMenu.jsm @@ -21,8 +21,8 @@ this.LoginManagerContextMenu = { /** * Look for login items and add them to the contextual menu. * - * @param {HTMLInputElement} inputElement - * The target input element of the context menu click. + * @param {Object} inputElementIdentifier + * An identifier generated for the input element via ContentDOMReference. * @param {xul:browser} browser * The browser for the document the context menu was open on. * @param {nsIURI} documentURI @@ -31,7 +31,7 @@ this.LoginManagerContextMenu = { * when subframes are involved. * @returns {DocumentFragment} a document fragment with all the login items. */ - addLoginsToMenu(inputElement, browser, documentURI) { + addLoginsToMenu(inputElementIdentifier, browser, documentURI) { let foundLogins = this._findLogins(documentURI); if (!foundLogins.length) { @@ -58,7 +58,7 @@ this.LoginManagerContextMenu = { // login is bound so we can keep the reference to each object. item.addEventListener("command", function(login, event) { - this._fillTargetField(login, inputElement, browser, documentURI); + this._fillTargetField(login, inputElementIdentifier, browser, documentURI); }.bind(this, login)); fragment.appendChild(item); @@ -149,8 +149,8 @@ this.LoginManagerContextMenu = { /** * @param {nsILoginInfo} login * The login we want to fill the form with. - * @param {Element} inputElement - * The target input element we want to fill. + * @param {Object} inputElementIdentifier + * An identifier generated for the input element via ContentDOMReference. * @param {xul:browser} browser * The target tab browser. * @param {nsIURI} documentURI @@ -158,12 +158,12 @@ this.LoginManagerContextMenu = { * This isn't the same as the browser's top-level * document URI when subframes are involved. */ - _fillTargetField(login, inputElement, browser, documentURI) { + _fillTargetField(login, inputElementIdentifier, browser, documentURI) { LoginManagerParent.fillForm({ browser, + inputElementIdentifier, loginFormOrigin: documentURI.displayPrePath, login, - inputElement, }).catch(Cu.reportError); }, diff --git a/toolkit/components/passwordmgr/LoginManagerParent.jsm b/toolkit/components/passwordmgr/LoginManagerParent.jsm index 6438a2c23d41..2a3ad06c395a 100644 --- a/toolkit/components/passwordmgr/LoginManagerParent.jsm +++ b/toolkit/components/passwordmgr/LoginManagerParent.jsm @@ -168,7 +168,7 @@ this.LoginManagerParent = { * Trigger a login form fill and send relevant data (e.g. logins and recipes) * to the child process (LoginManagerContent). */ - async fillForm({ browser, loginFormOrigin, login, inputElement }) { + async fillForm({ browser, loginFormOrigin, login, inputElementIdentifier }) { let recipes = []; if (loginFormOrigin) { let formHost; @@ -185,12 +185,12 @@ this.LoginManagerParent = { // doesn't support structured cloning. let jsLogins = [LoginHelper.loginToVanillaObject(login)]; - let objects = inputElement ? {inputElement} : null; browser.messageManager.sendAsyncMessage("PasswordManager:fillForm", { + inputElementIdentifier, loginFormOrigin, logins: jsLogins, recipes, - }, objects); + }); }, /** diff --git a/toolkit/modules/ContentDOMReference.jsm b/toolkit/modules/ContentDOMReference.jsm new file mode 100644 index 000000000000..b46c91b0f6be --- /dev/null +++ b/toolkit/modules/ContentDOMReference.jsm @@ -0,0 +1,141 @@ +/* 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/. */ + +/** + * This module holds weak references to DOM elements that exist within the + * current content process, and converts them to a unique identifier that can be + * passed between processes. The identifer, if received by the same content process + * that issued it, can then be converted back into the DOM element (presuming the + * element hasn't had all of its other references dropped). + * + * The hope is that this module can eliminate the need for passing CPOW references + * between processes during runtime. + */ + +var EXPORTED_SYMBOLS = ["ContentDOMReference"]; + +const {XPCOMUtils} = ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm"); + +XPCOMUtils.defineLazyServiceGetter(this, "gUUIDGenerator", + "@mozilla.org/uuid-generator;1", + "nsIUUIDGenerator"); + +/** + * An identifier generated by ContentDOMReference is a unique pair of BrowsingContext + * ID and a UUID. gRegistry maps BrowsingContext's to an object with the following + * properties: + * + * UUIDToElement: + * A Map of UUID strings to WeakReference's to the elements they refer to. + * + * elementToUUID: + * A WeakMap from a DOM element to a UUID that refers to it. + */ +var gRegistry = new WeakMap(); + +var ContentDOMReference = { + /** + * Generate and return an identifier for a given DOM element. + * + * @param {Element} element The DOM element to generate the identifier for. + * @return {Object} The identifier for the DOM element that can be passed between + * processes as a message. + */ + get(element) { + if (!element) { + throw new Error("Can't create a ContentDOMReference identifier for " + + "non-existant nodes."); + } + + let browsingContext = element.ownerGlobal.getWindowGlobalChild().browsingContext; + let mappings = gRegistry.get(browsingContext); + if (!mappings) { + mappings = { + UUIDToElement: new Map(), + elementToUUID: new WeakMap(), + }; + gRegistry.set(browsingContext, mappings); + } + + let uuid = mappings.elementToUUID.get(element); + if (uuid) { + // We already had this element registered, so return the pre-existing UUID. + return { browsingContextId: browsingContext.id, uuid }; + } + + // We must be registering a new element at this point. + uuid = gUUIDGenerator.generateUUID().toString(); + mappings.elementToUUID.set(element, uuid); + mappings.UUIDToElement.set(uuid, Cu.getWeakReference(element)); + + return { browsingContextId: browsingContext.id, uuid }; + }, + + /** + * Resolves an identifier back into the DOM Element that it was generated from. + * + * @param {Object} The identifier generated via ContentDOMReference.get for a + * DOM element. + * @return {Element} The DOM element that the identifier was generated for, or + * null if the element does not still exist. + */ + resolve(identifier) { + let browsingContext = BrowsingContext.get(identifier.browsingContextId); + let uuid = identifier.uuid; + return this._resolveUUIDToElement(browsingContext, uuid); + }, + + /** + * Removes an identifier from the registry so that subsequent attempts + * to resolve it will result in null. This is generally a good idea to avoid + * identifiers lying around taking up space (identifiers don't keep the + * DOM element alive, but the weak pointers themselves consume memory, and + * that's what we reclaim when revoking). + * + * @param {Object} The identifier to revoke, issued by ContentDOMReference.get for + * a DOM element. + */ + revoke(identifier) { + let browsingContext = BrowsingContext.get(identifier.browsingContextId); + let uuid = identifier.uuid; + + let mappings = gRegistry.get(browsingContext); + if (!mappings) { + return; + } + + let element = this._resolveUUIDToElement(browsingContext, uuid); + if (element) { + mappings.elementToUUID.delete(element); + } + + mappings.UUIDToElement.delete(uuid); + }, + + /** + * Private helper function that resolves a BrowsingContext and UUID (the + * pair that makes up an identifier) to a DOM element. + * + * @param {BrowsingContext} browsingContext The BrowsingContext that was hosting + * the DOM element at the time that the identifier was generated. + * @param {String} uuid The UUID generated for the DOM element. + * + * @return {Element} The DOM element that the identifier was generated for, or + * null if the element does not still exist. + */ + _resolveUUIDToElement(browsingContext, uuid) { + let mappings = gRegistry.get(browsingContext); + if (!mappings) { + return null; + } + + let weakReference = mappings.UUIDToElement.get(uuid); + if (!weakReference) { + return null; + } + + return weakReference.get(); + }, +}; diff --git a/toolkit/modules/moz.build b/toolkit/modules/moz.build index 272cd1c70170..53f1c2806c96 100644 --- a/toolkit/modules/moz.build +++ b/toolkit/modules/moz.build @@ -175,6 +175,7 @@ EXTRA_JS_MODULES += [ 'CharsetMenu.jsm', 'Color.jsm', 'Console.jsm', + 'ContentDOMReference.jsm', 'CreditCard.jsm', 'css-selector.js', 'DateTimePickerPanel.jsm',