Closed Bug 1359085 Opened 3 years ago Closed 3 years ago

Extract devtools/server/content-globals from devtools

Categories

(DevTools :: General, enhancement, P3)

enhancement

Tracking

(firefox55 fixed)

RESOLVED FIXED
Firefox 55
Tracking Status
firefox55 --- fixed

People

(Reporter: jdescottes, Assigned: jdescottes)

References

Details

Attachments

(1 file)

addon-sdk/source/lib/sdk/loader/sandbox.js relies on devtools/server/content-globals . It is actually the only consumer for this class in mozilla-central today (except for tests, including one in devtools: devtools/shared/tests/mochitest/test_devtools_extensions.html)

Since DevTools are being removed from mozilla central, content-globals.js should be moved outside of devtool to addon-sdk.
Simply moving the module to addon-sdk is not enough. 

content-globals.js relies on being loaded by the devtools loader in order to :
- be able to inject globals
- check if we are loaded in a worker (relying on the isWorker global)
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
I am actually not sure if the globals set using content-globals are ever used. The only call site for getContentGlobals used to be in devtools/server/actors/script.js but was removed in https://bugzilla.mozilla.org/show_bug.cgi?id=1039952

(I searched both in mc and addons code)

I will do a try run removing this code and see what happens.
Try seems green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3864dc81e1063fc35cf26db29cf9f9e2af3d8340 

Alex, do you have any insights about content-globals? As I said getContentGlobals is never used, so I think we should consider removing this completely.
Flags: needinfo?(poirot.alex)
This is related to Addon-SDK Content scripts. It was to allow debugging them. Seing them in the debugger. I'm not sure this feature still work. May be the platform code involved in bug 1039952 ensure they are still covered. It is possible this code replaces content-globals:
http://searchfox.org/mozilla-central/source/devtools/server/actors/tab.js#533-546

You can see this pattern appear in SDK test:
http://searchfox.org/mozilla-central/source/addon-sdk/source/test/test-page-mod-debug.js#35-37


In any case, I confirm, this code looks useless.
test-page-mode-debug.js be kept, but you can remove the logic around content-globals,
test_devtools_extensions.html could be completely removed with content-globals.js,
sandbox.js should be pruned from content-globals logic
Flags: needinfo?(poirot.alex)
Triaging dt-addon blockers as P3/enhancement since we're not using priorities for this project (filter on CLIMBING SHOES).
Severity: normal → enhancement
Priority: -- → P3
Comment on attachment 8863362 [details]
Bug 1359085 - remove unused devtools util content-globals.js;

https://reviewboard.mozilla.org/r/135126/#review138932

Thanks for this patch!
Attachment #8863362 - Flags: review?(poirot.alex) → review+
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 04ac52fd322c -d a86584f1364d: rebasing 393597:04ac52fd322c "Bug 1359085 - remove unused devtools util content-globals.js;r=ochameau" (tip)
merging devtools/shared/tests/mochitest/chrome.ini
warning: conflicts while merging devtools/shared/tests/mochitest/chrome.ini! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 1d2200ea18a8 -d c8e67ddc6f34: rebasing 393895:1d2200ea18a8 "Bug 1359085 - remove unused devtools util content-globals.js;r=ochameau" (tip)
merging devtools/shared/tests/mochitest/chrome.ini
warning: conflicts while merging devtools/shared/tests/mochitest/chrome.ini! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d69be43cf123
remove unused devtools util content-globals.js;r=ochameau
https://hg.mozilla.org/mozilla-central/rev/d69be43cf123
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.