Bug 1493655 - make nsISecureBrowserUI initialize from a docshell instead of a window, r=keeler,nika
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Wed, 26 Sep 2018 17:48:38 +0000
changeset 438336 d86f6268d191
parent 438335 513ebcc0f395
child 438337 e6f8e822851a
push id69979
push usergijskruitbosch@gmail.com
push dateWed, 26 Sep 2018 17:50:43 +0000
treeherderautoland@d86f6268d191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler, nika
bugs1493655
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1493655 - make nsISecureBrowserUI initialize from a docshell instead of a window, r=keeler,nika This also removes the (afaict, unused) stub implementation from TabParent. The netwerk header inclusions were necessary because those files included TabParent.h and through it, nsISecureBrowserUI, but now TabParent.h no longer does that. Differential Revision: https://phabricator.services.mozilla.com/D6829
browser/components/extensions/ExtensionPopups.jsm
docshell/base/nsDocShell.cpp
dom/base/test/chrome/cpows_parent.xul
dom/browser-element/BrowserElementChildPreload.js
dom/events/test/window_bug617528.xul
dom/ipc/TabParent.cpp
dom/ipc/TabParent.h
netwerk/base/nsISecureBrowserUI.idl
netwerk/protocol/ftp/FTPChannelParent.cpp
netwerk/protocol/http/HttpChannelParent.cpp
security/manager/ssl/nsSecureBrowserUIImpl.cpp
toolkit/components/browser/nsWebBrowser.cpp
toolkit/content/widgets/browser.xml
--- a/browser/components/extensions/ExtensionPopups.jsm
+++ b/browser/components/extensions/ExtensionPopups.jsm
@@ -276,16 +276,22 @@ class BasePopup {
     if (this.extension.remote) {
       readyPromise = promiseEvent(browser, "XULFrameLoaderCreated");
     } else {
       readyPromise = promiseEvent(browser, "load");
     }
 
     stack.appendChild(browser);
     viewNode.appendChild(stack);
+    if (!this.extension.remote) {
+      // FIXME: bug 1494029 - this code used to rely on the browser binding
+      // accessing browser.contentWindow. This is a stopgap to continue doing
+      // that, but we should get rid of it in the long term.
+      browser.contentWindow; // eslint-disable-line no-unused-expressions
+    }
 
     ExtensionParent.apiManager.emit("extension-browser-inserted", browser);
 
     let setupBrowser = browser => {
       let mm = browser.messageManager;
       mm.addMessageListener("DOMTitleChanged", this);
       mm.addMessageListener("Extension:BrowserBackgroundChanged", this);
       mm.addMessageListener("Extension:BrowserContentLoaded", this);
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -2246,17 +2246,16 @@ nsDocShell::GetSecurityUI(nsISecureBrows
 }
 
 NS_IMETHODIMP
 nsDocShell::SetSecurityUI(nsISecureBrowserUI* aSecurityUI)
 {
   MOZ_ASSERT(!mIsBeingDestroyed);
 
   mSecurityUI = aSecurityUI;
-  mSecurityUI->SetDocShell(this);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDocShell::GetLoadURIDelegate(nsILoadURIDelegate** aLoadURIDelegate)
 {
   NS_IF_ADDREF(*aLoadURIDelegate = mLoadURIDelegate);
   return NS_OK;
--- a/dom/base/test/chrome/cpows_parent.xul
+++ b/dom/base/test/chrome/cpows_parent.xul
@@ -249,17 +249,17 @@
         // one.
         let docshell = savedElement.ownerGlobal.docShell;
         ok(docshell, "We should have a docshell here!");
 
         let secureUI = Cc['@mozilla.org/secure_browser_ui;1']
                          .createInstance(Ci.nsISecureBrowserUI);
 
         try {
-          secureUI.setDocShell(docshell);
+          secureUI.init(docshell);
           ok(false, "expected exception passing CPOW to C++");
         } catch (e) {
           is(e.result, Cr.NS_ERROR_XPC_CANT_PASS_CPOW_TO_NATIVE,
              "got exception when passing CPOW to C++");
         }
       }
     }
 
--- a/dom/browser-element/BrowserElementChildPreload.js
+++ b/dom/browser-element/BrowserElementChildPreload.js
@@ -143,17 +143,17 @@ BrowserElementChild.prototype = {
       // TabChild::Init before we run this code which will perform this setup
       // for us.
       docShell.initSessionHistory();
     }
 
     // This is necessary to get security web progress notifications.
     var securityUI = Cc['@mozilla.org/secure_browser_ui;1']
                        .createInstance(Ci.nsISecureBrowserUI);
-    securityUI.init(content);
+    securityUI.init(docShell);
 
     // A cache of the menuitem dom objects keyed by the id we generate
     // and pass to the embedder
     this._ctxHandlers = {};
     // Counter of contextmenu events fired
     this._ctxCounter = 0;
 
     this._shuttingDown = false;
--- a/dom/events/test/window_bug617528.xul
+++ b/dom/events/test/window_bug617528.xul
@@ -1,9 +1,9 @@
 <?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
 
 <window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
         width="640" height="480">
 
   <browser id="browser" type="content" primary="true" flex="1" src="about:blank"
-           disablehistory="true" disablesecurity="true"/>
+           disablehistory="true"/>
 
 </window>
--- a/dom/ipc/TabParent.cpp
+++ b/dom/ipc/TabParent.cpp
@@ -128,17 +128,16 @@ using mozilla::Unused;
 namespace mozilla {
 namespace dom {
 
 TabParent::LayerToTabParentTable* TabParent::sLayerToTabParentTable = nullptr;
 
 NS_IMPL_ISUPPORTS(TabParent,
                   nsITabParent,
                   nsIAuthPromptProvider,
-                  nsISecureBrowserUI,
                   nsISupportsWeakReference)
 
 TabParent::TabParent(nsIContentParent* aManager,
                      const TabId& aTabId,
                      const TabContext& aContext,
                      uint32_t aChromeFlags)
   : TabContext(aContext)
   , mFrameElement(nullptr)
@@ -873,48 +872,16 @@ TabParent::Activate()
 void
 TabParent::Deactivate()
 {
   if (!mIsDestroyed) {
     Unused << Manager()->SendDeactivate(this);
   }
 }
 
-NS_IMETHODIMP
-TabParent::Init(mozIDOMWindowProxy *window)
-{
-  return NS_OK;
-}
-
-NS_IMETHODIMP
-TabParent::GetState(uint32_t *aState)
-{
-  NS_ENSURE_ARG(aState);
-  NS_WARNING("SecurityState not valid here");
-  *aState = 0;
-  return NS_OK;
-}
-
-NS_IMETHODIMP
-TabParent::GetSecInfo(nsITransportSecurityInfo** _result)
-{
-  NS_ENSURE_ARG_POINTER(_result);
-  NS_WARNING("TransportSecurityInfo not valid here");
-  *_result = nullptr;
-  return NS_OK;
-}
-
-NS_IMETHODIMP
-TabParent::SetDocShell(nsIDocShell *aDocShell)
-{
-  NS_ENSURE_ARG(aDocShell);
-  NS_WARNING("No mDocShell member in TabParent so there is no docShell to set");
-  return NS_OK;
-}
-
   a11y::PDocAccessibleParent*
 TabParent::AllocPDocAccessibleParent(PDocAccessibleParent* aParent,
                                      const uint64_t&, const uint32_t&,
                                      const IAccessibleHolder&)
 {
 #ifdef ACCESSIBILITY
   return new a11y::DocAccessibleParent();
 #else
--- a/dom/ipc/TabParent.h
+++ b/dom/ipc/TabParent.h
@@ -20,17 +20,16 @@
 #include "mozilla/layers/CompositorBridgeParent.h"
 #include "mozilla/RefPtr.h"
 #include "mozilla/Move.h"
 #include "nsCOMPtr.h"
 #include "nsIAuthPromptProvider.h"
 #include "nsIBrowserDOMWindow.h"
 #include "nsIDOMEventListener.h"
 #include "nsIKeyEventInPluginCallback.h"
-#include "nsISecureBrowserUI.h"
 #include "nsITabParent.h"
 #include "nsIXULBrowserWindow.h"
 #include "nsRefreshDriver.h"
 #include "nsWeakReference.h"
 #include "Units.h"
 #include "nsIWidget.h"
 
 class nsFrameLoader;
@@ -78,17 +77,16 @@ class DataTransfer;
 namespace ipc {
 class StructuredCloneData;
 } // ipc namespace
 
 class TabParent final : public PBrowserParent
                       , public nsIDOMEventListener
                       , public nsITabParent
                       , public nsIAuthPromptProvider
-                      , public nsISecureBrowserUI
                       , public nsIKeyEventInPluginCallback
                       , public nsSupportsWeakReference
                       , public TabContext
                       , public LiveResizeListener
 {
   typedef mozilla::dom::ClonedMessageData ClonedMessageData;
 
   virtual ~TabParent();
@@ -482,17 +480,16 @@ public:
   DeallocPIndexedDBPermissionRequestParent(
                                     PIndexedDBPermissionRequestParent* aActor)
                                     override;
 
   bool GetGlobalJSObject(JSContext* cx, JSObject** globalp);
 
   NS_DECL_ISUPPORTS
   NS_DECL_NSIAUTHPROMPTPROVIDER
-  NS_DECL_NSISECUREBROWSERUI
 
   void StartPersistence(uint64_t aOuterWindowID,
                         nsIWebBrowserPersistDocumentReceiver* aRecv,
                         ErrorResult& aRv);
 
   bool HandleQueryContentEvent(mozilla::WidgetQueryContentEvent& aEvent);
 
   bool SendPasteTransferable(const IPCDataTransfer& aDataTransfer,
--- a/netwerk/base/nsISecureBrowserUI.idl
+++ b/netwerk/base/nsISecureBrowserUI.idl
@@ -1,25 +1,23 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 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/. */
 
 #include "nsISupports.idl"
 
-interface mozIDOMWindowProxy;
 interface nsIDocShell;
 interface nsITransportSecurityInfo;
 
 [scriptable, uuid(718c662a-f810-4a80-a6c9-0b1810ecade2)]
 interface nsISecureBrowserUI : nsISupports
 {
-    void init(in mozIDOMWindowProxy window);
-    void setDocShell(in nsIDocShell docShell);
+    void init(in nsIDocShell docShell);
 
     readonly attribute unsigned long state;
     readonly attribute nsITransportSecurityInfo secInfo;
 };
 
 %{C++
 #define NS_SECURE_BROWSER_UI_CONTRACTID "@mozilla.org/secure_browser_ui;1"
 %}
--- a/netwerk/protocol/ftp/FTPChannelParent.cpp
+++ b/netwerk/protocol/ftp/FTPChannelParent.cpp
@@ -13,16 +13,17 @@
 #include "nsNetCID.h"
 #include "nsNetUtil.h"
 #include "nsQueryObject.h"
 #include "nsFtpProtocolHandler.h"
 #include "nsIAuthPrompt.h"
 #include "nsIAuthPromptProvider.h"
 #include "nsIEncodedChannel.h"
 #include "nsIHttpChannelInternal.h"
+#include "nsISecureBrowserUI.h"
 #include "nsIForcePendingChannel.h"
 #include "mozilla/ipc/IPCStreamUtils.h"
 #include "mozilla/ipc/URIUtils.h"
 #include "mozilla/Unused.h"
 #include "SerializedLoadContext.h"
 #include "nsIContentPolicy.h"
 #include "mozilla/ipc/BackgroundUtils.h"
 #include "mozilla/LoadInfo.h"
--- a/netwerk/protocol/http/HttpChannelParent.cpp
+++ b/netwerk/protocol/http/HttpChannelParent.cpp
@@ -42,16 +42,17 @@
 #include "nsQueryObject.h"
 #include "mozilla/BasePrincipal.h"
 #include "nsCORSListenerProxy.h"
 #include "nsIIPCSerializableInputStream.h"
 #include "nsIPrompt.h"
 #include "mozilla/net/RedirectChannelRegistrar.h"
 #include "nsIWindowWatcher.h"
 #include "nsIDocument.h"
+#include "nsISecureBrowserUI.h"
 #include "nsStreamUtils.h"
 #include "nsStringStream.h"
 #include "nsIStorageStream.h"
 #include "nsThreadUtils.h"
 #include "nsQueryObject.h"
 #include "nsIURIClassifier.h"
 
 using mozilla::BasePrincipal;
--- a/security/manager/ssl/nsSecureBrowserUIImpl.cpp
+++ b/security/manager/ssl/nsSecureBrowserUIImpl.cpp
@@ -27,39 +27,37 @@ nsSecureBrowserUIImpl::nsSecureBrowserUI
 }
 
 NS_IMPL_ISUPPORTS(nsSecureBrowserUIImpl,
                   nsISecureBrowserUI,
                   nsIWebProgressListener,
                   nsISupportsWeakReference)
 
 NS_IMETHODIMP
-nsSecureBrowserUIImpl::Init(mozIDOMWindowProxy* aWindow)
+nsSecureBrowserUIImpl::Init(nsIDocShell* aDocShell)
 {
   MOZ_ASSERT(NS_IsMainThread());
-  NS_ENSURE_ARG(aWindow);
+  NS_ENSURE_ARG(aDocShell);
+
+  aDocShell->SetSecurityUI(this);
 
-  auto* piwindow = nsPIDOMWindowOuter::From(aWindow);
-  nsIDocShell* docShell = piwindow->GetDocShell();
-
-  // The Docshell will own the SecureBrowserUI object
-  if (!docShell) {
-    return NS_ERROR_FAILURE;
+  // The Docshell will own the SecureBrowserUI object, we keep a weak ref.
+  nsresult rv;
+  mDocShell = do_GetWeakReference(aDocShell, &rv);
+  if (NS_FAILED(rv)) {
+    return rv;
   }
 
-  docShell->SetSecurityUI(this);
-
   // hook up to the webprogress notifications.
-  nsCOMPtr<nsIWebProgress> wp(do_GetInterface(docShell));
+  nsCOMPtr<nsIWebProgress> wp(do_GetInterface(aDocShell));
   if (!wp) {
     return NS_ERROR_FAILURE;
   }
 
   // Save this so we can compare it to the web progress in OnLocationChange.
-  nsresult rv;
   mWebProgress = do_GetWeakReference(wp, &rv);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   return wp->AddProgressListener(this, nsIWebProgress::NOTIFY_LOCATION);
 }
 
@@ -87,26 +85,16 @@ nsSecureBrowserUIImpl::GetSecInfo(nsITra
   NS_ENSURE_ARG_POINTER(result);
 
   *result = mTopLevelSecurityInfo;
   NS_IF_ADDREF(*result);
 
   return NS_OK;
 }
 
-NS_IMETHODIMP
-nsSecureBrowserUIImpl::SetDocShell(nsIDocShell* aDocShell)
-{
-  MOZ_ASSERT(NS_IsMainThread());
-  NS_ENSURE_ARG(aDocShell);
-  nsresult rv;
-  mDocShell = do_GetWeakReference(aDocShell, &rv);
-  return rv;
-}
-
 // Ask the docShell if we've blocked or loaded any mixed or tracking content.
 void
 nsSecureBrowserUIImpl::CheckForBlockedContent()
 {
   nsCOMPtr<nsIDocShell> docShell = do_QueryReferent(mDocShell);
   if (!docShell) {
     return;
   }
--- a/toolkit/components/browser/nsWebBrowser.cpp
+++ b/toolkit/components/browser/nsWebBrowser.cpp
@@ -1186,28 +1186,24 @@ nsWebBrowser::Create()
     rv = EnableGlobalHistory(mShouldEnableHistory);
     NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "EnableGlobalHistory() failed");
   }
 
   NS_ENSURE_SUCCESS(mDocShellAsWin->Create(), NS_ERROR_FAILURE);
 
   // Hook into the OnSecurityChange() notification for lock/unlock icon
   // updates
-  nsCOMPtr<mozIDOMWindowProxy> domWindow;
-  rv = GetContentDOMWindow(getter_AddRefs(domWindow));
-  if (NS_SUCCEEDED(rv)) {
-    // this works because the implementation of nsISecureBrowserUI
-    // (nsSecureBrowserUIImpl) gets a docShell from the domWindow,
-    // and calls docShell->SetSecurityUI(this);
-    nsCOMPtr<nsISecureBrowserUI> securityUI =
-      do_CreateInstance(NS_SECURE_BROWSER_UI_CONTRACTID, &rv);
-    if (NS_SUCCEEDED(rv)) {
-      securityUI->Init(domWindow);
-    }
+  // this works because the implementation of nsISecureBrowserUI
+  // (nsSecureBrowserUIImpl) calls docShell->SetSecurityUI(this);
+  nsCOMPtr<nsISecureBrowserUI> securityUI =
+    do_CreateInstance(NS_SECURE_BROWSER_UI_CONTRACTID, &rv);
+  if (NS_FAILED(rv)) {
+    return rv;
   }
+  securityUI->Init(mDocShell);
 
   mDocShellTreeOwner->AddToWatcher(); // evil twin of Remove in SetDocShell(0)
   mDocShellTreeOwner->AddChromeListeners();
 
   mInitInfo = nullptr;
 
   return NS_OK;
 }
--- a/toolkit/content/widgets/browser.xml
+++ b/toolkit/content/widgets/browser.xml
@@ -909,22 +909,19 @@
               var ptr = Cc["@mozilla.org/supports-interface-pointer;1"]
                           .createInstance(Ci.nsISupportsInterfacePointer);
               ptr.data = this._securityUI;
               return ptr.data.QueryInterface(Ci.nsISecureBrowserUI);
             }
 
             if (!this.docShell.securityUI) {
               const SECUREBROWSERUI_CONTRACTID = "@mozilla.org/secure_browser_ui;1";
-              if (!this.hasAttribute("disablesecurity") &&
-                  SECUREBROWSERUI_CONTRACTID in Cc) {
-                var securityUI = Cc[SECUREBROWSERUI_CONTRACTID]
-                                   .createInstance(Ci.nsISecureBrowserUI);
-                securityUI.init(this.contentWindow);
-              }
+              var securityUI = Cc[SECUREBROWSERUI_CONTRACTID]
+                                 .createInstance(Ci.nsISecureBrowserUI);
+              securityUI.init(this.docShell);
             }
 
             return this.docShell.securityUI;
           ]]>
         </getter>
       </property>
 
       <field name="urlbarChangeTracker">