Closed Bug 1374735 Opened 7 years ago Closed 7 years ago

[dt-addon] remaining dependencies between m-c and devtools

Categories

(DevTools :: General, defect, P3)

Product:
DevTools
Component:
General
Type:
defect

Tracking

(firefox57 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 57
Tracking Flags:
Tracking Status
firefox57 --- fixed

People

(Reporter: jdescottes, Assigned: jdescottes)

References

Details

Attachments

(4 files, 2 obsolete files)

Bug 1374735 - use DevToolsShim to create TabTarget in ext-devtools;
59 bytes, text/x-review-board-request
mixedpuppy
: review+
Details
Bug 1374735 - use DevToolsShim to retrieve the WebExtensionInspectedWindowFront;
59 bytes, text/x-review-board-request
rpl
: review+
ochameau
: review+
Details
Bug 1374735 - use DevToolsShim to open browser console from extensions;
59 bytes, text/x-review-board-request
bgrins
: review+
kmag
: review+
Details
Bug 1374735 - stop exposing devtools require to extensions ext-* files;
59 bytes, text/x-review-board-request
kmag
: review+
Details
We misseda few mozilla-central -> devtools dependencies. After facing an issue with XPIProvider.jsm, I did a new query to find remaining dependencies and got the following list: http://searchfox.org/mozilla-central/source/toolkit/components/extensions/Extension.jsm "resource://devtools/shared/Loader.jsm"); http://searchfox.org/mozilla-central/source/toolkit/components/extensions/ExtensionCommon.jsm "resource://devtools/shared/Loader.jsm"); http://searchfox.org/mozilla-central/source/toolkit/components/extensions/ext-browser-content.js "resource://devtools/shared/Loader.jsm"); http://searchfox.org/mozilla-central/source/toolkit/components/extensions/ext-c-toolkit.js require("devtools/client/framework/devtools-browser"); let hudservice = require("devtools/client/webconsole/hudservice"); http://searchfox.org/mozilla-central/source/toolkit/components/extensions/ext-management.js "resource://devtools/shared/event-emitter.js"); http://searchfox.org/mozilla-central/source/browser/components/extensions/ext-devtools-inspectedWindow.js } = require("devtools/shared/fronts/webextension-inspected-window"); http://searchfox.org/mozilla-central/source/browser/components/extensions/ext-devtools.js const {TabTarget} = require("devtools/client/framework/target"); http://searchfox.org/mozilla-central/source/browser/tools/mozscreenshots/mozscreenshots/extension/configurations/DevTools.jsm Cu.import("resource://devtools/client/framework/gDevTools.jsm"); let { devtools } = Cu.import("resource://devtools/shared/Loader.jsm", {}); http://searchfox.org/mozilla-central/source/browser/base/content/test/siteIdentity/test_no_mcb_on_http_site_font.css src: url(http://example.com/browser/devtools/client/fontinspector/test/browser_font.woff); http://searchfox.org/mozilla-central/source/dom/media/webvtt/vtt.jsm const { require } = Cu.import("resource://devtools/shared/Loader.jsm", {}); http://searchfox.org/mozilla-central/source/mobile/android/chrome/content/RemoteDebugger.js let { require } = Cu.import("resource://devtools/shared/Loader.jsm", {}); let { DebuggerServer } = require("devtools/server/main"); * devtools/shared/security/auth.js. http://searchfox.org/mozilla-central/source/mobile/android/components/extensions/ext-browserAction.js "resource://devtools/shared/event-emitter.js"); http://searchfox.org/mozilla-central/source/mobile/android/installer/allowed-dupes.mn modules/devtools/Console.jsm http://searchfox.org/mozilla-central/source/mobile/android/modules/dbg-browser-actors.js const { RootActor } = require("devtools/server/actors/root"); const { DebuggerServer } = require("devtools/server/main"); require("devtools/server/actors/webbrowser");
Comment on attachment 8880513 [details] Bug 1374735 - use toolkit eventemitter in extensions/ext-management.js; https://reviewboard.mozilla.org/r/151836/#review161674
Attachment #8880513 - Flags: review?(mixedpuppy) → review+
Comment on attachment 8880515 [details] Bug 1374735 - use DevToolsShim to create TabTarget in ext-devtools; https://reviewboard.mozilla.org/r/151840/#review161676 ::: browser/components/extensions/ext-devtools.js:57 (Diff revision 2) > if (!context.devToolsToolbox.target.isLocalTab) { > throw new Error("Unexpected target type: only local tabs are currently supported."); > } > > - const {TabTarget} = require("devtools/client/framework/target"); > - > + const tab = context.devToolsToolbox.target.tab; > + context.devToolsTarget = DevToolsShim.getTargetForTab(tab); https://dxr.mozilla.org/mozilla-central/source/devtools/shim/DevToolsShim.jsm#221 getTargetForTab is marked to go away with addon-sdk, seems like that should be changed.
Attachment #8880515 - Flags: review?(mixedpuppy) → review-
Comment on attachment 8880517 [details] Bug 1374735 - use DevToolsShim to open browser console from extensions; https://reviewboard.mozilla.org/r/151844/#review162300 r=me for the devtools-browser changes. Can you please request review for the extensions change from someone who can better review that?
Attachment #8880517 - Flags: review?(bgrinstead) → review+
Comment on attachment 8880516 [details] Bug 1374735 - use DevToolsShim to retrieve the WebExtensionInspectedWindowFront; https://reviewboard.mozilla.org/r/151842/#review162796 This relates to bug 1222047, it should be easier to create an actor/front out of toolbox or target object. Here the codebase already has access to the target object. I imagine that's a reasonable way to address that until we simplify this... ::: devtools/client/framework/devtools.js:581 (Diff revision 2) > initBrowserToolboxProcessForAddon: function (addonID) { > BrowserToolboxProcess.init({ addonID }); > }, > > /** > + * Compatibility layer for web-extensions. Used fromt the DevToolsShim by s/Used fromt the DevToolsShim by/Used by/
Attachment #8880516 - Flags: review?(poirot.alex) → review+
Comment on attachment 8880514 [details] Bug 1374735 - stop requiring devtools loader if not used; https://reviewboard.mozilla.org/r/151838/#review163022
Attachment #8880514 - Flags: review?(kmaglione+bmo) → review+
Comment on attachment 8880518 [details] Bug 1374735 - stop exposing devtools require to extensions ext-* files; https://reviewboard.mozilla.org/r/151850/#review163024
Attachment #8880518 - Flags: review?(kmaglione+bmo) → review+
Comment on attachment 8880517 [details] Bug 1374735 - use DevToolsShim to open browser console from extensions; https://reviewboard.mozilla.org/r/151844/#review163026 r=me, assuming the tests still pass. Will this keep working if it's triggered while the devtools extension isn't installed, though?
Attachment #8880517 - Flags: review+
Comment on attachment 8880515 [details] Bug 1374735 - use DevToolsShim to create TabTarget in ext-devtools; https://reviewboard.mozilla.org/r/151840/#review161676 > https://dxr.mozilla.org/mozilla-central/source/devtools/shim/DevToolsShim.jsm#221 > > getTargetForTab is marked to go away with addon-sdk, seems like that should be changed. Thanks for catching that! I moved it to the "webextensions" related methods. Is this semantically correct?
(In reply to Kris Maglione [:kmag] from comment #20) > Comment on attachment 8880517 [details] > Bug 1374735 - use event to open browser console from extensions; > > https://reviewboard.mozilla.org/r/151844/#review163026 > > r=me, assuming the tests still pass. Will this keep working if it's > triggered while the devtools extension isn't installed, though? If devtools are not installed, the event will not be processed so this will be a no-op. For now, DevTools are always considered to be installed (ie. all channels will have it built in at least until 57). After it becomes optional, we have several options if we feel like a no-op is not acceptable here: - 1: trigger the download & installation of the DevTools addon in the background - 2: open the devtools addon install page - 3: notify the user differently (dump? Services.prompt.alert?) Options 1 and 2 need to wait until the bugs for installing devtools are fixed (Bug 1361080 & Bug 1378397). Option 3 probably can be implemented right away behind a `if (DevToolsShim.isInstalled()) {...}`
Flags: needinfo?(kmaglione+bmo)
Comment on attachment 8880516 [details] Bug 1374735 - use DevToolsShim to retrieve the WebExtensionInspectedWindowFront; https://reviewboard.mozilla.org/r/151842/#review163480
Attachment #8880516 - Flags: review?(lgreco) → review+
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Comment on attachment 8880515 [details] Bug 1374735 - use DevToolsShim to create TabTarget in ext-devtools; https://reviewboard.mozilla.org/r/151840/#review163682
Attachment #8880515 - Flags: review?(mixedpuppy) → review+
try https://treeherder.mozilla.org/#/jobs?repo=try&revision=0920a6d19a138dd08744b7aa33e307afb4db5cdb Slightly changed the approach for https://reviewboard.mozilla.org/r/151844/diff/4#index_header (Bug 1374735 - use event to open browser console from extensions;) because of Bug 1359855. We are now using the DevTools shim rather than an event to communicate with devtools. This way if devtools is installed, but not initialized, the shim will take care of the initialization.
Blocks: 1369801
Attachment #8880513 - Attachment is obsolete: true
Attachment #8880514 - Attachment is obsolete: true
Just rebased on latest central. The two first changesets became redundant with other patches that landed before. Pushed to try again: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9f9d0166c9bb0e35a0e0a4cd4908105f675e8370 Clearing the ni?
Flags: needinfo?(kmaglione+bmo)
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/22baf4e67730 use DevToolsShim to create TabTarget in ext-devtools;r=mixedpuppy https://hg.mozilla.org/integration/autoland/rev/b7435cd66ce3 use DevToolsShim to retrieve the WebExtensionInspectedWindowFront;r=ochameau,rpl https://hg.mozilla.org/integration/autoland/rev/da075933f7bc use DevToolsShim to open browser console from extensions;r=bgrins,kmag https://hg.mozilla.org/integration/autoland/rev/50a36fb7c7f9 stop exposing devtools require to extensions ext-* files;r=kmag
Backed out for test_ext_i18n.js failures. https://hg.mozilla.org/integration/autoland/rev/8eeea4adcdd14613d4c4ba7c26edb8aabbe5985d https://treeherder.mozilla.org/logviewer.html#?job_id=124174019&repo=autoland TEST-UNEXPECTED-FAIL | xpcshell-e10s.ini:toolkit/components/extensions/test/xpcshell/test_ext_i18n.js | test_i18n - [test_i18n : 175] Extension left running at test shutdown - "running" == "unloaded"
Flags: needinfo?(jdescottes)
Pushed by kwierso@gmail.com: https://hg.mozilla.org/integration/autoland/rev/e2e779f54e10 use DevToolsShim to create TabTarget in ext-devtools;r=mixedpuppy https://hg.mozilla.org/integration/autoland/rev/7a0133d5ee1d use DevToolsShim to retrieve the WebExtensionInspectedWindowFront;r=ochameau,rpl https://hg.mozilla.org/integration/autoland/rev/d8b6d2584aec use DevToolsShim to open browser console from extensions;r=bgrins,kmag https://hg.mozilla.org/integration/autoland/rev/49ff7e926ea7 stop exposing devtools require to extensions ext-* files;r=kmag CLOSED TREE
Flags: needinfo?(jdescottes)
Depends on: 1392531
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: