Bug 1425494 - Make browser-open-newtab-start notify with extra info. r?dmose,mstriemer draft
authorNan Jiang <najiang@mozilla.com>
Wed, 16 May 2018 11:15:47 -0400
changeset 796419 5970f5d4949d41670d26fdb048259a891de68185
parent 795657 3c9d69736f4a421218e5eb01b6571d535d38318a
push id110251
push usernajiang@mozilla.com
push dateThu, 17 May 2018 17:28:08 +0000
reviewersdmose, mstriemer
bugs1425494
milestone62.0a1
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();