Bug 1314861: Lazily create view for Panels. r?rpl draft
authorKris Maglione <maglione.k@gmail.com>
Sun, 30 Oct 2016 16:51:00 -0700
changeset 433091 6da48856fad901712b7463c2ba79c9c40fc219b1
parent 433090 e76089cff72c4c36f3f342082b70924c386c600d
child 433092 531f125b08319f3b53c34b7f40c156f7640e0695
push id34481
push usermaglione.k@gmail.com
push dateThu, 03 Nov 2016 03:45:04 +0000
reviewersrpl
bugs1314861
milestone52.0a1
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
addon-sdk/source/lib/sdk/panel.js
addon-sdk/source/lib/sdk/system/events.js
addon-sdk/source/test/test-addon-extras.js
addon-sdk/source/test/test-panel.js
addon-sdk/source/test/test-system-events.js
--- a/addon-sdk/source/lib/sdk/panel.js
+++ b/addon-sdk/source/lib/sdk/panel.js
@@ -11,17 +11,17 @@ module.metadata = {
     "Firefox": "*",
     "SeaMonkey": "*"
   }
 };
 
 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");
 const { WeakReference } = require('./core/reference');
 const { contract: loaderContract } = require("./content/loader");
 const { contract } = require("./util/contract");
 const { on, off, emit, setListeners } = require("./event/core");
 const { EventTarget } = require("./event/target");
@@ -94,20 +94,46 @@ function setScriptState(panel, value) {
   getDocShell(view.viewFrame).allowJavascript = value;
   view.setAttribute("sdkscriptenabled", "" + value);
 }
 
 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);
 const modelFor = (panel) => models.get(panel);
 const panelFor = (view) => panels.get(view);
 const workerFor = (panel) => workers.get(panel);
 const styleFor = (panel) => styles.get(panel);
 
@@ -202,48 +228,32 @@ const Panel = Class({
 
     if (model.contentStyle || model.contentStyleFile) {
       styles.set(this, Style({
         uri: model.contentStyleFile,
         source: model.contentStyle
       }));
     }
 
-    // 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 */
   get width() {
     return modelFor(this).width;
   },
   set width(value) {
     this.resize(value, this.height);
@@ -296,31 +306,31 @@ const Panel = Class({
 
   /* Public API: Panel.isShowing */
   get isShowing() {
     return !isDisposed(this) && domPanel.isOpen(viewFor(this));
   },
 
   /* 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];
       }
 
       if (anchor instanceof Ci.nsIDOMElement) {
         console.warn(
           "Passing a DOM node to Panel.show() method is an unsupported " +
           "feature that will be soon replaced. " +
           "See: https://bugzilla.mozilla.org/show_bug.cgi?id=878877"
         );
       }
 
       let model = modelFor(this);
-      let view = viewFor(this);
       let anchorView = getNodeView(anchor || options.position || model.position);
 
       options = merge({
         position: model.position,
         width: model.width,
         height: model.height,
         defaultWidth: model.defaultWidth,
         defaultHeight: model.defaultHeight,
--- a/addon-sdk/source/lib/sdk/system/events.js
+++ b/addon-sdk/source/lib/sdk/system/events.js
@@ -161,21 +161,23 @@ on('sdk:loader:destroy', function onunlo
   // using logic from ./unload, to avoid a circular module reference
   if (subject.wrappedJSObject === unloadSubject) {
     off('sdk:loader:destroy', onunload, false);
 
     // don't bother
     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);
+    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);
--- a/addon-sdk/source/test/test-addon-extras.js
+++ b/addon-sdk/source/test/test-addon-extras.js
@@ -15,16 +15,17 @@ exports["test changing result from addon
     modules: {
       "sdk/self": merge({}, self, {
         data: merge({}, self.data, {url: fixtures.url})
       })
     }
   });
 
   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");
 
   var result = 1;
   var extrasVal = {
     test: function() {
       return result;
@@ -43,16 +44,19 @@ exports["test changing result from addon
       assert.pass("content-script-before-inserted done!");
     }
   });
 
   let panel = Panel({
     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;
     panel.port.emit("get-result");
   });
 
   panel.port.once("result2", (result) => {
     assert.equal(result, true, "result is a boolean");
--- a/addon-sdk/source/test/test-panel.js
+++ b/addon-sdk/source/test/test-panel.js
@@ -44,16 +44,17 @@ function makeEmptyPrivateBrowserWindow(o
       toolbar: true,
       private: true
     }
   });
 }
 
 exports["test Panel"] = function(assert, done) {
   const { Panel } = require('sdk/panel');
+  const { getActiveView } = require("sdk/view/core");
 
   let panel = Panel({
     contentURL: "about:buildconfig",
     contentScript: "self.postMessage(1); self.on('message', () => self.postMessage(2));",
     onMessage: function (message) {
       assert.equal(this, panel, "The 'this' object is the panel.");
       switch(message) {
         case 1:
@@ -63,56 +64,62 @@ exports["test Panel"] = function(assert,
         case 2:
           assert.pass("The panel posted a message and received a response.");
           panel.destroy();
           done();
           break;
       }
     }
   });
+  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",
     contentScript: "self.port.emit('loaded');" +
                    "self.port.on('addon-to-content', " +
                    "             () => self.port.emit('received'));",
   });
   panel.port.on("loaded", function () {
     assert.pass("The panel was loaded and sent a first event.");
     panel.port.emit("addon-to-content");
   });
   panel.port.on("received", function () {
     assert.pass("The panel posted a message and received a response.");
     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",
     contentScript: "self.port.on('addon-to-content', " +
                    "             () => self.port.emit('received'));",
   });
   panel.port.on("received", function () {
     assert.pass("The panel posted a message early and received a response.");
     panel.destroy();
     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('')",
     contentScriptWhen: "end",
     contentURL: "data:text/html;charset=utf-8,",
     onMessage: function (message) {
       panel.show();
     },
@@ -125,20 +132,22 @@ exports["test Show Hide Panel"] = functi
     onHide: function () {
       assert.pass("The panel was hidden.");
       assert.equal(this, panel, "The 'this' object is the panel.");
       assert.equal(this.isShowing, false, "panel.isShowing == false.");
       panel.destroy();
       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 =
     "<script>" +
     "window.addEventListener('message', function({ data }) {"+
     "  if (data == 'move') window.location = '" + url2 + "';" +
     '}, false);' +
     "</script>";
@@ -161,16 +170,17 @@ exports["test Document Reload"] = functi
       }
       else if (messageCount == 2) {
         assert.equal(message, url2, "Second document too; " + message);
         panel.destroy();
         done();
       }
     }
   });
+  getActiveView(panel);
   assert.pass('Panel was created');
 };
 
 // Test disabled because of bug 910230
 /*
 exports["test Parent Resize Hack"] = function(assert, done) {
   const { Panel } = require('sdk/panel');
 
@@ -222,16 +232,17 @@ exports["test Parent Resize Hack"] = fun
   });
 
   panel.show();
 }
 */
 
 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
   // before this one, it might not be (the untitled window in which the test
   // runner executes is often active).  So we make sure the browser window
   // is focused by focusing it before running the tests.  Then, to be the best
   // possible test citizen, we refocus whatever window was focused before we
   // started running these tests.
@@ -267,16 +278,17 @@ exports["test Resize Panel"] = function(
       onHide: function () {
         assert.ok((panel.width == 100) && (panel.height == 100),
           "The panel was resized.");
         if (activeWindow)
           activeWindow.focus();
         done();
       }
     });
+    getActiveView(panel);
   }
 
   if (browserWindow === activeWindow) {
     onFocus();
   }
   else {
     browserWindow.addEventListener("focus", onFocus, true);
     browserWindow.focus();
@@ -447,60 +459,67 @@ exports["test Panel Focus Not Set"] = fu
       done();
     }
   });
   panel.show();
 };
 
 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>";
   let panel = Panel({
     contentURL: "data:text/html;charset=utf-8," + encodeURI(html),
     contentScript: "self.port.emit('color', " +
                    "window.getComputedStyle(document.body.firstChild, null). " +
                    "       getPropertyValue('color'));"
   });
   panel.port.on("color", function (color) {
     assert.equal(color, "rgb(255, 255, 0)",
       "The panel text color style is preserved when a style exists.");
     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>";
 
   let panel = Panel({
     contentURL: "data:text/html;charset=utf-8," + encodeURI(html),
     contentScript: "self.port.emit('watch', 'test');"
   });
   panel.port.on("watch", function (msg) {
     assert.equal(msg, "test", 'watch event name works');
     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++;
     if (count == 1) {
       assert.equal(location, "about:blank");
       assert.equal(panel.contentURL, "about:blank");
       panel.contentURL = "about:buildconfig";
     }
@@ -620,16 +639,18 @@ exports["test ContentScriptOptions Optio
         assert.equal( msg[0], 'undefined', 'functions are stripped from contentScriptOptions' );
         assert.equal( typeof msg[1], 'object', 'object as contentScriptOptions' );
         assert.equal( msg[1].a, true, 'boolean in contentScriptOptions' );
         assert.equal( msg[1].b.join(), '1,2,3', 'array and numbers in contentScriptOptions' );
         assert.equal( msg[1].c, 'string', 'string in contentScriptOptions' );
         done();
       }
     });
+  const { getActiveView } = loader.require("sdk/view/core");
+  getActiveView(panel);
 };
 
 exports["test console.log in Panel"] = function(assert, done) {
   let text = 'console.log() in Panel works!';
   let html = '<script>onload = function log(){\
                 console.log("' + text + '");\
               }</script>';
 
--- a/addon-sdk/source/test/test-system-events.js
+++ b/addon-sdk/source/test/test-system-events.js
@@ -114,17 +114,17 @@ exports["test listeners are GC-ed"] = fu
     assert.equal(receivedFromWeak.length, 1, "weak listener was GC-ed");
     assert.equal(receivedFromStrong.length, 2, "strong listener was invoked");
 
     loader.unload();
     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);
   let events = loader.require('sdk/system/events');
 
   let type = 'test-alive-listeners-are-removed';
   const handler = (event) => receivedFromStrong.push(event);
   const weakHandler = (event) => receivedFromWeak.push(event);
@@ -132,16 +132,17 @@ exports["test alive listeners are remove
   events.on(type, handler, true);
   events.on(type, weakHandler);
 
   events.emit(type, { data: 1 });
   assert.equal(receivedFromStrong.length, 1, "strong listener invoked");
   assert.equal(receivedFromWeak.length, 1, "weak listener invoked");
 
   loader.unload();
+  yield Promise.resolve();
   events.emit(type, { data: 2 });
 
   assert.equal(receivedFromWeak.length, 1, "weak listener was removed");
   assert.equal(receivedFromStrong.length, 1, "strong listener was removed");
 };
 
 exports["test handle nsIObserverService notifications"] = function(assert) {
   let ios = Cc['@mozilla.org/network/io-service;1']