Architecture reactify step for creating top level NetworkMonitor react component. * Implement RequestListPanel component instead of requests-menu-view.js * Remove requests-menu-view.js

In order to implement a react friendly Splitbox in bug 1309183, I'd prefer to get rid of requests-menu-view.js. Splitbox component will build on top of both RequestList (this one) and NetworkDetailsPanel and adopt full react style. This is a big patch with a bunch of files changed because we used NetmonitorView.RequestsMenu global property in many mochitest files. It took a lot of my time to check and fix all these test failures step by step.

P.S. This is a great step to have a patch for getting rid of entire requests-menu-view.js. My feeling is that the requests-menu-view.js is the most dirty and worst modules in netmonitor. Architecture and code quality will be improved so much and it will be easier to introduce new feature such as React Virtualize.

Comment on attachment 8835916 [details] Bug 1336383 - Implement RequestsList component https://reviewboard.mozilla.org/r/111472/#review112852 Great job here Ricky! Just a few comments inline. There is a lot of changes in the tests but, but they seem to follow the same pattern so, I scanned it quickly. Honza ::: devtools/client/framework/test/browser_ignore_toolbox_network_requests.js:27 (Diff revision 4) > > is(panel.UI.editors.length, 1, "correct number of editors opened"); > > let monitor = yield toolbox.selectTool("netmonitor"); > - let { RequestsMenu } = monitor.panelWin.NetMonitorView; > - is(RequestsMenu.itemCount, 0, "No network requests appear in the network panel"); > + let { gStore, windowRequire } = monitor.panelWin; > + let Actions = windowRequire("devtools/client/netmonitor/actions/index"); The `Actions` object is never used. ::: devtools/client/netmonitor/components/request-list-context-menu.js:41 (Diff revision 4) > +}) { > + this.cloneSelectedRequest = cloneSelectedRequest; > + this.openStatistics = openStatistics; > +} > + > +RequestListContextMenu.prototype = { This isn't real React component. I think it shouldn't be in `components` directory ::: devtools/client/netmonitor/netmonitor-controller.js:701 (Diff revision 4) > + true)); > + } > + } > + } > + }), > + Adding the `addRequest` and `updateRequest` methods in this module now is ok but, we should think about how to refactor the netmonitor-controller.js module. It's getting big.

Comment on attachment 8835916 [details] Bug 1336383 - Implement RequestsList component https://reviewboard.mozilla.org/r/111472/#review112852 > This isn't real React component. I think it shouldn't be in `components` directory Nice catch! I will move request-list-tooltip.js out of components dir as well. > Adding the `addRequest` and `updateRequest` methods in this module now is ok but, we should think about how to refactor the netmonitor-controller.js module. It's getting big. Right! I have to figure out how to improve netmonitor-controller.js in someday.

Comment on attachment 8835916 [details] Bug 1336383 - Implement RequestsList component https://reviewboard.mozilla.org/r/111472/#review112866 R+ assuming Try is green. Thanks! Honza

Pushed by rchien@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/eee8f9b6791d Implement RequestsList component r=Honza

Backed out for failing browser_net_har_post_data.js and more: https://hg.mozilla.org/integration/autoland/rev/55916d685e88b8915fdda658c8dc8f0fadb0bfb4 Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=eee8f9b6791deff3db5fb7a3423ed04fdc4e6725&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=76371486&repo=autoland [task 2017-02-10T16:35:58.632625Z] 16:35:58 INFO - TEST-PASS | devtools/client/netmonitor/har/test/browser_net_har_post_data.js | Request to reconfigure the tab was recorded. - [task 2017-02-10T16:35:58.632699Z] 16:35:58 INFO - Buffered messages logged at 16:35:58 [task 2017-02-10T16:35:58.632801Z] 16:35:58 INFO - Network monitor pane shown successfully. [task 2017-02-10T16:35:58.632840Z] 16:35:58 INFO - Starting test... [task 2017-02-10T16:35:58.632986Z] 16:35:58 INFO - Buffered messages finished [task 2017-02-10T16:35:58.633209Z] 16:35:58 INFO - TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/har/test/browser_net_har_post_data.js | Uncaught exception - at chrome://mochitests/content/browser/devtools/client/netmonitor/har/test/browser_net_har_post_data.js:18 - TypeError: RequestsMenu is undefined [task 2017-02-10T16:35:58.633252Z] 16:35:58 INFO - Stack trace: [task 2017-02-10T16:35:58.633317Z] 16:35:58 INFO - @chrome://mochitests/content/browser/devtools/client/netmonitor/har/test/browser_net_har_post_data.js:18:3 [task 2017-02-10T16:35:58.633372Z] 16:35:58 INFO - Tester_execTest@chrome://mochikit/content/browser-test.js:735:9 [task 2017-02-10T16:35:58.633431Z] 16:35:58 INFO - Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:655:7 [task 2017-02-10T16:35:58.633504Z] 16:35:58 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:744:59 [task 2017-02-10T16:35:58.633553Z] 16:35:58 INFO - Tester_execTest@chrome://mochikit/content/browser-test.js:735:9 [task 2017-02-10T16:35:58.633608Z] 16:35:58 INFO - Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:655:7 [task 2017-02-10T16:35:58.633800Z] 16:35:58 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:744:59 Also: TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/har/test/browser_net_har_throttle_upload.js | Uncaught exception - at chrome://mochitests/content/browser/devtools/client/netmonitor/har/test/browser_net_har_throttle_upload.js:44 - TypeError: RequestsMenu is undefined TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_copy_headers.js | Uncaught exception - at chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_copy_headers.js:46 - TypeError: monitor._toolbox.doc.querySelector(...) is null

Pushed by rchien@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/030a883d87a7 Implement RequestsList component r=Honza

There are some Warning when enable `ac_add_options --enable-debug-js-modules` in mozconfig Warning: Failed prop type: Required prop `contextMenu` was not specified in `RequestListContent`. in RequestListContent (created by Connect(RequestListContent)) in Connect(RequestListContent) (created by RequestList) in div (created by RequestList) in RequestList (created by Connect(RequestList)) in Connect(RequestList) in Provider

Warning: Failed prop type: Required prop `tooltip` was not specified in `RequestListContent`. in RequestListContent (created by Connect(RequestListContent)) in Connect(RequestListContent) (created by RequestList) in div (created by RequestList) in RequestList (created by Connect(RequestList)) in Connect(RequestList) in Provider

Thanks for reporting this. Warnings will be addressed in bug 1309183.

This issue is verified fixed on latest Nightly 54.0a1 (2017-02-22) using the following platforms: - Windows 10 x64 - Mac OS X 10.11.6 - Ubuntu 16.04 x64 LTS