Bug 1266836 - Part 1 - Fix password manager handling of popup windows in e10s. r=MattN draft
authorJohann Hofmann <jhofmann@mozilla.com>
Tue, 30 Aug 2016 15:47:38 +0200
changeset 407417 3af885f95a96753995af3cb7ae132ef0524bf00f
parent 407237 26e22af660e543ebb69930f082188b69ec756185
child 407418 92be042e1d662242f751e450d910f27ecdc05b22
push id27972
push userbmo:jhofmann@mozilla.com
push dateTue, 30 Aug 2016 14:04:51 +0000
reviewersMattN
bugs1266836
milestone51.0a1
Bug 1266836 - Part 1 - Fix password manager handling of popup windows in e10s. r=MattN MozReview-Commit-ID: OVsgkHhyg9
dom/ipc/TabParent.cpp
mobile/android/components/LoginManagerPrompter.js
toolkit/components/passwordmgr/LoginManagerParent.jsm
toolkit/components/passwordmgr/nsILoginManagerPrompter.idl
toolkit/components/passwordmgr/nsLoginManagerPrompter.js
toolkit/components/passwordmgr/test/browser/browser.ini
toolkit/mozapps/extensions/internal/XPIProvider.jsm
--- a/dom/ipc/TabParent.cpp
+++ b/dom/ipc/TabParent.cpp
@@ -2549,17 +2549,17 @@ TabParent::GetAuthPrompt(uint32_t aPromp
   // of the dialogs works as it should when using tabs.
   nsCOMPtr<nsISupports> prompt;
   rv = wwatch->GetPrompt(window, iid, getter_AddRefs(prompt));
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsCOMPtr<nsILoginManagerPrompter> prompter = do_QueryInterface(prompt);
   if (prompter) {
     nsCOMPtr<nsIDOMElement> browser = do_QueryInterface(mFrameElement);
-    prompter->SetE10sData(browser, nullptr);
+    prompter->SetBrowser(browser);
   }
 
   *aResult = prompt.forget().take();
   return NS_OK;
 }
 
 PColorPickerParent*
 TabParent::AllocPColorPickerParent(const nsString& aTitle,
--- a/mobile/android/components/LoginManagerPrompter.js
+++ b/mobile/android/components/LoginManagerPrompter.js
@@ -100,28 +100,33 @@ LoginManagerPrompter.prototype = {
 
   /* ---------- nsILoginManagerPrompter prompts ---------- */
 
   /*
    * init
    *
    */
   init : function (aWindow, aFactory) {
-    this._window = aWindow;
+    this._chromeWindow = this._getChromeWindow(aWindow).wrappedJSObject;
     this._factory = aFactory || null;
+    this._browser = null;
 
     var prefBranch = Services.prefs.getBranch("signon.");
     this._debug = prefBranch.getBoolPref("debug");
     this.log("===== initialized =====");
   },
 
-  setE10sData : function (aBrowser, aOpener) {
-    throw new Error("This should be filled in when Android is multiprocess");
+  set browser(aBrowser) {
+    this._browser = aBrowser;
   },
 
+  // setting this attribute is ignored because Android does not consider
+  // opener windows when displaying login notifications
+  set opener(aOpener) { },
+
   /*
    * promptToSavePassword
    *
    */
   promptToSavePassword : function (aLogin) {
     this._showSaveLoginNotification(aLogin);
       Services.telemetry.getHistogramById("PWMGR_PROMPT_REMEMBER_ACTION").add(PROMPT_DISPLAYED);
   },
@@ -135,20 +140,17 @@ LoginManagerPrompter.prototype = {
    * @param aButtons
    *        Buttons to display with the doorhanger
    * @param aUsername
    *        Username string used in creating a doorhanger action
    * @param aPassword
    *        Password string used in creating a doorhanger action
    */
   _showLoginNotification : function (aBody, aButtons, aUsername, aPassword) {
-    let notifyWin = this._window.top;
-    let chromeWin = this._getChromeWindow(notifyWin).wrappedJSObject;
-    let browser = chromeWin.BrowserApp.getBrowserForWindow(notifyWin);
-    let tabID = chromeWin.BrowserApp.getTabForBrowser(browser).id;
+    let tabID = this._chromeWindow.BrowserApp.getTabForBrowser(this._browser).id;
 
     let actionText = {
       text: aUsername,
       type: "EDIT",
       bundle: { username: aUsername,
       password: aPassword }
     };
 
@@ -329,34 +331,35 @@ LoginManagerPrompter.prototype = {
   },
 
   /*
    * _getChromeWindow
    *
    * Given a content DOM window, returns the chrome window it's in.
    */
   _getChromeWindow: function (aWindow) {
+    if (aWindow instanceof Ci.nsIDOMChromeWindow)
+      return aWindow;
     var chromeWin = aWindow.QueryInterface(Ci.nsIInterfaceRequestor)
       .getInterface(Ci.nsIWebNavigation)
       .QueryInterface(Ci.nsIDocShell)
       .chromeEventHandler.ownerDocument.defaultView;
     return chromeWin;
   },
 
   /*
    * _getNativeWindow
    *
    * Returns the NativeWindow to this prompter, or null if there isn't
    * a NativeWindow available (w/ error sent to logcat).
    */
   _getNativeWindow : function () {
     let nativeWindow = null;
     try {
-      let notifyWin = this._window.top;
-      let chromeWin = this._getChromeWindow(notifyWin).wrappedJSObject;
+      let chromeWin = this._chromeWindow;
       if (chromeWin.NativeWindow) {
         nativeWindow = chromeWin.NativeWindow;
       } else {
         Cu.reportError("NativeWindow not available on window");
       }
 
     } catch (e) {
       // If any errors happen, just assume no native window helper.
--- a/toolkit/components/passwordmgr/LoginManagerParent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerParent.jsm
@@ -287,26 +287,19 @@ var LoginManagerParent = {
 
   onFormSubmit: function(hostname, formSubmitURL,
                          usernameField, newPasswordField,
                          oldPasswordField, opener,
                          target) {
     function getPrompter() {
       var prompterSvc = Cc["@mozilla.org/login-manager/prompter;1"].
                         createInstance(Ci.nsILoginManagerPrompter);
-      // XXX For E10S, we don't want to use the browser's contentWindow
-      // because it's in another process, so we use our chrome window as
-      // the window parent (the content process is responsible for
-      // making sure that its window is not in private browsing mode).
-      // In the same-process case, we can simply use the content window.
-      prompterSvc.init(target.isRemoteBrowser ?
-                          target.ownerDocument.defaultView :
-                          target.contentWindow);
-      if (target.isRemoteBrowser)
-        prompterSvc.setE10sData(target, opener);
+      prompterSvc.init(target.ownerDocument.defaultView);
+      prompterSvc.browser = target;
+      prompterSvc.opener = opener;
       return prompterSvc;
     }
 
     function recordLoginUse(login) {
       // Update the lastUsed timestamp and increment the use count.
       let propBag = Cc["@mozilla.org/hash-property-bag;1"].
                     createInstance(Ci.nsIWritablePropertyBag);
       propBag.setProperty("timeLastUsed", Date.now());
--- a/toolkit/components/passwordmgr/nsILoginManagerPrompter.idl
+++ b/toolkit/components/passwordmgr/nsILoginManagerPrompter.idl
@@ -14,28 +14,36 @@ interface nsILoginManagerPrompter : nsIS
   /**
    * Initialize the prompter. Must be called before using other interfaces.
    *
    * @param aWindow
    *        The in which the user is doing some login-related action that's
    *        resulting in a need to prompt them for something. The prompt
    *        will be associated with this window (or, if a notification bar
    *        is being used, topmost opener in some cases).
+   *
+   *        If this window is a content window, the corresponding window and browser
+   *        elements will be calculated. If this window is a chrome window, the
+   *        corresponding browser element needs to be set using setBrowser.
    */
   void init(in nsIDOMWindow aWindow);
 
   /**
-   * If the caller knows which browser this prompter is being created for,
-   * they can call this function to avoid having to calculate it from the
-   * window passed to init.
-   *
-   * @param aBrowser the <browser> to use for this prompter.
-   * @param aOpener the opener to use for this prompter.
+   * The browser this prompter is being created for.
+   * This is required if the init function received a chrome window as argument.
    */
-  void setE10sData(in nsIDOMElement aBrowser, in nsIDOMWindow aOpener);
+  attribute nsIDOMElement browser;
+
+  /**
+   * The opener that was used to open the window passed to init.
+   * The opener can be used to determine in which window the prompt
+   * should be shown. Must be a content window that is not a frame window,
+   * make sure to pass the top window using e.g. window.top.
+   */
+  attribute nsIDOMWindow opener;
 
   /**
    * Ask the user if they want to save a login (Yes, Never, Not Now)
    *
    * @param aLogin
    *        The login to be saved.
    */
   void promptToSavePassword(in nsILoginInfo aLogin);
--- a/toolkit/components/passwordmgr/nsLoginManagerPrompter.js
+++ b/toolkit/components/passwordmgr/nsLoginManagerPrompter.js
@@ -674,30 +674,37 @@ LoginManagerPrompter.prototype = {
   },
 
 
 
 
   /* ---------- nsILoginManagerPrompter prompts ---------- */
 
 
-
   init : function (aWindow, aFactory) {
-    this._window = aWindow;
+    if (aWindow instanceof Ci.nsIDOMChromeWindow) {
+      this._window = aWindow;
+      // needs to be set explicitly using setBrowser
+      this._browser = null;
+    } else {
+      let {win, browser} = this._getChromeWindow(aWindow);
+      this._window = win;
+      this._browser = browser;
+    }
+    this._opener = null;
     this._factory = aFactory || null;
-    this._browser = null;
-    this._opener = null;
 
     this.log("===== initialized =====");
   },
 
-  setE10sData : function (aBrowser, aOpener) {
-    if (!(this._window instanceof Ci.nsIDOMChromeWindow))
-      throw new Error("Unexpected call");
+  set browser(aBrowser) {
     this._browser = aBrowser;
+  },
+
+  set opener(aOpener) {
     this._opener = aOpener;
   },
 
   promptToSavePassword : function (aLogin) {
     this.log("promptToSavePassword");
     var notifyObj = this._getPopupNote() || this._getNotifyBox();
     if (notifyObj)
       this._showSaveLoginNotification(notifyObj, aLogin);
@@ -1326,88 +1333,54 @@ LoginManagerPrompter.prototype = {
       // value as timeLastUsed.
       propBag.setProperty("timePasswordChanged", now);
     }
     propBag.setProperty("timeLastUsed", now);
     propBag.setProperty("timesUsedIncrement", 1);
     this._pwmgr.modifyLogin(login, propBag);
   },
 
-
   /**
-   * Given a content DOM window, returns the chrome window it's in.
+   * Given a content DOM window, returns the chrome window and browser it's in.
    */
   _getChromeWindow: function (aWindow) {
-    // In e10s, aWindow may already be a chrome window.
-    if (aWindow instanceof Ci.nsIDOMChromeWindow)
-      return aWindow;
-    var chromeWin = aWindow.QueryInterface(Ci.nsIInterfaceRequestor)
-                           .getInterface(Ci.nsIWebNavigation)
-                           .QueryInterface(Ci.nsIDocShell)
-                           .chromeEventHandler.ownerDocument.defaultView;
-    return chromeWin;
+    let windows = Services.wm.getEnumerator(null);
+    while (windows.hasMoreElements()) {
+      let win = windows.getNext();
+      let browser = win.gBrowser.getBrowserForContentWindow(aWindow);
+      if (browser) {
+        return { win, browser };
+      }
+    }
+    return null;
   },
 
-
   _getNotifyWindow: function () {
-
     try {
       // Get topmost window, in case we're in a frame.
-      var notifyWin = this._window.top;
-      var isE10s = (notifyWin instanceof Ci.nsIDOMChromeWindow);
-      var useOpener = false;
+      let win = this._window;
+      let browser = this._browser;
 
       // Some sites pop up a temporary login window, which disappears
       // upon submission of credentials. We want to put the notification
       // bar in the opener window if this seems to be happening.
-      if (notifyWin.opener) {
-        var chromeDoc = this._getChromeWindow(notifyWin).
-                             document.documentElement;
-
-        var hasHistory;
-        if (isE10s) {
-          if (!this._browser)
-            throw new Error("Expected a browser in e10s");
-          hasHistory = this._browser.canGoBack;
-        } else {
-          var webnav = notifyWin.
-                       QueryInterface(Ci.nsIInterfaceRequestor).
-                       getInterface(Ci.nsIWebNavigation);
-          hasHistory = webnav.sessionHistory.count > 1;
-        }
+      if (this._opener) {
+        let chromeDoc = win.document.documentElement;
 
         // Check to see if the current window was opened with chrome
         // disabled, and if so use the opener window. But if the window
         // has been used to visit other pages (ie, has a history),
         // assume it'll stick around and *don't* use the opener.
-        if (chromeDoc.getAttribute("chromehidden") && !hasHistory) {
+        if (chromeDoc.getAttribute("chromehidden") && !browser.canGoBack) {
           this.log("Using opener window for notification bar.");
-          notifyWin = notifyWin.opener;
-          useOpener = true;
+          return this._getChromeWindow(this._opener);
         }
       }
 
-      let browser;
-      if (useOpener && this._opener && isE10s) {
-        // In e10s, we have to reconstruct the opener browser from
-        // the CPOW passed in the message (and then passed to us in
-        // setE10sData).
-        // NB: notifyWin is now the chrome window for the opening
-        // window.
-
-        browser = notifyWin.gBrowser.getBrowserForContentWindow(this._opener);
-      } else if (isE10s) {
-        browser = this._browser;
-      } else {
-        var chromeWin = this._getChromeWindow(notifyWin).wrappedJSObject;
-        browser = chromeWin.gBrowser
-                           .getBrowserForDocument(notifyWin.top.document);
-      }
-
-      return { notifyWin: notifyWin, browser: browser };
+      return { win, browser };
 
     } catch (e) {
       // If any errors happen, just assume no notification box.
       this.log("Unable to get notify window: " + e.fileName + ":" + e.lineNumber + ": " + e.message);
       return null;
     }
   },
 
@@ -1415,46 +1388,40 @@ LoginManagerPrompter.prototype = {
   /**
    * Returns the popup notification to this prompter,
    * or null if there isn't one available.
    */
   _getPopupNote : function () {
     let popupNote = null;
 
     try {
-      let { notifyWin } = this._getNotifyWindow();
+      let { win: notifyWin } = this._getNotifyWindow();
 
-      // Get the chrome window for the content window we're using.
       // .wrappedJSObject needed here -- see bug 422974 comment 5.
-      let chromeWin = this._getChromeWindow(notifyWin).wrappedJSObject;
-
-      popupNote = chromeWin.PopupNotifications;
+      popupNote = notifyWin.wrappedJSObject.PopupNotifications;
     } catch (e) {
       this.log("Popup notifications not available on window");
     }
 
     return popupNote;
   },
 
 
   /**
    * Returns the notification box to this prompter, or null if there isn't
    * a notification box available.
    */
   _getNotifyBox : function () {
     let notifyBox = null;
 
     try {
-      let { notifyWin } = this._getNotifyWindow();
+      let { win: notifyWin } = this._getNotifyWindow();
 
-      // Get the chrome window for the content window we're using.
       // .wrappedJSObject needed here -- see bug 422974 comment 5.
-      let chromeWin = this._getChromeWindow(notifyWin).wrappedJSObject;
-
-      notifyBox = chromeWin.getNotificationBox(notifyWin);
+      notifyBox = notifyWin.wrappedJSObject.getNotificationBox(notifyWin);
     } catch (e) {
       this.log("Notification bars not available on window");
     }
 
     return notifyBox;
   },
 
 
--- a/toolkit/components/passwordmgr/test/browser/browser.ini
+++ b/toolkit/components/passwordmgr/test/browser/browser.ini
@@ -25,17 +25,16 @@ support-files =
   subtst_notifications_4.html
   subtst_notifications_5.html
   subtst_notifications_6.html
   subtst_notifications_8.html
   subtst_notifications_9.html
   subtst_notifications_10.html
   subtst_notifications_change_p.html
 [browser_capture_doorhanger_window_open.js]
-skip-if = e10s # Bug 1266836 - Prompt code is broken with popups in e10s
 support-files =
   subtst_notifications_11.html
   subtst_notifications_11_popup.html
 [browser_username_select_dialog.js]
 support-files =
   subtst_notifications_change_p.html
 [browser_DOMFormHasPassword.js]
 [browser_DOMInputPasswordAdded.js]
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -6487,18 +6487,18 @@ AddonInstall.prototype = {
       let win = this.window;
       if (!win && this.browser)
         win = this.browser.ownerDocument.defaultView;
 
       let factory = Cc["@mozilla.org/prompter;1"].
                     getService(Ci.nsIPromptFactory);
       let prompt = factory.getPrompt(win, Ci.nsIAuthPrompt2);
 
-      if (this.browser && this.browser.isRemoteBrowser && prompt instanceof Ci.nsILoginManagerPrompter)
-        prompt.setE10sData(this.browser, null);
+      if (this.browser && prompt instanceof Ci.nsILoginManagerPrompter)
+        prompt.browser = this.browser;
 
       return prompt;
     }
     else if (iid.equals(Ci.nsIChannelEventSink)) {
       return this;
     }
 
     return this.badCertHandler.getInterface(iid);