Move webConsoleClient from controller to utils/client.js and export some useful public APIs. * Let utils/client.js takes over webConsoleClient getString, sendHTTPRequest * Move controller public APIs to client.js * triggerActivity * inspectRequest (also use in webconsole panel) * Export viewSourceInDebugger as a public API * Export supportsCustomRequest as a public API * Use these exported APIs in utils/client.js such as getString, supportCustomRequest in our codebase.

Comment on attachment 8845808 [details] Bug 1344158 - PART 2: remove gNetwork and move getString to utils/client; https://reviewboard.mozilla.org/r/118976/#review121260 ::: devtools/client/netmonitor/components/monitor-panel.js:73 (Diff revision 2) > requestHeadersFromUploadStream && requestPostData) { > getFormDataSections( > requestHeaders, > requestHeadersFromUploadStream, > requestPostData, > - window.gNetwork.getString.bind(window.gNetwork), > + getLongString, I think we don't need to pass getString as a paramenter into getFormDataSections(). Instead, we can use getString in request-utils.js directly and then we don't have to pass it as a parameter anymore. ::: devtools/client/netmonitor/request-list-context-menu.js:226 (Diff revision 2) > // Try to extract any form data parameters. > let formDataSections = await getFormDataSections( > selected.requestHeaders, > selected.requestHeadersFromUploadStream, > selected.requestPostData, > - window.gNetwork.getString.bind(window.gNetwork)); > + getLongString); I think we don't need to pass getString as a paramenter into getFormDataSections(). Instead, we can use getString in request-utils.js directly and then we don't have to pass it as a parameter anymore.

Comment on attachment 8845742 [details] Bug 1344158 - PART 1: wrap webconsole; https://reviewboard.mozilla.org/r/118876/#review121258 ::: devtools/client/netmonitor/netmonitor-controller.js:18 (Diff revision 1) > formDataURI, > } = require("./utils/request-utils"); > const { > onFirefoxConnect, > onFirefoxDisconnect, > + getWebConsoleClient, nit: alphabetical order ::: devtools/client/netmonitor/utils/client.js:60 (Diff revision 1) > /** > * Process disconnect events. > * > * @param {Object} tabTarget > */ > function onFirefoxDisconnect(tabTarget) { nit: activeConsole = null; ::: devtools/client/netmonitor/utils/client.js:77 (Diff revision 1) > +} > + > module.exports = { > onFirefoxConnect, > onFirefoxDisconnect, > + getWebConsoleClient, nit: in alphabetical order

(In reply to Ricky Chien [:rickychien] from comment #4) > Comment on attachment 8845808 [details] > Bug 1344158 - PART 2: remove gNetwork and move getString to utils/client; > > https://reviewboard.mozilla.org/r/118976/#review121260 > > ::: devtools/client/netmonitor/components/monitor-panel.js:73 > (Diff revision 2) > > requestHeadersFromUploadStream && requestPostData) { > > getFormDataSections( > > requestHeaders, > > requestHeadersFromUploadStream, > > requestPostData, > > - window.gNetwork.getString.bind(window.gNetwork), > > + getLongString, > > I think we don't need to pass getString as a paramenter into > getFormDataSections(). Instead, we can use getString in request-utils.js > directly and then we don't have to pass it as a parameter anymore. > When I import getLongString in request-utils, the mysterious side-affect happens https://treeherder.mozilla.org/#/jobs?repo=try&revision=63f12120496d (ex: actions/selection.js:48 - TypeError: getDisplayedRequests is not a function) I think its related to our init process is not in the right order yet. (We render and hook the UI then start the controller, function such as `getFormDataSections` might be called in UI before we assign the webconsoleClient) And to prevent the cyclic dependency, it's more clear to pass the getLongString call instead of require utils/client in request-utils. So I tend to leave `getFormDataSections` as it is now.

Comment on attachment 8845808 [details] Bug 1344158 - PART 2: remove gNetwork and move getString to utils/client; https://reviewboard.mozilla.org/r/118976/#review121438 ::: devtools/client/netmonitor/request-list-context-menu.js:151 (Diff revision 4) > })); > > menu.append(new MenuItem({ > type: "separator", > - visible: !!selectedRequest, > + visible: !!(window.NetMonitorController.supportsCustomRequest && > + selectedRequest && !selectedRequest.isCustom), This is a bug that when the build doesn't support custom request, there will be an extra separator shown on context menu

Comment on attachment 8845742 [details] Bug 1344158 - PART 1: wrap webconsole; https://reviewboard.mozilla.org/r/118876/#review124746

Comment on attachment 8845808 [details] Bug 1344158 - PART 2: remove gNetwork and move getString to utils/client; https://reviewboard.mozilla.org/r/118976/#review124748 I like removing globals! R+ assuming try is green. Thanks, Honza

I spot the leak in treeherfer(which didn't happen in previous try with these patch). It seems happened in all run, so I'd rebase and add a new try run again to confirm if its a not related issue

The leak on OSX-debug build is known in Bug 1329836, so I think its ok to land this, thanks!

Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/76897119071c PART 1: wrap webconsole;r=Honza https://hg.mozilla.org/integration/autoland/rev/1e4c005a9563 PART 2: remove gNetwork and move getString to utils/client;r=Honza

https://hg.mozilla.org/mozilla-central/rev/76897119071c https://hg.mozilla.org/mozilla-central/rev/1e4c005a9563