Bug 1442360 part 1. Change InstallTrigger to not use the JavaScript-global-property category. r=kmag
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 21 Mar 2018 23:18:51 -0400
changeset 409451 48ad847eb4a76dff27602b694cfa4d1eefe48079
parent 409450 9624e53b439e891166959fa87eb31ac41a9041b1
child 409452 4d642b71d6f4f852eb2ee66cf72408965c3cd42a
push id33687
push userapavel@mozilla.com
push dateThu, 22 Mar 2018 09:31:48 +0000
treeherdermozilla-central@7771df14ea18 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskmag
bugs1442360, 609794
milestone61.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 1442360 part 1. Change InstallTrigger to not use the JavaScript-global-property category. r=kmag test_bug609794.html was testing a behavior that the method before the current method of attaching InstallTrigger to windows depended on. We don't really need that behavior, which is good, because this change is not producing it. MozReview-Commit-ID: GPzif89UYYl
dom/base/nsGlobalWindowInner.cpp
dom/base/nsGlobalWindowInner.h
dom/webidl/Window.webidl
toolkit/mozapps/extensions/amInstallTrigger.js
toolkit/mozapps/extensions/extensions.manifest
toolkit/mozapps/extensions/test/mochitest/mochitest.ini
toolkit/mozapps/extensions/test/mochitest/test_bug609794.html
--- a/dom/base/nsGlobalWindowInner.cpp
+++ b/dom/base/nsGlobalWindowInner.cpp
@@ -234,16 +234,17 @@
 #include "mozilla/dom/PrimitiveConversions.h"
 #include "mozilla/dom/WindowBinding.h"
 #include "nsITabChild.h"
 #include "mozilla/dom/MediaQueryList.h"
 #include "mozilla/dom/ScriptSettings.h"
 #include "mozilla/dom/NavigatorBinding.h"
 #include "mozilla/dom/ImageBitmap.h"
 #include "mozilla/dom/ImageBitmapBinding.h"
+#include "mozilla/dom/InstallTriggerBinding.h"
 #include "mozilla/dom/ServiceWorker.h"
 #include "mozilla/dom/ServiceWorkerRegistration.h"
 #include "mozilla/dom/ServiceWorkerRegistrationDescriptor.h"
 #include "mozilla/dom/U2F.h"
 #include "mozilla/dom/WebIDLGlobalNameHash.h"
 #include "mozilla/dom/Worklet.h"
 #ifdef HAVE_SIDEBAR
 #include "mozilla/dom/ExternalBinding.h"
@@ -1215,16 +1216,17 @@ nsGlobalWindowInner::CleanUp()
   mIndexedDB = nullptr;
 
   mConsole = nullptr;
 
   mAudioWorklet = nullptr;
   mPaintWorklet = nullptr;
 
   mExternal = nullptr;
+  mInstallTrigger = nullptr;
 
   mPerformance = nullptr;
 
 #ifdef MOZ_WEBSPEECH
   mSpeechSynthesis = nullptr;
 #endif
 
 #if defined(MOZ_WIDGET_ANDROID)
@@ -1518,16 +1520,17 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mStatusbar)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mScrollbars)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mCrypto)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mU2F)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mConsole)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mAudioWorklet)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPaintWorklet)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mExternal)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mInstallTrigger)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mIntlUtils)
 
   tmp->TraverseHostObjectURIs(cb);
 
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mChromeFields.mMessageManager)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mChromeFields.mGroupMessageManagers)
 
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPendingPromises)
@@ -1602,16 +1605,17 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(ns
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mStatusbar)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mScrollbars)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mCrypto)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mU2F)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mConsole)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mAudioWorklet)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mPaintWorklet)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mExternal)
+  NS_IMPL_CYCLE_COLLECTION_UNLINK(mInstallTrigger)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mIntlUtils)
 
   tmp->UnlinkHostObjectURIs();
 
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mIdleRequestExecutor)
 
   // Here the IdleRequest list would've been unlinked, but we rely on
   // that IdleRequest objects have been traced and will remove
@@ -2445,16 +2449,34 @@ nsGlobalWindowInner::ShouldReportForServ
     nsGlobalWindowInner::Cast(topOuter->GetCurrentInnerWindow());
   NS_ENSURE_TRUE(topInner, false);
 
   topInner->ShouldReportForServiceWorkerScopeInternal(NS_ConvertUTF16toUTF8(aScope),
                                                       &result);
   return result;
 }
 
+already_AddRefed<InstallTriggerImpl>
+nsGlobalWindowInner::GetInstallTrigger()
+{
+  if (!mInstallTrigger) {
+    JS::Rooted<JSObject*> jsImplObj(RootingCx());
+    ErrorResult rv;
+    ConstructJSImplementation("@mozilla.org/addons/installtrigger;1", this,
+                              &jsImplObj, rv);
+    if (rv.Failed()) {
+      rv.SuppressException();
+      return nullptr;
+    }
+    mInstallTrigger = new InstallTriggerImpl(jsImplObj, this);
+  }
+
+  return do_AddRef(mInstallTrigger);
+}
+
 nsGlobalWindowInner::CallState
 nsGlobalWindowInner::ShouldReportForServiceWorkerScopeInternal(const nsACString& aScope,
                                                                bool* aResultOut)
 {
   MOZ_DIAGNOSTIC_ASSERT(aResultOut);
 
   // First check to see if this window is controlled.  If so, then we have
   // found a match and are done.
@@ -7697,18 +7719,17 @@ nsGlobalWindowInner::IsSecureContext() c
   return JS_GetIsSecureContext(js::GetObjectCompartment(GetWrapperPreserveColor()));
 }
 
 already_AddRefed<External>
 nsGlobalWindowInner::GetExternal(ErrorResult& aRv)
 {
 #ifdef HAVE_SIDEBAR
   if (!mExternal) {
-    AutoJSContext cx;
-    JS::Rooted<JSObject*> jsImplObj(cx);
+    JS::Rooted<JSObject*> jsImplObj(RootingCx());
     ConstructJSImplementation("@mozilla.org/sidebar;1", this, &jsImplObj, aRv);
     if (aRv.Failed()) {
       return nullptr;
     }
     mExternal = new External(jsImplObj, this);
   }
 
   RefPtr<External> external = static_cast<External*>(mExternal.get());
--- a/dom/base/nsGlobalWindowInner.h
+++ b/dom/base/nsGlobalWindowInner.h
@@ -105,16 +105,17 @@ class CustomElementRegistry;
 class DocGroup;
 class External;
 class Function;
 class Gamepad;
 enum class ImageBitmapFormat : uint8_t;
 class IdleRequest;
 class IdleRequestCallback;
 class IncrementalRunnable;
+class InstallTriggerImpl;
 class IntlUtils;
 class Location;
 class MediaQueryList;
 class OwningExternalOrWindowProxy;
 class Promise;
 class PostMessageEvent;
 struct RequestInit;
 class RequestOrUSVString;
@@ -984,16 +985,18 @@ public:
   void GetInterface(JSContext* aCx, nsIJSID* aIID,
                     JS::MutableHandle<JS::Value> aRetval,
                     mozilla::ErrorResult& aError);
 
   already_AddRefed<nsWindowRoot> GetWindowRoot(mozilla::ErrorResult& aError);
 
   bool ShouldReportForServiceWorkerScope(const nsAString& aScope);
 
+  already_AddRefed<mozilla::dom::InstallTriggerImpl> GetInstallTrigger();
+
   void UpdateTopInnerWindow();
 
   virtual bool IsInSyncOperation() override
   {
     return GetExtantDoc() && GetExtantDoc()->IsInSyncOperation();
   }
 
 protected:
@@ -1382,16 +1385,17 @@ protected:
   RefPtr<mozilla::dom::Console> mConsole;
   RefPtr<mozilla::dom::Worklet> mAudioWorklet;
   RefPtr<mozilla::dom::Worklet> mPaintWorklet;
   // We need to store an nsISupports pointer to this object because the
   // mozilla::dom::External class doesn't exist on b2g and using the type
   // forward declared here means that ~nsGlobalWindow wouldn't compile because
   // it wouldn't see the ~External function's declaration.
   nsCOMPtr<nsISupports>         mExternal;
+  RefPtr<mozilla::dom::InstallTriggerImpl> mInstallTrigger;
 
   RefPtr<mozilla::dom::Storage> mLocalStorage;
   RefPtr<mozilla::dom::Storage> mSessionStorage;
 
   RefPtr<mozilla::EventListenerManager> mListenerManager;
   RefPtr<mozilla::dom::Location> mLocation;
   RefPtr<nsHistory>           mHistory;
   RefPtr<mozilla::dom::CustomElementRegistry> mCustomElements;
--- a/dom/webidl/Window.webidl
+++ b/dom/webidl/Window.webidl
@@ -343,16 +343,24 @@ partial interface Window {
   readonly attribute WindowRoot? windowRoot;
 
   /**
    * ChromeOnly method to determine if a particular window should see console
    * reports from service workers of the given scope.
    */
   [ChromeOnly]
   boolean shouldReportForServiceWorkerScope(USVString aScope);
+
+  /**
+   * InstallTrigger is used for extension installs.  Ideally it would
+   * be something like a WebIDL namespace, but we don't support
+   * JS-implemented static things yet.  See bug 863952.
+   */
+  [Replaceable]
+  readonly attribute InstallTriggerImpl? InstallTrigger;
 };
 
 Window implements TouchEventHandlers;
 
 Window implements OnErrorEventHandlerForWindow;
 
 #if defined(MOZ_WIDGET_ANDROID)
 // https://compat.spec.whatwg.org/#windoworientation-interface
--- a/toolkit/mozapps/extensions/amInstallTrigger.js
+++ b/toolkit/mozapps/extensions/amInstallTrigger.js
@@ -116,40 +116,32 @@ RemoteMediator.prototype = {
   QueryInterface: XPCOMUtils.generateQI([Ci.nsISupportsWeakReference])
 };
 
 
 function InstallTrigger() {
 }
 
 InstallTrigger.prototype = {
-  // Here be magic. We've declared ourselves as providing the
-  // nsIDOMGlobalPropertyInitializer interface, and are registered in the
-  // "JavaScript-global-property" category in the XPCOM category manager. This
-  // means that for newly created windows, XPCOM will createinstance this
-  // object, and then call init, passing in the window for which we need to
-  // provide an instance. We then initialize ourselves and return the webidl
-  // version of this object using the webidl-provided _create method, which
-  // XPCOM will then duly expose as a property value on the window. All this
-  // indirection is necessary because webidl does not (yet) support statics
-  // (bug 863952). See bug 926712 for more details about this implementation.
+  // We've declared ourselves as providing the nsIDOMGlobalPropertyInitializer
+  // interface.  This means that when the InstallTrigger property is gotten from
+  // the window that will createInstance this object and then call init(),
+  // passing the window were bound to.  It will then automatically create the
+  // WebIDL wrapper (InstallTriggerImpl) for this object.  This indirection is
+  // necessary because webidl does not (yet) support statics (bug 863952). See
+  // bug 926712 and then bug 1442360 for more details about this implementation.
   init(window) {
     this._window = window;
     this._principal = window.document.nodePrincipal;
     this._url = window.document.documentURIObject;
 
-    try {
-      this._mediator = new RemoteMediator(window);
-    } catch (ex) {
-      // If we can't set up IPC (e.g., because this is a top-level window
-      // or something), then don't expose InstallTrigger.
-      return null;
-    }
-
-    return window.InstallTriggerImpl._create(window, this);
+    this._mediator = new RemoteMediator(window);
+    // If we can't set up IPC (e.g., because this is a top-level window or
+    // something), then don't expose InstallTrigger.  The Window code handles
+    // that, if we throw an exception here.
   },
 
   enabled() {
     return this._mediator.enabled(this._url.spec);
   },
 
   updateEnabled() {
     return this.enabled();
--- a/toolkit/mozapps/extensions/extensions.manifest
+++ b/toolkit/mozapps/extensions/extensions.manifest
@@ -9,15 +9,14 @@ component {4399533d-08d1-458c-a87a-235f7
 contract @mozilla.org/addons/integration;1 {4399533d-08d1-458c-a87a-235f74451cfa}
 #ifndef MOZ_WIDGET_ANDROID
 category update-timer addonManager @mozilla.org/addons/integration;1,getService,addon-background-update-timer,extensions.update.interval,86400
 #endif
 component {7beb3ba8-6ec3-41b4-b67c-da89b8518922} amContentHandler.js
 contract @mozilla.org/uriloader/content-handler;1?type=application/x-xpinstall {7beb3ba8-6ec3-41b4-b67c-da89b8518922}
 component {9df8ef2b-94da-45c9-ab9f-132eb55fddf1} amInstallTrigger.js
 contract @mozilla.org/addons/installtrigger;1 {9df8ef2b-94da-45c9-ab9f-132eb55fddf1}
-category JavaScript-global-property InstallTrigger @mozilla.org/addons/installtrigger;1
 #ifndef MOZ_WIDGET_ANDROID
 category addon-provider-module PluginProvider resource://gre/modules/addons/PluginProvider.jsm
 #endif
 category addon-provider-module GMPProvider resource://gre/modules/addons/GMPProvider.jsm
 component {8866d8e3-4ea5-48b7-a891-13ba0ac15235} amWebAPI.js
 contract @mozilla.org/addon-web-api/manager;1 {8866d8e3-4ea5-48b7-a891-13ba0ac15235}
--- a/toolkit/mozapps/extensions/test/mochitest/mochitest.ini
+++ b/toolkit/mozapps/extensions/test/mochitest/mochitest.ini
@@ -1,9 +1,8 @@
 [DEFAULT]
 support-files =
   file_empty.html
   file_bug687194.xpi
 
-[test_bug609794.html]
 [test_bug687194.html]
 skip-if = e10s || os == "android" # this test creates its own child process, no need to run it in e10s
 [test_bug887098.html]
deleted file mode 100644
--- a/toolkit/mozapps/extensions/test/mochitest/test_bug609794.html
+++ /dev/null
@@ -1,27 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<!--
-https://bugzilla.mozilla.org/show_bug.cgi?id=609794
--->
-<head>
-  <title>Test for Bug 609794</title>
-  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
-  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
-</head>
-<body>
-<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=609794">Mozilla Bug 609794</a>
-<p id="display"></p>
-<div id="content" style="display: none">
-  
-</div>
-<pre id="test">
-<script type="application/javascript">
-
-/** Test for Bug 609794 **/
-var obj = Object.create(window);
-is(Object.prototype.toString.call(obj.InstallTrigger), "[object InstallTriggerImpl]", "can get InstallTrigger through the prototype");
-
-</script>
-</pre>
-</body>
-</html>