Bug 1425494 - Make browser-open-newtab-start notify with extra info. r=dmose,mstriemer
authorNan Jiang <najiang@mozilla.com>
Wed, 16 May 2018 11:15:47 -0400
changeset 418808 9111dee5d50185d07582776207f58617bbb54227
parent 418807 2eba9608447d0e8d2572b292d8d733645d698286
child 418809 9dd8b6f551abb9d1e3ecb3b68eba4ee683371376
push id34014
push useraciure@mozilla.com
push dateFri, 18 May 2018 22:04:52 +0000
treeherdermozilla-central@b54f574a1dd2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdmose, mstriemer
bugs1425494
milestone62.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 1425494 - Make browser-open-newtab-start notify with extra info. r=dmose,mstriemer MozReview-Commit-ID: EjDFjUvreEp
browser/base/content/browser.js
browser/base/content/test/tabs/browser_open_newtab_start_observer_notification.js
browser/base/content/utilityOverlay.js
browser/components/extensions/ExtensionControlledPopup.jsm
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -2309,26 +2309,16 @@ function openLocation() {
       // If there are no open browser windows, open a new one
       window.openDialog("chrome://browser/content/", "_blank",
                         "chrome,all,dialog=no", BROWSER_NEW_TAB_URL);
     }
   }
 }
 
 function BrowserOpenTab(event) {
-  // A notification intended to be useful for modular peformance tracking
-  // starting as close as is reasonably possible to the time when the user
-  // expressed the intent to open a new tab.  Since there are a lot of
-  // entry points, this won't catch every single tab created, but most
-  // initiated by the user should go through here.
-  //
-  // This is also used to notify a user that an extension has changed the
-  // New Tab page.
-  Services.obs.notifyObservers(null, "browser-open-newtab-start");
-
   let where = "tab";
   let relatedToCurrent = false;
 
   if (event) {
     where = whereToOpenLink(event, false, true);
 
     switch (where) {
       case "tab":
@@ -2338,19 +2328,34 @@ function BrowserOpenTab(event) {
         relatedToCurrent = true;
         break;
       case "current":
         where = "tab";
         break;
     }
   }
 
-  openTrustedLinkIn(BROWSER_NEW_TAB_URL, where, {
-    relatedToCurrent,
-  });
+  // A notification intended to be useful for modular peformance tracking
+  // starting as close as is reasonably possible to the time when the user
+  // expressed the intent to open a new tab.  Since there are a lot of
+  // entry points, this won't catch every single tab created, but most
+  // initiated by the user should go through here.
+  //
+  // Note 1: This notification gets notified with a promise that resolves
+  //         with the linked browser when the tab gets created
+  // Note 2: This is also used to notify a user that an extension has changed
+  //         the New Tab page.
+  Services.obs.notifyObservers({
+    wrappedJSObject: new Promise(resolve => {
+      openTrustedLinkIn(BROWSER_NEW_TAB_URL, where, {
+        relatedToCurrent,
+        resolveOnNewTabCreated: resolve
+      });
+    })
+  }, "browser-open-newtab-start");
 }
 
 var gLastOpenDirectory = {
   _lastDir: null,
   get path() {
     if (!this._lastDir || !this._lastDir.exists()) {
       try {
         this._lastDir = Services.prefs.getComplexValue("browser.open.lastDir",
--- a/browser/base/content/test/tabs/browser_open_newtab_start_observer_notification.js
+++ b/browser/base/content/test/tabs/browser_open_newtab_start_observer_notification.js
@@ -1,22 +1,25 @@
 "use strict";
 
 add_task(async function test_browser_open_newtab_start_observer_notification() {
 
   let observerFiredPromise = new Promise(resolve => {
-    function observe() {
-      resolve();
+    function observe(subject) {
+      Services.obs.removeObserver(observe, "browser-open-newtab-start");
+      resolve(subject.wrappedJSObject);
     }
     Services.obs.addObserver(observe, "browser-open-newtab-start");
   });
 
   // We're calling BrowserOpenTab() (rather the using BrowserTestUtils
   // because we want to be sure that it triggers the event to fire, since
   // it's very close to where various user-actions are triggered.
   BrowserOpenTab();
-  await observerFiredPromise;
+  const newTabCreatedPromise = await observerFiredPromise;
+  const browser = await newTabCreatedPromise;
   const tab = gBrowser.selectedTab;
 
   ok(true, "browser-open-newtab-start observer not called");
+  Assert.deepEqual(browser, tab.linkedBrowser, "browser-open-newtab-start notified with the created browser");
 
   BrowserTestUtils.removeTab(tab);
 });
--- a/browser/base/content/utilityOverlay.js
+++ b/browser/base/content/utilityOverlay.js
@@ -293,16 +293,17 @@ function openLinkIn(url, where, params) 
   var aNoReferrer           = params.noReferrer;
   var aAllowPopups          = !!params.allowPopups;
   var aUserContextId        = params.userContextId;
   var aIndicateErrorPageLoad = params.indicateErrorPageLoad;
   var aPrincipal            = params.originPrincipal;
   var aTriggeringPrincipal  = params.triggeringPrincipal;
   var aForceAboutBlankViewerInCurrent =
       params.forceAboutBlankViewerInCurrent;
+  var aResolveOnNewTabCreated = params.resolveOnNewTabCreated;
 
   if (where == "save") {
     // TODO(1073187): propagate referrerPolicy.
 
     // ContentClick.jsm passes isContentWindowPrivate for saveURL instead of passing a CPOW initiatingDoc
     if ("isContentWindowPrivate" in params) {
       saveURL(url, null, null, true, true, aNoReferrer ? null : aReferrerURI, null, params.isContentWindowPrivate);
     } else {
@@ -542,16 +543,20 @@ function openLinkIn(url, where, params) 
       noReferrer: aNoReferrer,
       userContextId: aUserContextId,
       originPrincipal: aPrincipal,
       triggeringPrincipal: aTriggeringPrincipal,
       focusUrlBar,
     });
     targetBrowser = tabUsedForLoad.linkedBrowser;
 
+    if (aResolveOnNewTabCreated) {
+      aResolveOnNewTabCreated(targetBrowser);
+    }
+
     if (params.frameOuterWindowID != undefined && w) {
       // Only notify it as a WebExtensions' webNavigation.onCreatedNavigationTarget
       // event if it contains the expected frameOuterWindowID params.
       // (e.g. we should not notify it as a onCreatedNavigationTarget if the user is
       // opening a new tab using the keyboard shortcut).
       Services.obs.notifyObservers({
         wrappedJSObject: {
           url,
--- a/browser/components/extensions/ExtensionControlledPopup.jsm
+++ b/browser/components/extensions/ExtensionControlledPopup.jsm
@@ -129,18 +129,24 @@ class ExtensionControlledPopup {
     return ExtensionSettingsStore.removeSetting(id, this.confirmedType, id);
   }
 
   observe(subject, topic, data) {
     // Remove the observer here so we don't get multiple open() calls if we get
     // multiple observer events in quick succession.
     this.removeObserver();
 
+    let targetWindow;
+    // Some notifications (e.g. browser-open-newtab-start) do not have a window subject.
+    if (subject && subject.document) {
+      targetWindow = subject;
+    }
+
     // Do this work in an idle callback to avoid interfering with new tab performance tracking.
-    this.topWindow.requestIdleCallback(() => this.open(subject));
+    this.topWindow.requestIdleCallback(() => this.open(targetWindow));
   }
 
   removeObserver() {
     if (this.observerRegistered) {
       Services.obs.removeObserver(this, this.observerTopic);
       this.observerRegistered = false;
       if (this.onObserverRemoved) {
         this.onObserverRemoved();