Bug 1541485. Stop using AutoJSContext in android widget code. r=snorp
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 13 May 2019 14:44:51 +0000
changeset 532428 abaf133a405518a1c014b704c4fe72dc27a018c1
parent 532427 83c6540f1fc1e9e409cf25e417c74009f3e5ab32
child 532429 96e678846de742425e0f48cbea15bad6731160f4
push id11268
push usercsabou@mozilla.com
push dateTue, 14 May 2019 15:24:22 +0000
treeherdermozilla-beta@5fb7fcd568d6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssnorp
bugs1541485
milestone68.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 1541485. Stop using AutoJSContext in android widget code. r=snorp Differential Revision: https://phabricator.services.mozilla.com/D30762
mobile/android/components/geckoview/GeckoViewHistory.cpp
widget/android/EventDispatcher.cpp
widget/android/nsIAndroidBridge.idl
--- a/mobile/android/components/geckoview/GeckoViewHistory.cpp
+++ b/mobile/android/components/geckoview/GeckoViewHistory.cpp
@@ -306,37 +306,30 @@ class OnVisitedCallback final : public n
  public:
   explicit OnVisitedCallback(GeckoViewHistory* aHistory,
                              nsIGlobalObject* aGlobalObject, nsIURI* aURI)
       : mHistory(aHistory), mGlobalObject(aGlobalObject), mURI(aURI) {}
 
   NS_DECL_ISUPPORTS
 
   NS_IMETHOD
-  OnSuccess(JS::HandleValue aData) override {
+  OnSuccess(JS::HandleValue aData, JSContext* aCx) override {
     bool shouldNotify = false;
-    {
-      // Scope `jsapi`.
-      dom::AutoJSAPI jsapi;
-      if (NS_WARN_IF(!jsapi.Init(mGlobalObject))) {
-        return NS_ERROR_FAILURE;
-      }
-      shouldNotify = ShouldNotifyVisited(jsapi.cx(), aData);
-      JS_ClearPendingException(jsapi.cx());
-    }
+    shouldNotify = ShouldNotifyVisited(aCx, aData);
+    JS_ClearPendingException(aCx);
     if (shouldNotify) {
       AutoTArray<VisitedURI, 1> visitedURIs;
       visitedURIs.AppendElement(VisitedURI{mURI, true});
       mHistory->HandleVisitedState(visitedURIs);
     }
     return NS_OK;
   }
 
   NS_IMETHOD
-  OnError(JS::HandleValue aData) override { return NS_OK; }
+  OnError(JS::HandleValue aData, JSContext* aCx) override { return NS_OK; }
 
  private:
   virtual ~OnVisitedCallback() {}
 
   bool ShouldNotifyVisited(JSContext* aCx, JS::HandleValue aData) {
     if (NS_WARN_IF(!aData.isBoolean())) {
       return false;
     }
@@ -499,35 +492,28 @@ class GetVisitedCallback final : public 
   explicit GetVisitedCallback(GeckoViewHistory* aHistory,
                               nsIGlobalObject* aGlobalObject,
                               const nsTArray<nsCOMPtr<nsIURI>>& aURIs)
       : mHistory(aHistory), mGlobalObject(aGlobalObject), mURIs(aURIs) {}
 
   NS_DECL_ISUPPORTS
 
   NS_IMETHOD
-  OnSuccess(JS::HandleValue aData) override {
+  OnSuccess(JS::HandleValue aData, JSContext* aCx) override {
     nsTArray<VisitedURI> visitedURIs;
-    {
-      // Scope `jsapi`.
-      dom::AutoJSAPI jsapi;
-      if (NS_WARN_IF(!jsapi.Init(mGlobalObject))) {
-        return NS_ERROR_FAILURE;
-      }
-      if (!ExtractVisitedURIs(jsapi.cx(), aData, visitedURIs)) {
-        JS_ClearPendingException(jsapi.cx());
-        return NS_ERROR_FAILURE;
-      }
+    if (!ExtractVisitedURIs(aCx, aData, visitedURIs)) {
+      JS_ClearPendingException(aCx);
+      return NS_ERROR_FAILURE;
     }
     mHistory->HandleVisitedState(visitedURIs);
     return NS_OK;
   }
 
   NS_IMETHOD
-  OnError(JS::HandleValue aData) override { return NS_OK; }
+  OnError(JS::HandleValue aData, JSContext* aCx) override { return NS_OK; }
 
  private:
   virtual ~GetVisitedCallback() {}
 
   /**
    * Unpacks an array of Boolean visited statuses from the session handler into
    * an array of `VisitedURI` structs. Each element in the array corresponds to
    * a URI in `mURIs`.
--- a/widget/android/EventDispatcher.cpp
+++ b/widget/android/EventDispatcher.cpp
@@ -574,76 +574,75 @@ nsresult UnboxData(jni::String::Param aE
   return NS_ERROR_INVALID_ARG;
 }
 
 class JavaCallbackDelegate final : public nsIAndroidEventCallback {
   const java::EventCallback::GlobalRef mCallback;
 
   virtual ~JavaCallbackDelegate() {}
 
-  NS_IMETHOD Call(JS::HandleValue aData,
+  NS_IMETHOD Call(JSContext* aCx, JS::HandleValue aData,
                   void (java::EventCallback::*aCall)(jni::Object::Param)
                       const) {
     MOZ_ASSERT(NS_IsMainThread());
-    AutoJSContext cx;
 
     jni::Object::LocalRef data(jni::GetGeckoThreadEnv());
-    nsresult rv = BoxData(NS_LITERAL_STRING("callback"), cx, aData, data,
+    nsresult rv = BoxData(NS_LITERAL_STRING("callback"), aCx, aData, data,
                           /* ObjectOnly */ false);
-    NS_ENSURE_SUCCESS(rv, JS_IsExceptionPending(cx) ? NS_OK : rv);
+    NS_ENSURE_SUCCESS(rv, rv);
 
     dom::AutoNoJSAPI nojsapi;
 
     (java::EventCallback(*mCallback).*aCall)(data);
     return NS_OK;
   }
 
  public:
   explicit JavaCallbackDelegate(java::EventCallback::Param aCallback)
       : mCallback(jni::GetGeckoThreadEnv(), aCallback) {}
 
   NS_DECL_ISUPPORTS
 
-  NS_IMETHOD OnSuccess(JS::HandleValue aData) override {
-    return Call(aData, &java::EventCallback::SendSuccess);
+  NS_IMETHOD OnSuccess(JS::HandleValue aData, JSContext* aCx) override {
+    return Call(aCx, aData, &java::EventCallback::SendSuccess);
   }
 
-  NS_IMETHOD OnError(JS::HandleValue aData) override {
-    return Call(aData, &java::EventCallback::SendError);
+  NS_IMETHOD OnError(JS::HandleValue aData, JSContext* aCx) override {
+    return Call(aCx, aData, &java::EventCallback::SendError);
   }
 };
 
 NS_IMPL_ISUPPORTS(JavaCallbackDelegate, nsIAndroidEventCallback)
 
 class NativeCallbackDelegateSupport final
     : public java::EventDispatcher::NativeCallbackDelegate ::Natives<
           NativeCallbackDelegateSupport> {
   using CallbackDelegate = java::EventDispatcher::NativeCallbackDelegate;
   using Base = CallbackDelegate::Natives<NativeCallbackDelegateSupport>;
 
   const nsCOMPtr<nsIAndroidEventCallback> mCallback;
   const nsCOMPtr<nsIAndroidEventFinalizer> mFinalizer;
   const nsCOMPtr<nsIGlobalObject> mGlobalObject;
 
   void Call(jni::Object::Param aData,
-            nsresult (nsIAndroidEventCallback::*aCall)(JS::HandleValue)) {
+            nsresult (nsIAndroidEventCallback::*aCall)(JS::HandleValue,
+                                                       JSContext*)) {
     MOZ_ASSERT(NS_IsMainThread());
 
     // Use either the attached window's realm or a default realm.
 
     dom::AutoJSAPI jsapi;
     NS_ENSURE_TRUE_VOID(jsapi.Init(mGlobalObject));
 
     JS::RootedValue data(jsapi.cx());
     nsresult rv = UnboxData(NS_LITERAL_STRING("callback"), jsapi.cx(), aData,
                             &data, /* BundleOnly */ false);
     NS_ENSURE_SUCCESS_VOID(rv);
 
-    dom::AutoNoJSAPI nojsapi;
-    rv = (mCallback->*aCall)(data);
+    rv = (mCallback->*aCall)(data, jsapi.cx());
     NS_ENSURE_SUCCESS_VOID(rv);
   }
 
  public:
   using Base::AttachNative;
 
   template <typename Functor>
   static void OnNativeCall(Functor&& aCall) {
@@ -808,16 +807,20 @@ EventDispatcher::Dispatch(JS::HandleValu
 
   java::EventDispatcher::LocalRef dispatcher(mDispatcher);
   if (!dispatcher) {
     return NS_OK;
   }
 
   jni::Object::LocalRef data(jni::GetGeckoThreadEnv());
   nsresult rv = BoxData(event, aCx, aData, data, /* ObjectOnly */ true);
+  // Keep XPConnect from overriding the JSContext exception with one
+  // based on the nsresult.
+  //
+  // XXXbz Does xpconnect still do that?  Needs to be checked/tested.
   NS_ENSURE_SUCCESS(rv, JS_IsExceptionPending(aCx) ? NS_OK : rv);
 
   dom::AutoNoJSAPI nojsapi;
   dispatcher->DispatchToThreads(event, data,
                                 WrapCallback(aCallback, aFinalizer));
   return NS_OK;
 }
 
--- a/widget/android/nsIAndroidBridge.idl
+++ b/widget/android/nsIAndroidBridge.idl
@@ -29,17 +29,19 @@ interface nsIAndroidBrowserApp : nsISupp
   readonly attribute nsIBrowserTab selectedTab;
   nsIBrowserTab getBrowserTab(in int32_t tabId);
   nsIUITelemetryObserver getUITelemetryObserver();
 };
 
 [scriptable, uuid(e64c39b8-b8ec-477d-aef5-89d517ff9219)]
 interface nsIAndroidEventCallback : nsISupports
 {
+  [implicit_jscontext]
   void onSuccess([optional] in jsval data);
+  [implicit_jscontext]
   void onError([optional] in jsval data);
 };
 
 [scriptable, function, uuid(819ee2db-d3b8-46dd-a476-40f89c49133c)]
 interface nsIAndroidEventFinalizer : nsISupports
 {
   void onFinalize();
 };