Bug 1255849. Add some documentation for AutoJSAPI instances that seem to be used just for cxpushing. r=bholley
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 14 Mar 2016 20:47:13 -0400
changeset 288662 9b9dde0705bdcd960e78bbab5083ae7a1543e33b
parent 288661 ba45f827d74b3103f151eb75827713eab8f9368b
child 288663 3149ea08a83f3bd4041671f14316a946ab542f3c
push id73503
push userbzbarsky@mozilla.com
push dateTue, 15 Mar 2016 00:52:06 +0000
treeherdermozilla-inbound@21e5497ebb72 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs1255849
milestone48.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 1255849. Add some documentation for AutoJSAPI instances that seem to be used just for cxpushing. r=bholley
docshell/base/nsDocShell.cpp
dom/base/nsGlobalWindow.cpp
dom/network/TCPServerSocket.cpp
embedding/components/windowwatcher/nsWindowWatcher.cpp
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -13486,16 +13486,21 @@ public:
                    nsIInputStream* aPostDataStream,
                    nsIInputStream* aHeadersDataStream,
                    bool aIsTrusted);
 
   NS_IMETHOD Run()
   {
     nsAutoPopupStatePusher popupStatePusher(mPopupState);
 
+    // We need to set up an AutoJSAPI here for the following reason: When we do
+    // OnLinkClickSync we'll eventually end up in nsGlobalWindow::OpenInternal
+    // which only does popup blocking if !LegacyIsCallerChromeOrNativeCode().
+    // So we need to fake things so that we don't look like native code as far
+    // as LegacyIsCallerNativeCode() is concerned.
     AutoJSAPI jsapi;
     if (mIsTrusted || jsapi.Init(mContent->OwnerDoc()->GetScopeObject())) {
       mHandler->OnLinkClickSync(mContent, mURI,
                                 mTargetSpec.get(), mFileName,
                                 mPostDataStream, mHeadersDataStream,
                                 nullptr, nullptr);
     }
     return NS_OK;
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -11437,16 +11437,20 @@ nsGlobalWindow::OpenInternal(const nsASt
 
   // Popups from apps are never blocked.
   bool isApp = false;
   if (mDoc) {
     isApp = mDoc->NodePrincipal()->GetAppStatus() >=
               nsIPrincipal::APP_STATUS_INSTALLED;
   }
 
+  // XXXbz When this gets fixed to not use LegacyIsCallerNativeCode()
+  // (indirectly) maybe we can nix the AutoJSAPI usage OnLinkClickEvent::Run.
+  // But note that if you change this to GetEntryGlobal(), say, then
+  // OnLinkClickEvent::Run will need a full-blown AutoEntryScript.
   const bool checkForPopup = !nsContentUtils::LegacyIsCallerChromeOrNativeCode() &&
     !isApp && !aDialog && !WindowExists(aName, !aCalledNoScript);
 
   // Note: it's very important that this be an nsXPIDLCString, since we want
   // .get() on it to return nullptr until we write stuff to it.  The window
   // watcher expects a null URL string if there is no URL to load.
   nsXPIDLCString url;
   nsresult rv = NS_OK;
--- a/dom/network/TCPServerSocket.cpp
+++ b/dom/network/TCPServerSocket.cpp
@@ -117,19 +117,16 @@ TCPServerSocket::Close()
   if (mServerSocket) {
     mServerSocket->Close();
   }
 }
 
 void
 TCPServerSocket::FireEvent(const nsAString& aType, TCPSocket* aSocket)
 {
-  AutoJSAPI api;
-  api.Init(GetOwnerGlobal());
-
   TCPServerSocketEventInit init;
   init.mBubbles = false;
   init.mCancelable = false;
   init.mSocket = aSocket;
 
   RefPtr<TCPServerSocketEvent> event =
       TCPServerSocketEvent::Constructor(this, aType, init);
   event->SetTrusted(true);
--- a/embedding/components/windowwatcher/nsWindowWatcher.cpp
+++ b/embedding/components/windowwatcher/nsWindowWatcher.cpp
@@ -625,16 +625,19 @@ nsWindowWatcher::OpenWindowInternal(mozI
   CalcSizeSpec(features.get(), sizeSpec);
 
   nsCOMPtr<nsIScriptSecurityManager> sm(
     do_GetService(NS_SCRIPTSECURITYMANAGER_CONTRACTID));
 
   bool isCallerChrome =
     nsContentUtils::LegacyIsCallerChromeOrNativeCode() && !openedFromRemoteTab;
 
+  // XXXbz Why is an AutoJSAPI good enough here?  Wouldn't AutoEntryScript (so
+  // we affect the entry global) make more sense?  Or do we just want to affect
+  // GetSubjectPrincipal()?
   dom::AutoJSAPI jsapiChromeGuard;
 
   bool windowTypeIsChrome =
     chromeFlags & nsIWebBrowserChrome::CHROME_OPENAS_CHROME;
   if (isCallerChrome && !hasChromeParent && !windowTypeIsChrome) {
     // open() is called from chrome on a non-chrome window, initialize an
     // AutoJSAPI with the callee to prevent the caller's privileges from leaking
     // into code that runs while opening the new window.