Bug 1397452 - Remove all SDK compatiblity code from DevTools. r=jdescottes draft
authorAlexandre Poirot <poirot.alex@gmail.com>
Mon, 25 Sep 2017 16:04:26 +0200
changeset 669927 d0bb81a6d9ea48c2aab97b39675fb04a06e56b7b
parent 669567 877c02446745576ac97f181dd9eeec78ef605451
child 670145 4fa1bf7c869fdb1caed35003d10fc09931ba9681
push id81470
push userbmo:poirot.alex@gmail.com
push dateMon, 25 Sep 2017 16:57:03 +0000
reviewersjdescottes
bugs1397452
milestone58.0a1
Bug 1397452 - Remove all SDK compatiblity code from DevTools. r=jdescottes MozReview-Commit-ID: IRhZeIS97cy
devtools/client/framework/devtools.js
devtools/client/framework/test/browser.ini
devtools/client/framework/test/browser_devtools_shim.js
devtools/server/actors/tab.js
devtools/shim/DevToolsShim.jsm
devtools/shim/tests/unit/test_devtools_shim.js
--- a/devtools/client/framework/devtools.js
+++ b/devtools/client/framework/devtools.js
@@ -13,21 +13,16 @@ const {DevToolsShim} = Cu.import("chrome
 loader.lazyRequireGetter(this, "TargetFactory", "devtools/client/framework/target", true);
 loader.lazyRequireGetter(this, "TabTarget", "devtools/client/framework/target", true);
 loader.lazyRequireGetter(this, "Toolbox", "devtools/client/framework/toolbox", true);
 loader.lazyRequireGetter(this, "ToolboxHostManager", "devtools/client/framework/toolbox-host-manager", true);
 loader.lazyRequireGetter(this, "gDevToolsBrowser", "devtools/client/framework/devtools-browser", true);
 loader.lazyRequireGetter(this, "HUDService", "devtools/client/webconsole/hudservice", true);
 loader.lazyImporter(this, "ScratchpadManager", "resource://devtools/client/scratchpad/scratchpad-manager.jsm");
 
-// Dependencies required for addon sdk compatibility layer.
-loader.lazyRequireGetter(this, "DebuggerServer", "devtools/server/main", true);
-loader.lazyRequireGetter(this, "DebuggerClient", "devtools/shared/client/main", true);
-loader.lazyImporter(this, "BrowserToolboxProcess", "resource://devtools/client/framework/ToolboxProcess.jsm");
-
 loader.lazyRequireGetter(this, "WebExtensionInspectedWindowFront",
       "devtools/shared/fronts/webextension-inspected-window", true);
 
 const {defaultTools: DefaultTools, defaultThemes: DefaultThemes} =
   require("devtools/client/definitions");
 const EventEmitter = require("devtools/shared/old-event-emitter");
 const AboutDevTools = require("devtools/client/framework/about-devtools-toolbox");
 const {Task} = require("devtools/shared/task");
@@ -543,52 +538,16 @@ DevTools.prototype = {
    *
    * @return {TabTarget} A target object
    */
   getTargetForTab: function (tab) {
     return TargetFactory.forTab(tab);
   },
 
   /**
-   * Compatibility layer for addon-sdk. Remove when Firefox 57 hits release.
-   * Initialize the debugger server if needed and and create a connection.
-   *
-   * @return {DebuggerTransport} a client-side DebuggerTransport for communicating with
-   *         the created connection.
-   */
-  connectDebuggerServer: function () {
-    if (!DebuggerServer.initialized) {
-      DebuggerServer.init();
-      DebuggerServer.addBrowserActors();
-    }
-
-    return DebuggerServer.connectPipe();
-  },
-
-  /**
-   * Compatibility layer for addon-sdk. Remove when Firefox 57 hits release.
-   *
-   * Create a connection to the debugger server and return a debugger client for this
-   * new connection.
-   */
-  createDebuggerClient: function () {
-    let transport = this.connectDebuggerServer();
-    return new DebuggerClient(transport);
-  },
-
-  /**
-   * Compatibility layer for addon-sdk. Remove when Firefox 57 hits release.
-   *
-   * Create a BrowserToolbox process linked to the provided addon id.
-   */
-  initBrowserToolboxProcessForAddon: function (addonID) {
-    BrowserToolboxProcess.init({ addonID });
-  },
-
-  /**
    * Compatibility layer for web-extensions. Used by DevToolsShim for
    * browser/components/extensions/ext-devtools.js
    *
    * web-extensions need to use dedicated instances of TabTarget and cannot reuse the
    * cached instances managed by DevTools target factory.
    */
   createTargetForTab: function (tab) {
     return new TabTarget(tab);
--- a/devtools/client/framework/test/browser.ini
+++ b/devtools/client/framework/test/browser.ini
@@ -47,17 +47,16 @@ support-files =
   test_browser_toolbox_debugger.js
   !/devtools/client/debugger/new/test/mochitest/head.js
 
 [browser_browser_toolbox.js]
 [browser_browser_toolbox_debugger.js]
 skip-if = debug # Bug 1282269
 [browser_devtools_api.js]
 [browser_devtools_api_destroy.js]
-[browser_devtools_shim.js]
 [browser_dynamic_tool_enabling.js]
 [browser_ignore_toolbox_network_requests.js]
 [browser_keybindings_01.js]
 [browser_keybindings_02.js]
 [browser_keybindings_03.js]
 [browser_menu_api.js]
 [browser_new_activation_workflow.js]
 [browser_source_map-01.js]
deleted file mode 100644
--- a/devtools/client/framework/test/browser_devtools_shim.js
+++ /dev/null
@@ -1,97 +0,0 @@
-/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
-/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
-/* Any copyright is dedicated to the Public Domain.
- * http://creativecommons.org/publicdomain/zero/1.0/ */
-
-"use strict";
-
-// Tests DevToolsShim API works as expected when DevTools are available.
-
-const TOOL_ID = "test-tool";
-
-const { DevToolsShim } =
-    Components.utils.import("chrome://devtools-shim/content/DevToolsShim.jsm", {});
-
-add_task(function* () {
-  yield addTab("about:blank");
-
-  yield testThemeRegistrationWithShim();
-  yield testToolRegistrationWithShim();
-
-  gBrowser.removeCurrentTab();
-});
-
-// Test that theme registration works with the DevToolsShim.
-function* testThemeRegistrationWithShim() {
-  let themeId = yield new Promise(resolve => {
-    DevToolsShim.on("theme-registered", function onThemeRegistered(e, id) {
-      resolve(id);
-    });
-
-    DevToolsShim.registerTheme({
-      id: "test-theme",
-      label: "Test theme",
-      stylesheets: [CHROME_URL_ROOT + "doc_theme.css"],
-      classList: ["theme-test"],
-    });
-  });
-
-  is(themeId, "test-theme", "theme-registered event handler sent theme id");
-
-  ok(gDevTools.getThemeDefinitionMap().has(themeId), "theme added to map");
-  DevToolsShim.unregisterTheme("test-theme");
-  ok(!gDevTools.getThemeDefinitionMap().has(themeId), "theme removed");
-}
-
-// Test that tool registration works with the DevToolsShim.
-function* testToolRegistrationWithShim() {
-  let toolDefinition = {
-    id: TOOL_ID,
-    isTargetSupported: () => true,
-    visibilityswitch: "devtools.test-tool.enabled",
-    url: "about:blank",
-    label: "someLabel",
-    build: function (iframeWindow, toolbox) {
-      let panel = createTestPanel(iframeWindow, toolbox);
-      return panel.open();
-    },
-  };
-
-  // Check that tool registration works when using the DevToolsShim.
-  ok(!gDevTools.getToolDefinitionMap().has(TOOL_ID), "The tool is not registered");
-  DevToolsShim.registerTool(toolDefinition);
-  ok(gDevTools.getToolDefinitionMap().has(TOOL_ID), "The tool is registered");
-
-  let events = {};
-
-  // Check that events can be listened to on the shim in the same way as on gDevTools
-  DevToolsShim.on(TOOL_ID + "-init", function onTool1Init(event, toolbox, iframe) {
-    DevToolsShim.off(TOOL_ID + "-init", onTool1Init);
-    ok(toolbox, "toolbox argument available");
-    ok(iframe, "iframe argument available");
-    events.init = true;
-  });
-
-  DevToolsShim.on(TOOL_ID + "-ready", function onToolReady(event, toolbox, iframe) {
-    DevToolsShim.off(TOOL_ID + "-ready", onToolReady);
-    ok(toolbox, "toolbox argument available");
-    ok(iframe, "iframe argument available");
-    events.ready = true;
-  });
-
-  let target = TargetFactory.forTab(gBrowser.selectedTab);
-  let toolbox = yield gDevTools.showToolbox(target, TOOL_ID);
-
-  // init & ready events should have been fired when opening the toolbox.
-  ok(events.init, "init event fired");
-  ok(events.ready, "ready event fired");
-
-  ok(gDevTools.getToolDefinitionMap().has(TOOL_ID), "The tool is still registered");
-  DevToolsShim.unregisterTool(TOOL_ID);
-  ok(!gDevTools.getToolDefinitionMap().has(TOOL_ID), "The tool is no longer registered");
-
-  // Wait for toolbox select event after unregistering the currently selected tool.
-  yield toolbox.once("select");
-
-  yield toolbox.destroy();
-}
--- a/devtools/server/actors/tab.js
+++ b/devtools/server/actors/tab.js
@@ -527,16 +527,17 @@ TabActor.prototype = {
    */
   _shouldAddNewGlobalAsDebuggee(wrappedGlobal) {
     if (wrappedGlobal.hostAnnotations &&
         wrappedGlobal.hostAnnotations.type == "document" &&
         wrappedGlobal.hostAnnotations.element === this.window) {
       return true;
     }
 
+    // Otherwise, check if it is a WebExtension content script sandbox
     let global = unwrapDebuggerObjectGlobal(wrappedGlobal);
     if (!global) {
       return false;
     }
 
     // Check if the global is a sdk page-mod sandbox.
     let metadata = {};
     let id = "";
--- a/devtools/shim/DevToolsShim.jsm
+++ b/devtools/shim/DevToolsShim.jsm
@@ -28,28 +28,25 @@ function removeItem(array, callback) {
 /**
  * The DevToolsShim is a part of the DevTools go faster project, which moves the Firefox
  * DevTools outside of mozilla-central to an add-on. It aims to bridge the gap for
  * existing mozilla-central code that still needs to interact with DevTools (such as
  * web-extensions).
  *
  * DevToolsShim is a singleton that provides a set of helpers to interact with DevTools,
  * that work whether the DevTools addon is installed or not. It can be used to start
- * listening to events, register tools, themes. As soon as a DevTools addon is installed
- * the DevToolsShim will forward all the requests received until then to the real DevTools
- * instance.
+ * listening to events. As soon as a DevTools addon is installed the DevToolsShim will
+ * forward all the requests received until then to the real DevTools instance.
  *
  * DevToolsShim.isInstalled() can also be used to know if DevTools are currently
  * installed.
  */
 this.DevToolsShim = {
   _gDevTools: null,
   listeners: [],
-  tools: [],
-  themes: [],
 
   /**
    * Lazy getter for the `gDevTools` instance. Should only be called when users interacts
    * with DevTools as it will force loading them.
    *
    * @return {DevTools} a devtools instance (from client/framework/devtools)
    */
   get gDevTools() {
@@ -105,20 +102,16 @@ this.DevToolsShim = {
       this._gDevTools = null;
     }
   },
 
   /**
    * The following methods can be called before DevTools are initialized:
    * - on
    * - off
-   * - registerTool
-   * - unregisterTool
-   * - registerTheme
-   * - unregisterTheme
    *
    * If DevTools are not initialized when calling the method, DevToolsShim will call the
    * appropriate method as soon as a gDevTools instance is registered.
    */
 
   /**
    * This method is used by browser/components/extensions/ext-devtools.js for the events:
    * - toolbox-created
@@ -140,64 +133,16 @@ this.DevToolsShim = {
     if (this.isInitialized()) {
       this._gDevTools.off(event, listener);
     } else {
       removeItem(this.listeners, ([e, l]) => e === event && l === listener);
     }
   },
 
   /**
-   * This method is only used by the addon-sdk and should be removed when Firefox 56 is
-   * no longer supported.
-   */
-  registerTool: function (tool) {
-    if (this.isInitialized()) {
-      this._gDevTools.registerTool(tool);
-    } else {
-      this.tools.push(tool);
-    }
-  },
-
-  /**
-   * This method is only used by the addon-sdk and should be removed when Firefox 56 is
-   * no longer supported.
-   */
-  unregisterTool: function (tool) {
-    if (this.isInitialized()) {
-      this._gDevTools.unregisterTool(tool);
-    } else {
-      removeItem(this.tools, t => t === tool);
-    }
-  },
-
-  /**
-   * This method is only used by the addon-sdk and should be removed when Firefox 56 is
-   * no longer supported.
-   */
-  registerTheme: function (theme) {
-    if (this.isInitialized()) {
-      this._gDevTools.registerTheme(theme);
-    } else {
-      this.themes.push(theme);
-    }
-  },
-
-  /**
-   * This method is only used by the addon-sdk and should be removed when Firefox 56 is
-   * no longer supported.
-   */
-  unregisterTheme: function (theme) {
-    if (this.isInitialized()) {
-      this._gDevTools.unregisterTheme(theme);
-    } else {
-      removeItem(this.themes, t => t === theme);
-    }
-  },
-
-  /**
    * Called from SessionStore.jsm in mozilla-central when saving the current state.
    *
    * @param {Object} state
    *                 A SessionStore state object that gets modified by reference
    */
   saveDevToolsSession: function (state) {
     if (!this.isInitialized()) {
       return;
@@ -256,57 +201,31 @@ this.DevToolsShim = {
   },
 
   _onDevToolsRegistered: function () {
     // Register all pending event listeners on the real gDevTools object.
     for (let [event, listener] of this.listeners) {
       this._gDevTools.on(event, listener);
     }
 
-    for (let tool of this.tools) {
-      this._gDevTools.registerTool(tool);
-    }
-
-    for (let theme of this.themes) {
-      this._gDevTools.registerTheme(theme);
-    }
-
     this.listeners = [];
-    this.tools = [];
-    this.themes = [];
   },
 };
 
 /**
- * Compatibility layer for addon-sdk. Remove when Firefox 57 hits release.
- *
- * The methods below are used by classes and tests from addon-sdk/
- * If DevTools are not installed when calling one of them, the call will throw.
- */
-
-let addonSdkMethods = [
-  "closeToolbox",
-  "connectDebuggerServer",
-  "createDebuggerClient",
-  "getToolbox",
-  "initBrowserToolboxProcessForAddon",
-  "showToolbox",
-];
-
-/**
  * Compatibility layer for webextensions.
  *
  * Those methods are called only after a DevTools webextension was loaded in DevTools,
  * therefore DevTools should always be available when they are called.
  */
 let webExtensionsMethods = [
   "createTargetForTab",
   "createWebExtensionInspectedWindowFront",
   "getTargetForTab",
   "getTheme",
   "openBrowserConsole",
 ];
 
-for (let method of [...addonSdkMethods, ...webExtensionsMethods]) {
+for (let method of webExtensionsMethods) {
   this.DevToolsShim[method] = function () {
     return this.gDevTools[method].apply(this.gDevTools, arguments);
   };
 }
--- a/devtools/shim/tests/unit/test_devtools_shim.js
+++ b/devtools/shim/tests/unit/test_devtools_shim.js
@@ -12,20 +12,16 @@ const { DevToolsShim } =
 /**
  * Create a mocked version of DevTools that records all calls made to methods expected
  * to be called by DevToolsShim.
  */
 function createMockDevTools() {
   let methods = [
     "on",
     "off",
-    "registerTool",
-    "registerTheme",
-    "unregisterTool",
-    "unregisterTheme",
     "emit",
     "saveDevToolsSession",
     "restoreDevToolsSession",
   ];
 
   let mock = {
     callLog: {}
   };
@@ -134,82 +130,16 @@ function test_off_called_before_with_bad
   // on should still be called
   checkCalls(mock, "on", 1, ["test_event", cb1]);
   // Calls to off should not be held and forwarded.
   checkCalls(mock, "off", 0);
 
   restoreDevToolsInstalled();
 }
 
-function test_registering_tool() {
-  mockDevToolsInstalled(true);
-
-  ok(!DevToolsShim.isInitialized(), "DevTools are not initialized");
-
-  let tool1 = {};
-  let tool2 = {};
-  let tool3 = {};
-  let mock = createMockDevTools();
-
-  // Pre-register tool1
-  DevToolsShim.registerTool(tool1);
-
-  // Pre-register tool3, but unregister right after
-  DevToolsShim.registerTool(tool3);
-  DevToolsShim.unregisterTool(tool3);
-
-  DevToolsShim.register(mock);
-  checkCalls(mock, "registerTool", 1, [tool1]);
-
-  DevToolsShim.registerTool(tool2);
-  checkCalls(mock, "registerTool", 2, [tool2]);
-
-  DevToolsShim.unregister();
-
-  // Create a new mock and check the tools are not added once again.
-  mock = createMockDevTools();
-  DevToolsShim.register(mock);
-  checkCalls(mock, "registerTool", 0);
-
-  restoreDevToolsInstalled();
-}
-
-function test_registering_theme() {
-  mockDevToolsInstalled(true);
-
-  ok(!DevToolsShim.isInitialized(), "DevTools are not initialized");
-
-  let theme1 = {};
-  let theme2 = {};
-  let theme3 = {};
-  let mock = createMockDevTools();
-
-  // Pre-register theme1
-  DevToolsShim.registerTheme(theme1);
-
-  // Pre-register theme3, but unregister right after
-  DevToolsShim.registerTheme(theme3);
-  DevToolsShim.unregisterTheme(theme3);
-
-  DevToolsShim.register(mock);
-  checkCalls(mock, "registerTheme", 1, [theme1]);
-
-  DevToolsShim.registerTheme(theme2);
-  checkCalls(mock, "registerTheme", 2, [theme2]);
-
-  DevToolsShim.unregister();
-
-  // Create a new mock and check the themes are not added once again.
-  mock = createMockDevTools();
-  DevToolsShim.register(mock);
-  checkCalls(mock, "registerTheme", 0);
-
-  restoreDevToolsInstalled();
-}
-
 function test_events() {
   mockDevToolsInstalled(true);
 
   ok(!DevToolsShim.isInitialized(), "DevTools are not initialized");
 
   let mock = createMockDevTools();
   // Check emit was not called.
   checkCalls(mock, "emit", 0);
@@ -278,19 +208,13 @@ function run_test() {
   DevToolsShim.unregister();
 
   test_off_called_before_registering_devtools();
   DevToolsShim.unregister();
 
   test_off_called_before_with_bad_callback();
   DevToolsShim.unregister();
 
-  test_registering_tool();
-  DevToolsShim.unregister();
-
-  test_registering_theme();
-  DevToolsShim.unregister();
-
   test_scratchpad_apis();
   DevToolsShim.unregister();
 
   test_events();
 }