Used in: devtools/client/shared/doorhanger.js devtools/shared/worker/worker.js

Used also in: devtools/shared/gcli/commands/rulers.js

Comment on attachment 8884227 [details] Bug 1378821 - Stop using sdk/window/utils in DevTools; https://reviewboard.mozilla.org/r/155180/#review160240 r+ but the comments addressed, since this code could behave differently from the original. ::: devtools/client/shared/doorhanger.js:153 (Diff revision 2) > > return promise; > } > > function getGBrowser() { > - return getMostRecentBrowserWindow().gBrowser; > + return Services.wm.getMostRecentWindow(gDevTools.chromeWindowType).gBrowser; I'm not sure I would add the whole framework/devtools module, and the gDevTools just for a string here. It's okay to use if we already require that, but I think here we can just use `Services.wm.getMostRecentWindow("navigator:browser")` as in most of the firefox's codebase seems to be done – I was looking if Services or wm exposed this string as constant, but basically it's plain almost everywhere. Unless we don't want "navigator:browser" all the times: `chromeWindowType` seems to be a property that other gecko apps can override: http://searchfox.org/mozilla-central/source/devtools/client/framework/devtools.js#70-72 So, what we want here exactly? If we want to have the previous behavior, it should be "navigator:browser", otherwise it means the previous behavior was wrong all long, and we should take this occasion to fix it, and use `chromeWindowType`. Here it seems to me that we want to have the Global Browser, so I suggest to use: ```js Services.wm.getMostRecentWindow("navigator:browser") ``` ::: devtools/shared/worker/worker.js:146 (Diff revision 2) > "used in production."); > - // Fetch via window/utils here as we don't want to include > - // this module normally. > - let { getMostRecentBrowserWindow } = require("sdk/window/utils"); > - let { URL, Blob } = getMostRecentBrowserWindow(); > + // Fetch modules here as we don't want to include it normally. > + const Services = require("Services"); > + const { gDevTools } = require("devtools/client/framework/devtools"); > + > + let { URL, Blob } = Services.wm.getMostRecentWindow(gDevTools.chromeWindowType); Same here. If we want to address the `chromeWindowType`, that *might* not be the browser window, it's OK (however, in that case I'm suggesting to implement in `gDevTools` a method to do so, like `gDevTools.getMostRecentChromeWindow()`); otherwise if we want the browser window, we should use directly "navigator:browser".

Comment on attachment 8884227 [details] Bug 1378821 - Stop using sdk/window/utils in DevTools; https://reviewboard.mozilla.org/r/155180/#review163488 Looks good to me! I just noticed that we have an occurrence here to: http://searchfox.org/mozilla-central/source/testing/talos/talos/tests/devtools/addon/content/damp.js#7 maybe we want to address that in this patch in the same way you've done for the other files (for sd/window/utils, it just uses `getMostRecentBrowserWindow`).

I added r+ since the changes to damp.js would be the same of the others, so no review required. If you do not address the issue in this patch for some reasons, we should file a follow up bug.

(In reply to Matteo Ferretti [:zer0] [:matteo] from comment #6) > Comment on attachment 8884227 [details] > Bug 1378821 - Stop using sdk/window/utils in DevTools; > > https://reviewboard.mozilla.org/r/155180/#review163488 > > Looks good to me! I just noticed that we have an occurrence here to: > http://searchfox.org/mozilla-central/source/testing/talos/talos/tests/ > devtools/addon/content/damp.js#7 maybe we want to address that in this patch > in the same way you've done for the other files (for sd/window/utils, it > just uses `getMostRecentBrowserWindow`). There's a different bug on file for getting rid of SDK usage in DAMP: Bug 1364596

(In reply to Brian Grinstead [:bgrins] from comment #8) > There's a different bug on file for getting rid of SDK usage in DAMP: Bug > 1364596 Thanks, let's solve the issue in this bug. Honza

Pushed by jodvarko@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6a297d875e0e Stop using sdk/window/utils in DevTools; r=zer0