Bug 860438 - Remove context stack craziness from nsWindowWatcher. r=gabor
authorBobby Holley <bobbyholley@gmail.com>
Mon, 22 Apr 2013 16:49:52 -0400
changeset 140466 776a69c7f3eb7efcb75424c3333b7ff806ad41d9
parent 140465 756245b309a286767ee55a460ea6218bdb86c300
child 140467 fdaa0659fe13283cd81955309fefd7e1b4889f13
push id2579
push userakeybl@mozilla.com
push dateMon, 24 Jun 2013 18:52:47 +0000
treeherdermozilla-beta@b69b7de8a05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgabor
bugs860438
milestone23.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 860438 - Remove context stack craziness from nsWindowWatcher. r=gabor This patch should not change any behavior.
embedding/components/windowwatcher/src/nsWindowWatcher.cpp
embedding/components/windowwatcher/src/nsWindowWatcher.h
--- a/embedding/components/windowwatcher/src/nsWindowWatcher.cpp
+++ b/embedding/components/windowwatcher/src/nsWindowWatcher.cpp
@@ -25,17 +25,16 @@
 #include "nsIDOMWindow.h"
 #include "nsIDOMChromeWindow.h"
 #include "nsIDOMModalContentWindow.h"
 #include "nsIPrompt.h"
 #include "nsIScriptObjectPrincipal.h"
 #include "nsIScreen.h"
 #include "nsIScreenManager.h"
 #include "nsIScriptContext.h"
-#include "nsIJSContextStack.h"
 #include "nsIObserverService.h"
 #include "nsIScriptGlobalObject.h"
 #include "nsIScriptSecurityManager.h"
 #include "nsXPCOM.h"
 #include "nsIURI.h"
 #include "nsIWebBrowser.h"
 #include "nsIWebBrowserChrome.h"
 #include "nsIWebNavigation.h"
@@ -61,18 +60,16 @@
 #include "mozilla/Preferences.h"
 
 #ifdef USEWEAKREFS
 #include "nsIWeakReference.h"
 #endif
 
 using namespace mozilla;
 
-static const char *sJSStackContractID="@mozilla.org/js/xpc/ContextStack;1";
-
 /****************************************************************
  ******************** nsWatcherWindowEntry **********************
  ****************************************************************/
 
 class nsWindowWatcher;
 
 struct nsWatcherWindowEntry {
 
@@ -231,70 +228,16 @@ nsWatcherWindowEnumerator::FindNext()
 void nsWatcherWindowEnumerator::WindowRemoved(nsWatcherWindowEntry *inInfo) {
 
   if (mCurrentPosition == inInfo)
     mCurrentPosition = mCurrentPosition != inInfo->mYounger ?
                        inInfo->mYounger : 0;
 }
 
 /****************************************************************
- ********************** JSContextAutoPopper *********************
- ****************************************************************/
-
-class MOZ_STACK_CLASS JSContextAutoPopper {
-public:
-  JSContextAutoPopper();
-  ~JSContextAutoPopper();
-
-  nsresult   Push(JSContext *cx = nullptr);
-  JSContext *get() { return mContext; }
-
-protected:
-  nsCOMPtr<nsIThreadJSContextStack>  mService;
-  JSContext                         *mContext;
-  nsCOMPtr<nsIScriptContext>         mContextKungFuDeathGrip;
-};
-
-JSContextAutoPopper::JSContextAutoPopper() : mContext(nullptr)
-{
-}
-
-JSContextAutoPopper::~JSContextAutoPopper()
-{
-  JSContext *cx;
-  nsresult   rv;
-
-  if(mContext) {
-    rv = mService->Pop(&cx);
-    NS_ASSERTION(NS_SUCCEEDED(rv) && cx == mContext, "JSContext push/pop mismatch");
-  }
-}
-
-nsresult JSContextAutoPopper::Push(JSContext *cx)
-{
-  if (mContext) // only once
-    return NS_ERROR_FAILURE;
-
-  mService = do_GetService(sJSStackContractID);
-  if (mService) {
-    // Get the safe context if we're not provided one.
-    if (!cx) {
-      cx = mService->GetSafeJSContext();
-    }
-
-    // Save cx in mContext to indicate need to pop.
-    if (cx && NS_SUCCEEDED(mService->Push(cx))) {
-      mContext = cx;
-      mContextKungFuDeathGrip = nsJSUtils::GetDynamicScriptContext(cx);
-    }
-  }
-  return mContext ? NS_OK : NS_ERROR_FAILURE;
-}
-
-/****************************************************************
  *********************** nsWindowWatcher ************************
  ****************************************************************/
 
 NS_IMPL_ADDREF(nsWindowWatcher)
 NS_IMPL_RELEASE(nsWindowWatcher)
 NS_IMPL_QUERY_INTERFACE3(nsWindowWatcher,
                          nsIWindowWatcher,
                          nsIPromptFactory,
@@ -493,17 +436,17 @@ nsWindowWatcher::OpenWindowInternal(nsID
                                   uriToLoadIsChrome = false,
                                   windowIsModalContentDialog = false;
   uint32_t                        chromeFlags;
   nsAutoString                    name;             // string version of aName
   nsAutoCString                   features;         // string version of aFeatures
   nsCOMPtr<nsIURI>                uriToLoad;        // from aUrl, if any
   nsCOMPtr<nsIDocShellTreeOwner>  parentTreeOwner;  // from the parent window, if any
   nsCOMPtr<nsIDocShellTreeItem>   newDocShellItem;  // from the new window
-  JSContextAutoPopper             callerContextGuard;
+  nsCxPusher                      callerContextGuard;
 
   NS_ENSURE_ARG_POINTER(_retval);
   *_retval = 0;
 
   if (!nsContentUtils::IsSafeToRunScript()) {
     return NS_ERROR_FAILURE;
   }
 
@@ -922,46 +865,47 @@ nsWindowWatcher::OpenWindowInternal(nsID
     nsCOMPtr<nsILoadContext> childContext = do_QueryInterface(newDocShellItem);
     if (childContext) {
       childContext->SetPrivateBrowsing(isPrivateBrowsingWindow);
     }
   }
 
   nsCOMPtr<nsIDocShellLoadInfo> loadInfo;
   if (uriToLoad && aNavigate) { // get the script principal and pass it to docshell
-    JSContextAutoPopper contextGuard;
 
-    cx = GetJSContextFromCallStack();
-
-    // get the security manager
-    if (!cx)
+    // The historical ordering of attempts here is:
+    // (1) Stack-top cx.
+    // (2) cx associated with aParent.
+    // (3) Safe JSContext.
+    //
+    // We could just use an AutoJSContext here if it weren't for (2), which
+    // is probably nonsensical anyway. But we preserve the old semantics for
+    // now, because this is part of a large patch where we don't want to risk
+    // subtle behavioral modifications.
+    cx = nsContentUtils::GetCurrentJSContext();
+    nsCxPusher pusher;
+    if (!cx) {
       cx = GetJSContextFromWindow(aParent);
-    if (!cx) {
-      rv = contextGuard.Push();
-      if (NS_FAILED(rv))
-        return rv;
-      cx = contextGuard.get();
+      if (!cx)
+        cx = nsContentUtils::GetSafeJSContext();
+      pusher.Push(cx);
     }
 
     newDocShell->CreateLoadInfo(getter_AddRefs(loadInfo));
     NS_ENSURE_TRUE(loadInfo, NS_ERROR_FAILURE);
 
     if (subjectPrincipal) {
       loadInfo->SetOwner(subjectPrincipal);
     }
 
     // Set the new window's referrer from the calling context's document:
 
-    // get the calling context off the JS context stack
-    nsCOMPtr<nsIJSContextStack> stack = do_GetService(sJSStackContractID);
-
-    JSContext* ccx = nullptr;
-
     // get its document, if any
-    if (stack && NS_SUCCEEDED(stack->Peek(&ccx)) && ccx) {
+    JSContext* ccx = nsContentUtils::GetCurrentJSContext();
+    if (ccx) {
       nsIScriptGlobalObject *sgo = nsJSUtils::GetDynamicScriptGlobal(ccx);
 
       nsCOMPtr<nsPIDOMWindow> w(do_QueryInterface(sgo));
       if (w) {
         /* use the URL from the *extant* document, if any. The usual accessor
            GetDocument will synchronously create an about:blank document if
            it has no better answer, and we only care about a real document.
            Also using GetDocument to force document creation seems to
@@ -1407,17 +1351,17 @@ nsWindowWatcher::URIfromURL(const char *
                             nsIDOMWindow *aParent,
                             nsIURI **aURI)
 {
   nsCOMPtr<nsIDOMWindow> baseWindow;
 
   /* build the URI relative to the calling JS Context, if any.
      (note this is the same context used to make the security check
      in nsGlobalWindow.cpp.) */
-  JSContext *cx = GetJSContextFromCallStack();
+  JSContext *cx = nsContentUtils::GetCurrentJSContext();
   if (cx) {
     nsIScriptContext *scriptcx = nsJSUtils::GetDynamicScriptContext(cx);
     if (scriptcx) {
       baseWindow = do_QueryInterface(scriptcx->GetGlobalObject());
     }
   }
 
   // failing that, build it relative to the parent window, if possible
@@ -1769,25 +1713,17 @@ nsWindowWatcher::FindItemWithName(const 
   } while(1);
 
   return rv;
 }
 
 already_AddRefed<nsIDocShellTreeItem>
 nsWindowWatcher::GetCallerTreeItem(nsIDocShellTreeItem* aParentItem)
 {
-  nsCOMPtr<nsIJSContextStack> stack =
-    do_GetService(sJSStackContractID);
-
-  JSContext *cx = nullptr;
-
-  if (stack) {
-    stack->Peek(&cx);
-  }
-
+  JSContext *cx = nsContentUtils::GetCurrentJSContext();
   nsIDocShellTreeItem* callerItem = nullptr;
 
   if (cx) {
     nsCOMPtr<nsIWebNavigation> callerWebNav =
       do_GetInterface(nsJSUtils::GetDynamicScriptGlobal(cx));
 
     if (callerWebNav) {
       CallQueryInterface(callerWebNav, &callerItem);
@@ -2156,28 +2092,16 @@ nsWindowWatcher::GetWindowTreeOwner(nsID
 
   nsCOMPtr<nsIDocShellTreeItem> treeItem;
   GetWindowTreeItem(inWindow, getter_AddRefs(treeItem));
   if (treeItem)
     treeItem->GetTreeOwner(outTreeOwner);
 }
 
 JSContext *
-nsWindowWatcher::GetJSContextFromCallStack()
-{
-  JSContext *cx = 0;
-
-  nsCOMPtr<nsIThreadJSContextStack> cxStack(do_GetService(sJSStackContractID));
-  if (cxStack)
-    cxStack->Peek(&cx);
-
-  return cx;
-}
-
-JSContext *
 nsWindowWatcher::GetJSContextFromWindow(nsIDOMWindow *aWindow)
 {
   JSContext *cx = 0;
 
   if (aWindow) {
     nsCOMPtr<nsIScriptGlobalObject> sgo(do_QueryInterface(aWindow));
     if (sgo) {
       nsIScriptContext *scx = sgo->GetContext();
--- a/embedding/components/windowwatcher/src/nsWindowWatcher.h
+++ b/embedding/components/windowwatcher/src/nsWindowWatcher.h
@@ -78,17 +78,16 @@ protected:
                               const char *aFeatures,
                               bool aCalledFromJS,
                               bool aDialog,
                               bool aNavigate,
                               nsIArray *argv,
                               nsIDOMWindow **_retval);
 
   static JSContext *GetJSContextFromWindow(nsIDOMWindow *aWindow);
-  static JSContext *GetJSContextFromCallStack();
   static nsresult   URIfromURL(const char *aURL,
                                nsIDOMWindow *aParent,
                                nsIURI **aURI);
   
   static uint32_t   CalculateChromeFlags(nsIDOMWindow *aParent,
                                          const char *aFeatures,
                                          bool aFeaturesSpecified,
                                          bool aDialog,