Skip to content
This repository was archived by the owner on Jan 17, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions addon/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ Mustache.parse(templates.experimentList);

const Metrics = require('./lib/metrics');
const survey = require('./lib/survey');
const WebExtensionChannels = require('./lib/webextension-channels');

const PANEL_WIDTH = 300;
const FOOTER_HEIGHT = 50;
Expand Down Expand Up @@ -233,8 +234,8 @@ panel.on('show', showExperimentList);
panel.port.on('back', showExperimentList);

function showExperimentList() {
panel.port.emit('show', getExperimentList(store.availableExperiments,
store.installedAddons));
panel.port.emit('show', getExperimentList(store.availableExperiments || {},
store.installedAddons || {}));
}

panel.port.on('link', url => {
Expand Down Expand Up @@ -475,6 +476,7 @@ const addonListener = {
version: addon.version
});
Metrics.experimentEnabled(addon.id);
WebExtensionChannels.updateExperimentChannels();
}
},
onDisabled: function(addon) {
Expand All @@ -486,6 +488,7 @@ const addonListener = {
version: addon.version
});
Metrics.experimentDisabled(addon.id);
WebExtensionChannels.updateExperimentChannels();
}
},
onUninstalling: function(addon) {
Expand All @@ -511,6 +514,7 @@ const addonListener = {
}

Metrics.experimentDisabled(addon.id);
WebExtensionChannels.updateExperimentChannels();
}
}
};
Expand All @@ -525,6 +529,7 @@ const installListener = {
formatInstallData(install, addon), addon);
});
Metrics.experimentEnabled(addon.id);
WebExtensionChannels.updateExperimentChannels();
},
onInstallFailed: function(install) {
app.send('addon-install:install-failed', formatInstallData(install));
Expand Down Expand Up @@ -571,6 +576,7 @@ exports.main = function(options) {

initServerEnvironmentPreference();
Metrics.init();
WebExtensionChannels.init();
};

exports.onUnload = function(reason) {
Expand All @@ -579,6 +585,7 @@ exports.onUnload = function(reason) {
panel.destroy();
button.destroy();
Metrics.destroy();
WebExtensionChannels.destroy();

if (reason === 'uninstall' || reason === 'disable') {
Metrics.onDisable();
Expand Down
171 changes: 171 additions & 0 deletions addon/lib/webextension-channels.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
/*
* This Source Code is subject to the terms of the Mozilla Public License
* version 2.0 (the 'License'). You can obtain a copy of the License at
* http://mozilla.org/MPL/2.0/.
*/

/* global XPCOMUtils, Services */

const { Ci, Cu } = require('chrome');
const { Class } = require('sdk/core/heritage');
const { Disposable } = require('sdk/core/disposable');

const store = require('sdk/simple-storage').storage;

const Metrics = require('./metrics');

const TESTPILOT_TELEMETRY_CHANNEL = 'testpilot-telemetry';

Cu.import('resource://gre/modules/XPCOMUtils.jsm');

XPCOMUtils.defineLazyModuleGetter(this, 'Services',
'resource://gre/modules/Services.jsm');

const {getExtensionUUID} = Cu.import('resource://gre/modules/Extension.jsm', {});

function createChannelForAddonId(name, addonId) {
// The BroadcastChannel API allows messaging between different windows that
// share the same origin. Bug 1186732 extended this to WebExtensions (which
// may not have an origin) by adding a special URL that loads an about:blank
// page at the (generalized) "origin" of the extension.
//
// Load that about:blank page, and use its `window` to get a BroadcastChannel
// that allows two-way communication between the main Test Pilot extension and
// individual experiment extensions.

// Note: the `targetExtensionUUID` is different for each copy of Firefox,
// so that malicious websites can't guess the special URL associated with
// an extension.
const targetExtensionUUID = getExtensionUUID(addonId);

// Create the special about:blank URL for the extension.
const baseURI = Services.io
.newURI(`moz-extension://${targetExtensionUUID}/_blank.html`, null, null);

// Create a principal (security context) for the generalized origin given
// by the extension's special URL and its `addonId`.
const principal = Services.scriptSecurityManager
.createCodebasePrincipal(baseURI, { addonId });

// Create a hidden window and open the special about:blank page for the
// extension.
const addonChromeWebNav = Services.appShell.createWindowlessBrowser(true);
const docShell = addonChromeWebNav.QueryInterface(Ci.nsIInterfaceRequestor) // eslint-disable-line new-cap
.getInterface(Ci.nsIDocShell);
docShell.createAboutBlankContentViewer(principal);
const window = docShell.contentViewer.DOMDocument.defaultView;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lmorchard Is this (lines 28-38) the best way to get a DOM window pointer for the given moz-extension URL? I guess I'm wondering if you found this snippet somewhere, or if it was suggested by someone on the FF team.

I asked about safely getting a DOM Window in #fx-team but got no response. I'm wondering if there might be a simpler or safer way--usually Gecko has N ways to do something, but only 1-3 ways that are considered good ways. I'll look further into it.

Copy link
Copy Markdown
Contributor Author

@lmorchard lmorchard Jul 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@6a68 This is mostly not my code, it comes from experiments with BroadcastChannel over here. Honestly, I didn't really know you could get a DOM window pointer for another origin from add-on code :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, I didn't really know you could get a DOM window pointer for another origin from add-on code :)

@lmorchard Ah yeah, once you require('chrome') you effectively have rootly powers, and a lot of weird stuff becomes possible ^_^

After looking at this snippet for a while, and reading through https://bugzil.la/1186732, I'm much less scared of it. But it might be nice to document it for the sake of our future selves. Here's an attempt, take whatever seems useful, or skip it if it seems to make things more confusing overall:

function createChannelForAddonId(name, addonId) {
  // The BroadcastChannel API allows messaging between different windows that
  // share the same origin. Bug 1186732 extended this to WebExtensions (which
  // may not have an origin) by adding a special URL that loads an about:blank
  // page at the (generalized) "origin" of the extension.
  //
  // Load that about:blank page, and use its `window` to get a BroadcastChannel
  // that allows two-way communication between the main Test Pilot extension and
  // individual experiment extensions. 

  // Note: the `targetExtensionUUID` is different for each copy of Firefox,
  // so that malicious websites can't guess the special URL associated with
  // an extension. 
  const targetExtensionUUID = getExtensionUUID(addonId);

  // Create the special about:blank URL for the extension.
  const baseURI = Services.io
        .newURI(`moz-extension://${targetExtensionUUID}/_blank.html`, null, null);

  // Create a principal (security context) for the generalized origin given
  // by the extension's special URL and its `addonId`.
  const principal = Services.scriptSecurityManager
        .createCodebasePrincipal(baseURI, { addonId });

  // Create a hidden window and open the special about:blank page for the
  // extension.
  const addonChromeWebNav = Services.appShell.createWindowlessBrowser(true);
  const docShell = addonChromeWebNav.QueryInterface(Ci.nsIInterfaceRequestor) // eslint-disable-line new-cap
        .getInterface(Ci.nsIDocShell);
  docShell.createAboutBlankContentViewer(principal);
  const window = docShell.contentViewer.DOMDocument.defaultView;

  // Finally, get the BroadcastChannel associated with the extension.
  const addonBroadcastChannel = new window.BroadcastChannel(name);

  // Callers need to keep the pointer to the window, otherwise the window's 
  // BroadcastChannel will get garbage collected.
  return {
    addonChromeWebNav,
    addonBroadcastChannel
  };
} 

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think I'll just plonk that on in - verbose commenting of this magic is a great addition, thanks!


// Finally, get the BroadcastChannel associated with the extension.
const addonBroadcastChannel = new window.BroadcastChannel(name);

// Callers need to keep the pointer to the window, otherwise the window's
// BroadcastChannel will get garbage collected.
return {
addonChromeWebNav,
addonBroadcastChannel
};
}

const WebExtensionChannel = Class({ // eslint-disable-line new-cap
implements: [Disposable],

initialize(targetAddonId) {
this.pingListeners = new Set();

this.targetAddonId = targetAddonId;

const {
addonChromeWebNav,
addonBroadcastChannel
} = createChannelForAddonId(TESTPILOT_TELEMETRY_CHANNEL, targetAddonId);

// NOTE: Keep a ref to prevent it from going away during garbage collection
// (or the BroadcastChannel will stop working).
this.addonChromeWebNav = addonChromeWebNav;
this.addonBroadcastChannel = addonBroadcastChannel;

this.handleEventBound = ev => this.handleEvent(ev);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems weird to me. I'll look into it this evening.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could probably just do this.handleEvent.bind(this), but thought this looked clearer. I think the add/remove event methods need a consistent bound function ref to deal with. Though I did think handleEvent and passing this was enough.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, there's probably something fishy here: the BroadcastChannel implementation of add/removeEventListener just wraps the default implementation. That said, it does work and it's readable, so I think we're fine to just leave it and move on.

},

dispose() {
this.addonBroadcastChannel.removeEventListener('message', this.handleEventBound);
this.addonBroadcastChannel.close();
this.addonChromeWebNav.close();
this.pingListeners.clear();

this.addonBroadcastChannel = null;
this.addonChromeWebNav = null;
},

registerPingListener(callback) {
this.pingListeners.add(callback);

if (this.pingListeners.size >= 0) {
this.addonBroadcastChannel.addEventListener('message', this.handleEventBound);
}
},

unregisterPingListener(callback) {
this.pingListeners.delete(callback);

if (this.pingListeners.size === 0) {
this.addonBroadcastChannel.removeEventListener('message', this.handleEventBound);
}
},

handleEvent(event) {
if (event.data) {
this.notifyPing(event.data, {addonId: this.targetAddonId});
}
},

notifyPing(data, sender) {
for (let pingListener of this.pingListeners) { // eslint-disable-line prefer-const
try {
pingListener({
senderAddonId: sender.addonId,
testpilotPingData: data
});
} catch (err) {
console.error('Error executing pingListener', err); // eslint-disable-line no-console
}
}
}
});

let channels = {};

module.exports = {
WebExtensionChannel,

// Update all the channels on init.
init() {
this.updateExperimentChannels();
},

// Drop refs to channels for garbage collection
destroy() {
channels = {};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Continuing with the theme of this review, I'm not sure if this will free the channels for GC, how sure are you feeling about this / have you asked anybody on the DOM team about it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand it right, the WebExtensionChannel is also a Disposable, so dispose gets called on GC. It's not actually a BroadcastChannel but it does contain one. I just added a .close() call in dispose - so I think that should cover it?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, nice! I skimmed right past the mention of Disposable, which I wasn't familiar with. 👍

},

// Rebuild channels for all known experiments
updateExperimentChannels() {
channels = {};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question as line 124

if (store.installedAddons) {
Object.keys(store.installedAddons).forEach(id => {
const channel = new WebExtensionChannel(id);
channels[id] = channel;
channel.registerPingListener(data =>
this.handleWebExtensionPing(id, data));
});
}
},

// Pass a ping message along to Telemetry via Metrics
handleWebExtensionPing(id, data) {
Metrics.onExperimentPing({
subject: id,
data: JSON.stringify(data)
});
}
};
12 changes: 12 additions & 0 deletions docs/examples/webextension/background.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
var TESTPILOT_TELEMETRY_CHANNEL = 'testpilot-telemetry';
var testpilotPingChannel = new BroadcastChannel(TESTPILOT_TELEMETRY_CHANNEL);
setInterval(function () {
testpilotPingChannel.postMessage({
boolData: true,
arrayOfData: ["one", "two", "three"],
nestedData: {
intData: 10,
},
});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use some elaboration on what the postMessage payload should contain. Just a JSON blob of data? Any required properties?

Copy link
Copy Markdown
Contributor Author

@lmorchard lmorchard Jun 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the other mechanism - just a JS object with what you want included in the payload of the Telemetry ping. Test Pilot itself is agnostic on this, so it's more a concern between you & the metrics team to spec out.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is to say, what you send here is the ... in the example ping shown in README-METRICS.md for the testpilottest ping - Test Pilot wraps that in the other envelope info and the full client environment data.

console.log("TEST PILOT PING SENT", Date.now());
}, 5000);
19 changes: 19 additions & 0 deletions docs/examples/webextension/manifest.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"manifest_version": 2,
"name": "Test Pilot WebExtension Example",
"version": "1.0",
"description": "This is a WebExtension built as an example Test Pilot experiment",
"icons": {
"32": "icons/icon-32.png"
},
"permissions": ["background"],
"applications": {
"gecko": {
"id": "testpilotexample1@mozilla.org",
"strict_min_version": "45.0"
}
},
"background": {
"scripts": ["background.js"]
}
}