Bug 1314861: Lazily create view for Panels. r=rpl

This is the one change that's slightly risky, since it may impact add-ons that
depend on panel documents loading immediately, rather than only when they're
needed. It may require some developer outreach.

MozReview-Commit-ID: 4ICPKcvLiQR

--HG--
extra : rebase_source : a08640c3a753b20c7457e7ec025698cce501b30f

addon-sdk/source/lib/sdk/panel.js

@@ -16,7 +16,7 @@ module.metadata = {
 const { Cu, Ci } = require("chrome");
 const { setTimeout } = require('./timers');
 const { Class } = require("./core/heritage");
-const { merge } = require("./util/object");
+const { DefaultWeakMap, merge } = require("./util/object");
 const { WorkerHost } = require("./content/utils");
 const { Worker } = require("./deprecated/sync-worker");
 const { Disposable } = require("./core/disposable");
@@ -99,10 +99,36 @@ function isDisposed(panel) {
   return !views.has(panel);
 }
 
+var optionsMap = new WeakMap();
 var panels = new WeakMap();
 var models = new WeakMap();
-var views = new WeakMap();
-var workers = new WeakMap();
+var views = new DefaultWeakMap(panel => {
+  let model = models.get(panel);
+
+  // Setup view
+  let viewOptions = {allowJavascript: !model.allow || (model.allow.script !== false)};
+  let view = domPanel.make(null, viewOptions);
+  panels.set(view, panel);
+
+  // Load panel content.
+  domPanel.setURL(view, model.contentURL);
+
+  // Allow context menu
+  domPanel.allowContextMenu(view, model.contextMenu);
+
+  return view;
+});
+var workers = new DefaultWeakMap(panel => {
+  let options = optionsMap.get(panel);
+
+  let worker = new Worker(stripListeners(options));
+  workers.set(panel, worker);
+
+  // pipe events from worker to a panel.
+  pipe(worker, panel);
+
+  return worker;
+});
 var styles = new WeakMap();
 
 const viewFor = (panel) => views.get(panel);
@@ -207,38 +233,22 @@ const Panel = Class({
       }));
     }
 
-    // Setup view
-    let viewOptions = {allowJavascript: !model.allow || (model.allow.script !== false)};
-    let view = domPanel.make(null, viewOptions);
-    panels.set(view, this);
-    views.set(this, view);
-
-    // Load panel content.
-    domPanel.setURL(view, model.contentURL);
-
-    // Allow context menu
-    domPanel.allowContextMenu(view, model.contextMenu);
+    optionsMap.set(this, options);
 
     // Setup listeners.
     setListeners(this, options);
-    let worker = new Worker(stripListeners(options));
-    workers.set(this, worker);
-
-    // pipe events from worker to a panel.
-    pipe(worker, this);
   },
   dispose: function dispose() {
-    this.hide();
+    if (views.has(this))
+      this.hide();
     off(this);
 
     workerFor(this).destroy();
     detach(styleFor(this));
 
-    domPanel.dispose(viewFor(this));
+    if (views.has(this))
+      domPanel.dispose(viewFor(this));
 
-    // Release circular reference between view and panel instance. This
-    // way view will be GC-ed. And panel as well once all the other refs
-    // will be removed from it.
     views.delete(this);
   },
   /* Public API: Panel.width */
@@ -301,6 +311,7 @@ const Panel = Class({
 
   /* Public API: Panel.show */
   show: function show(options={}, anchor) {
+    let view = viewFor(this);
     SinglePanelManager.requestOpen(this, () => {
       if (options instanceof Ci.nsIDOMElement) {
         [anchor, options] = [options, null];
@@ -315,7 +326,6 @@ const Panel = Class({
       }
 
       let model = modelFor(this);
-      let view = viewFor(this);
       let anchorView = getNodeView(anchor || options.position || model.position);
 
       options = merge({

addon-sdk/source/lib/sdk/system/events.js

@@ -166,16 +166,18 @@ on('sdk:loader:destroy', function onunload({ subject, data: reason }) {
     if (reason === 'shutdown')
       return;
 
-    stillAlive.forEach( (type, ref) => {
-      let observer = ref.get();
-      if (observer) {
-        if (wasShimmed.get(ref)) {
-          removeObserver(observer, type);
-        } else {
-          removeObserverNoShim(observer, type);
+    // Wait until the next tick to let other shutdown handlers finish first.
+    Promise.resolve().then(() => {
+      stillAlive.forEach((type, ref) => {
+        let observer = ref.get();
+        if (observer) {
+          if (wasShimmed.get(ref)) {
+            removeObserver(observer, type);
+          } else {
+            removeObserverNoShim(observer, type);
+          }
         }
-      }
-    })
+      });
+    });
   }
-  // a strong reference
 }, true, false);

addon-sdk/source/test/addons/chrome/main.js

@@ -9,6 +9,7 @@ const { WindowTracker } = require('sdk/deprecated/window-utils');
 const { close, open } = require('sdk/window/helpers');
 const { data } = require('sdk/self');
 const { Panel } = require('sdk/panel');
+const { getActiveView } = require("sdk/view/core");
 
 const XUL_URL = 'chrome://test/content/new-window.xul'
 
@@ -74,6 +75,8 @@ exports.testChromeInPanel = function*(assert) {
     contentScriptFile: data.url('panel.js')
   });
 
+  getActiveView(panel);
+
   yield new Promise(resolve => panel.port.once('start', resolve));
   assert.pass('start was emitted');
 

addon-sdk/source/test/test-addon-extras.js

@@ -20,6 +20,7 @@ exports["test changing result from addon extras in panel"] = function(assert, do
   });
 
   const { Panel } = loader.require("sdk/panel");
+  const { getActiveView } = loader.require("sdk/view/core");
   const { events } = loader.require("sdk/content/sandbox/events");
   const { on } = loader.require("sdk/event/core");
   const { isAddonContent } = loader.require("sdk/content/utils");
@@ -48,6 +49,9 @@ exports["test changing result from addon extras in panel"] = function(assert, do
     contentURL: "./test-addon-extras.html"
   });
 
+  // Force the panel view to actually load.
+  getActiveView(panel);
+
   panel.port.once("result1", (result) => {
     assert.equal(result, 1, "result is a number");
     result = true;

addon-sdk/source/test/test-panel.js

@@ -49,6 +49,7 @@ function makeEmptyPrivateBrowserWindow(options) {
 
 exports["test Panel"] = function(assert, done) {
   const { Panel } = require('sdk/panel');
+  const { getActiveView } = require("sdk/view/core");
 
   let panel = Panel({
     contentURL: "about:buildconfig",
@@ -68,10 +69,12 @@ exports["test Panel"] = function(assert, done) {
       }
     }
   });
+  getActiveView(panel);
 };
 
 exports["test Panel Emit"] = function(assert, done) {
   const { Panel } = require('sdk/panel');
+  const { getActiveView } = require("sdk/view/core");
 
   let panel = Panel({
     contentURL: "about:buildconfig",
@@ -88,10 +91,12 @@ exports["test Panel Emit"] = function(assert, done) {
     panel.destroy();
     done();
   });
+  getActiveView(panel);
 };
 
 exports["test Panel Emit Early"] = function(assert, done) {
   const { Panel } = require('sdk/panel');
+  const { getActiveView } = require("sdk/view/core");
 
   let panel = Panel({
     contentURL: "about:buildconfig",
@@ -104,10 +109,12 @@ exports["test Panel Emit Early"] = function(assert, done) {
     done();
   });
   panel.port.emit("addon-to-content");
+  getActiveView(panel);
 };
 
 exports["test Show Hide Panel"] = function(assert, done) {
   const { Panel } = require('sdk/panel');
+  let { getActiveView } = require('sdk/view/core');
 
   let panel = Panel({
     contentScript: "self.postMessage('')",
@@ -130,10 +137,12 @@ exports["test Show Hide Panel"] = function(assert, done) {
       done();
     }
   });
+  getActiveView(panel);
 };
 
 exports["test Document Reload"] = function(assert, done) {
   const { Panel } = require('sdk/panel');
+  const { getActiveView } = require("sdk/view/core");
 
   let url2 = "data:text/html;charset=utf-8,page2";
   let content =
@@ -166,6 +175,7 @@ exports["test Document Reload"] = function(assert, done) {
       }
     }
   });
+  getActiveView(panel);
   assert.pass('Panel was created');
 };
 
@@ -227,6 +237,7 @@ exports["test Parent Resize Hack"] = function(assert, done) {
 
 exports["test Resize Panel"] = function(assert, done) {
   const { Panel } = require('sdk/panel');
+  let { getActiveView } = require('sdk/view/core');
 
   // These tests fail on Linux if the browser window in which the panel
   // is displayed is not active.  And depending on what other tests have run
@@ -272,6 +283,7 @@ exports["test Resize Panel"] = function(assert, done) {
         done();
       }
     });
+    getActiveView(panel);
   }
 
   if (browserWindow === activeWindow) {
@@ -452,6 +464,7 @@ exports["test Panel Focus Not Set"] = function(assert, done) {
 
 exports["test Panel Text Color"] = function(assert, done) {
   const { Panel } = require('sdk/panel');
+  let { getActiveView } = require('sdk/view/core');
 
   let html = "<html><head><style>body {color: yellow}</style></head>" +
              "<body><p>Foo</p></body></html>";
@@ -467,11 +480,13 @@ exports["test Panel Text Color"] = function(assert, done) {
     panel.destroy();
     done();
   });
+  getActiveView(panel);
 };
 
 // Bug 866333
 exports["test watch event name"] = function(assert, done) {
   const { Panel } = require('sdk/panel');
+  let { getActiveView } = require('sdk/view/core');
 
   let html = "<html><head><style>body {color: yellow}</style></head>" +
              "<body><p>Foo</p></body></html>";
@@ -485,17 +500,21 @@ exports["test watch event name"] = function(assert, done) {
     panel.destroy();
     done();
   });
+  getActiveView(panel);
 }
 
 // Bug 696552: Ensure panel.contentURL modification support
 exports["test Change Content URL"] = function(assert, done) {
   const { Panel } = require('sdk/panel');
+  const { getActiveView } = require("sdk/view/core");
 
   let panel = Panel({
     contentURL: "about:blank",
     contentScript: "self.port.emit('ready', document.location.href);"
   });
 
+  getActiveView(panel);
+
   let count = 0;
   panel.port.on("ready", function (location) {
     count++;
@@ -625,6 +644,8 @@ exports["test ContentScriptOptions Option"] = function(assert, done) {
         done();
       }
     });
+  const { getActiveView } = loader.require("sdk/view/core");
+  getActiveView(panel);
 };
 
 exports["test console.log in Panel"] = function(assert, done) {

addon-sdk/source/test/test-system-events.js

@@ -119,7 +119,7 @@ exports["test listeners are GC-ed"] = function(assert, done) {
   });
 };
 
-exports["test alive listeners are removed on unload"] = function(assert) {
+exports["test alive listeners are removed on unload"] = function*(assert) {
   let receivedFromWeak = [];
   let receivedFromStrong = [];
   let loader = Loader(module);
@@ -137,6 +137,8 @@ exports["test alive listeners are removed on unload"] = function(assert) {
   assert.equal(receivedFromWeak.length, 1, "weak listener invoked");
 
   loader.unload();
+  // Give the cleanup code a chance to run.
+  yield Promise.resolve();
   events.emit(type, { data: 2 });
 
   assert.equal(receivedFromWeak.length, 1, "weak listener was removed");