Closed Bug 1005755 Opened 10 years ago Closed 7 years ago

A button to pause/stop the HTTP traffic in the network monitor view

Categories

(DevTools :: Netmonitor, defect, P3)

Product:
DevTools
Component:
Netmonitor
Version:
30 Branch
Platform:
x86
macOS
Type:
defect

Tracking

(firefox58 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 58
Tracking Flags:
Tracking Status
firefox58 --- fixed

People

(Reporter: karlcow, Assigned: Honza)

References

(Blocks 2 open bugs)

Details

(Keywords: dev-doc-complete)

Attachments

(5 files)

Bug 1005755 - Introduce pause/resume button;
59 bytes, text/x-review-board-request
rickychien
: review+
Details
Bug 1005755 - Stop using Connector as a singleton;
59 bytes, text/x-review-board-request
rickychien
: review+
Details
Bug 1005755 - Update tests;
59 bytes, text/x-review-board-request
rickychien
: review+
Details
Bug 1005755 - Update Net logging in the Console;
59 bytes, text/x-review-board-request
nchevobbe
: review+
Details
Bug 1005755 - Test for pause/stop button;
59 bytes, text/x-review-board-request
nchevobbe
: review+
Details 
When inspecting the HTTP traffic of a Web site in the "Network Monitor" view, there are moments you would like to pause all future network transactions. Providing a button to pause or stop the traffic would be excellent.

For example, Huffington Mobile Web site provides a beacon loaded through XHR pinging the mother site every x seconds. When inspecting, you need to have a stable view which is not evolving anymore.
Honza, to estimate priority vs cost; do you have a sense for how feasible this is? Maybe we can build on event queue from Quantum DOM and could pause the page's event queue?
Flags: needinfo?(odvarko)
(In reply to :Harald Kirschner :digitarald from comment #1)
> Honza, to estimate priority vs cost; do you have a sense for how feasible
> this is? Maybe we can build on event queue from Quantum DOM and could pause
> the page's event queue?
Not sure how this would affect the rest of the page behavior and e.g. debugging.

Tom, could we use the throttling API for this somehow?

Honza
Flags: needinfo?(odvarko) → needinfo?(ttromey)
Priority: -- → P3
(In reply to Jan Honza Odvarko [:Honza] from comment #2)

> Tom, could we use the throttling API for this somehow?

I think so -- or at least it is worth a try -- depending on what the goal is.

With throttling, the network communication still happens.  However we
just buffer the results and release them back to content more slowly (or
in this case hold them indefinitely).  So, if you want to avoid the connection
and transfer of data entirely, then throttling won't work.
Flags: needinfo?(ttromey)
(In reply to Tom Tromey :tromey from comment #3)
> With throttling, the network communication still happens.  However we
> just buffer the results and release them back to content more slowly (or
> in this case hold them indefinitely).  So, if you want to avoid the
> connection
> and transfer of data entirely, then throttling won't work.
I see, thanks for the update.

(In reply to Karl Dubost :karlcow from comment #0)
> For example, Huffington Mobile Web site provides a beacon loaded through XHR
> pinging the mother site every x seconds. When inspecting, you need to have a
> stable view which is not evolving anymore.
So, we can just stop the monitoring.

What about introducing a toggle button on the Net panel's toolbar allowing:

1) Stop recording network log
2) Record network log

If disabled, the page would continue sending/receiving HTTP request/responses, but
the Network panel would not be listening. The view would be stable then.

Honza
Assignee: nobody → odvarko
Status: NEW → ASSIGNED
> So, we can just stop the monitoring.

yup that would work in the beacon case.

That would not work in the case where you get a JS or html (meta) reloading the full page (like some news Web sites).
1) Bug 1005755 - Introduce pause/resume button;
@Ricky: Introducing new toolbar button. There is a new action, reducer and selector. The button will be used to pause/resume the recording.

2) Bug 1005755 - Stop using Connector as a singleton;
@Ricky: This is quite a few changes, but essentially it's all about passing 'connector' object down the component hierarchy. This patch stops using the Connector as a global singleton and it rather treats it as a property to components.

3) Bug 1005755 - Update tests;
@Ricky: updating existing tests

4) Bug 1005755 - Update Net logging in the Console;
@Nicolas: This is just a small update on the Console side. A connector object (mostly empty) must be passed down the hierarchy since it's mandatory prop. It's currently used only to open stack frame in the debugger, which should fix Bug 1403898.

Honza
(In reply to Karl Dubost :karlcow from comment #5)
> > So, we can just stop the monitoring.
> 
> yup that would work in the beacon case.
> 
> That would not work in the case where you get a JS or html (meta) reloading
> the full page (like some news Web sites).
We wanted to have the pause/resume feature and I used this bug report for it, hope it's ok :-)

We might need another bug for the thing you mentioned in comment #5
Currently, reloading the page resumes the recording.
(I can do that report if you want me)

Honza
@Harald: I used pause/resume icons from the debugger for that button. It looks good, but we might want to use something specific. Just let me know what you think.

Honza
Flags: needinfo?(hkirschner)
Comment on attachment 8913641 [details]
Bug 1005755 - Update Net logging in the Console;

https://reviewboard.mozilla.org/r/185038/#review190132

This looks good to me, although i'm not sure how this is related to the bug, but I trust you :)
Sonce you'are adding viewSourceInDebugger, i think we need a mochitest to test that in the console context.
Attachment #8913641 - Flags: review?(nchevobbe) → review+
Honza, LGTM on a UI level! Just to confirm an affordance requirement: If the panel is in paused state, would it start recording when on user-initiated refresh?

Addressing Karl's feedback: breaking on network requests is often requested and would solve the second issue of pausing the whole site state.
Flags: needinfo?(hkirschner)
Comment on attachment 8913638 [details]
Bug 1005755 - Introduce pause/resume button;

https://reviewboard.mozilla.org/r/185032/#review190216

looks good, need to keep declaration in alphabet orders.

::: devtools/client/netmonitor/src/actions/requests.js:112
(Diff revision 1)
>    clearRequests,
>    cloneSelectedRequest,
>    removeSelectedCustomRequest,
>    sendCustomRequest,
>    updateRequest,
> +  toggleRecording

should be in alphabet order under sendCustomRequest, and keep the trail `,`

::: devtools/client/netmonitor/src/components/toolbar.js:22
(Diff revision 1)
>  const {
>    getDisplayedRequestsSummary,
>    getRequestFilterTypes,
>    getTypeFilteredRequests,
>    isNetworkDetailsToggleButtonDisabled,
> +  getRecordingState,

should in alphabet order

::: devtools/client/netmonitor/src/constants.js:8
(Diff revision 1)
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  "use strict";
>  
>  const actionTypes = {
> +  TOGGLE_RECORDING: "TOGGLE_RECORDING",

please keep the alphabet order

::: devtools/client/netmonitor/src/reducers/requests.js:10
(Diff revision 1)
>  "use strict";
>  
>  const I = require("devtools/client/shared/vendor/immutable");
>  const { getUrlDetails } = require("../utils/request-utils");
>  const {
> +  TOGGLE_RECORDING,

alphabet order

::: devtools/client/netmonitor/src/reducers/requests.js:171
(Diff revision 1)
>          st.lastEndedMillis = lastEndedMillis;
>        });
>      }
>      case CLEAR_REQUESTS: {
> -      return new Requests();
> +      return new Requests({
> +        recording: state.recording

If user cleared all requests, he might want to start over again. Why we need to just keep the current recording state?

::: devtools/client/netmonitor/src/selectors/requests.js:146
(Diff revision 1)
>    getDisplayedRequestsSummary,
>    getRequestById,
>    getSelectedRequest,
>    getSortedRequests,
>    getTypeFilteredRequests,
> +  getRecordingState,

alphabet order
Comment on attachment 8913638 [details]
Bug 1005755 - Introduce pause/resume button;

https://reviewboard.mozilla.org/r/185032/#review190440

LGTM for the pause/resume button.

::: devtools/client/netmonitor/src/actions/requests.js:15
(Diff revision 1)
>    CLEAR_REQUESTS,
>    CLONE_SELECTED_REQUEST,
>    REMOVE_SELECTED_CUSTOM_REQUEST,
>    SEND_CUSTOM_REQUEST,
>    UPDATE_REQUEST,
> +  TOGGLE_RECORDING

nit: alphabetical order

::: devtools/client/netmonitor/src/components/toolbar.js:153
(Diff revision 1)
>  
> +    // Calculate class-list for toggle recording button. The button
> +    // has two states: pause/play.
> +    let toggleButtonClassList = [
> +      "devtools-button",
> +      recording ? "devtools-pause-icon" : "devtools-play-icon"

nit: tailing comma at the end of array.

::: devtools/client/netmonitor/src/components/toolbar.js:232
(Diff revision 1)
>      persistentLogsEnabled: state.ui.persistentLogsEnabled,
>      browserCacheDisabled: state.ui.browserCacheDisabled,
>      requestFilterTypes: getRequestFilterTypes(state),
>      filteredRequests: getTypeFilteredRequests(state),
>      summary: getDisplayedRequestsSummary(state),
> +    recording: getRecordingState(state),

nit: alphabetical order

::: devtools/client/netmonitor/src/components/toolbar.js:236
(Diff revision 1)
>      summary: getDisplayedRequestsSummary(state),
> +    recording: getRecordingState(state),
>    }),
>    (dispatch) => ({
>      clearRequests: () => dispatch(Actions.clearRequests()),
> +    toggleRecording: () => dispatch(Actions.toggleRecording()),

nit: alphabetical order

::: devtools/client/netmonitor/src/middleware/recording.js:17
(Diff revision 1)
> + * Start/stop HTTP traffic recording.
> + */
> +function recordingMiddleware(store) {
> +  return next => action => {
> +    const res = next(action);
> +    if (action.type == TOGGLE_RECORDING) {

nit: === instead of ==

::: devtools/client/netmonitor/src/reducers/requests.js:101
(Diff revision 1)
>    });
>  }
>  
>  function requestsReducer(state = new Requests(), action) {
>    switch (action.type) {
> +    case TOGGLE_RECORDING: {

nit: swtich case should be order in alphabetical order.
Attachment #8913638 - Flags: review?(rchien) → review+
Comment on attachment 8913639 [details]
Bug 1005755 - Stop using Connector as a singleton;

https://reviewboard.mozilla.org/r/185034/#review190448

I'm not seeing major issues from this patch. Please address all nits like naming or alphabetical order...etc. Thanks!

I also see some code improvement from this patch. Well done!

::: devtools/client/netmonitor/index.html:67
(Diff revision 1)
>              top.openUILinkIn(link, "tab");
>            };
>  
> -          const App = createFactory(require("./src/components/app"));
> +          // Render the root Application component.
>            const sourceMapService = toolbox.sourceMapURLService;
> -          const app = App({ sourceMapService, openLink });
> +          const app = App({ sourceMapService, openLink, connector });

nit: alphabetical order

::: devtools/client/netmonitor/index.html:85
(Diff revision 1)
> +         * Selects the specified request in the waterfall and opens the details view.
> +         *
> +         * @param {string} requestId The actor ID of the request to inspect.
> +         * @return {object} A promise resolved once the task finishes.
> +         */
> +        inspectRequest(requestId) {

inspectRequest is only using by Firefox internal devtools. (Link to netmonitor from other panel)

Moving from firefox-connector to index.html looks fine to me.

At least we can add a comment here to explain this is a firefox toolbox specific API, which providing an ablity to inspect a network request directly from other internal toolbox panel.

::: devtools/client/netmonitor/src/components/app.js:35
(Diff revision 1)
>    return (
>      div({ className: "network-monitor" },
> -      !statisticsOpen ? MonitorPanel({ openLink, sourceMapService }) : StatisticsPanel()
> +      !statisticsOpen ? MonitorPanel({
> +        openLink,
> +        sourceMapService,
> +        connector,

nit: alphabetical order

::: devtools/client/netmonitor/src/components/app.js:37
(Diff revision 1)
> -      !statisticsOpen ? MonitorPanel({ openLink, sourceMapService }) : StatisticsPanel()
> +      !statisticsOpen ? MonitorPanel({
> +        openLink,
> +        sourceMapService,
> +        connector,
> +      }) : StatisticsPanel({
> +        connector

nit: it's okay to write it as one line pattern if there is only one property.

```
StatisticsPanel({ connector })
```

::: devtools/client/netmonitor/src/components/monitor-panel.js:147
(Diff revision 1)
>      );
>    }
>  });
>  
>  module.exports = connect(
> -  (state) => ({
> +  (state, props) => ({

nit: remove `props` since it is unused here.

::: devtools/client/netmonitor/src/components/network-details-panel.js:42
(Diff revision 1)
>  
>    return (
>      div({ className: "network-details-panel" },
>        !request.isCustom ?
>          TabboxPanel({
> +          connector,

nit: alphabetical order

::: devtools/client/netmonitor/src/components/request-list-content.js:38
(Diff revision 1)
>   */
>  const RequestListContent = createClass({
>    displayName: "RequestListContent",
>  
>    propTypes: {
> +    connector: PropTypes.object.isRequired,

nit: alphabetical order

::: devtools/client/netmonitor/src/components/request-list-content.js:57
(Diff revision 1)
> +      getLongString: connector.getLongString,
> +      getTabTarget: connector.getTabTarget,

nit: alphabetical order

::: devtools/client/netmonitor/src/components/request-list.js:24
(Diff revision 1)
>  const { div } = DOM;
>  
>  /**
>   * Request panel component
>   */
> -function RequestList({ isEmpty }) {
> +function RequestList({ isEmpty, connector }) {

nit: define properties as multi-line pattern.

::: devtools/client/netmonitor/src/components/tabbox-panel.js:39
(Diff revision 1)
> -/*
> +/**
>   * Tabbox panel component
>   * Display the network request details
>   */
>  function TabboxPanel({
> +  connector,

nit: alphabetical order

::: devtools/client/netmonitor/src/components/tabbox-panel.js:110
(Diff revision 1)
>  }
>  
>  TabboxPanel.displayName = "TabboxPanel";
>  
>  TabboxPanel.propTypes = {
> +  connector: PropTypes.object.isRequired,

nit: alphabetical order

::: devtools/client/netmonitor/src/connector/firefox-connector.js:44
(Diff revision 1)
>      this.dataProvider = new FirefoxDataProvider({
>        webConsoleClient: this.webConsoleClient,
>        actions: this.actions,
>      });
>  
> -    this.tabTarget.on("will-navigate", this.willNavigate);
> +    this.addListeners();

nit:

How about registerListeners / setupListeners

::: devtools/client/netmonitor/src/connector/firefox-connector.js:67
(Diff revision 1)
>      if (this.tabTarget.getTrait("documentLoadingMarkers") && this.timelineFront) {
>        this.timelineFront.off("doc-loading", this.onDocLoadingMarker);
>        await this.timelineFront.destroy();
>      }
>  
> -    this.tabTarget.off("will-navigate");
> +    this.removeListeners();

nit:

How about unregisterListener / teardownListeners?

::: devtools/client/netmonitor/src/connector/index.js:15
(Diff revision 1)
> + * to the client type.
> + */
> +class Connector {
> +  constructor() {
> +    this.connector = null;
> +

Do we need to bind `connect` and `disconnect` APIs as well?

::: devtools/client/netmonitor/src/har/test/browser_net_har_throttle_upload.js:19
(Diff revision 1)
>    let { tab, monitor } = yield initNetMonitor(
>      HAR_EXAMPLE_URL + "html_har_post-data-test-page.html");
>  
>    info("Starting test... (actuallyThrottle = " + actuallyThrottle + ")");
>  
> -  let { store, windowRequire } = monitor.panelWin;
> +  let { store, connector, windowRequire } = monitor.panelWin;

nit: alphabetical order

::: devtools/client/netmonitor/src/middleware/recording.js:30
(Diff revision 1)
> +
> +    // Pause/resume HTTP monitoring according to
> +    // the user action.
>      if (action.type == TOGGLE_RECORDING) {
> -      // TODO connect/disconnect the backend.
> +      let state = store.getState();
> +      let recording = getRecordingState(state);

nit: define state as a variable but doens't use in other places looks weird.

`getRecordingState(store.getState())` is enough.

::: devtools/client/netmonitor/src/request-list-context-menu.js:26
(Diff revision 1)
> +  getLongString,
> +  getTabTarget,

nit: alphabetical order

::: devtools/client/netmonitor/src/request-list-context-menu.js:31
(Diff revision 1)
> +  getLongString,
> +  getTabTarget,
>    cloneSelectedRequest,
>    openStatistics,
>  }) {
> +  this.getLongString = getLongString;

nit: alphabetical order
Attachment #8913639 - Flags: review?(rchien) → review+
Comment on attachment 8913640 [details]
Bug 1005755 - Update tests;

https://reviewboard.mozilla.org/r/185036/#review190466

LGTM. Thank you Honza :)
Attachment #8913640 - Flags: review?(rchien) → review+
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #14)
> This looks good to me, although i'm not sure how this is related to the bug,
> but I trust you :)
See the comment #0: "When inspecting the HTTP traffic of a Web site in the "Network Monitor" view, there are moments you would like to pause all future network transactions. Providing a button to pause or stop the traffic would be excellent."

> Sonce you'are adding viewSourceInDebugger, i think we need a mochitest to
> test that in the console context.
Agree, I am working on it.

(In reply to Fred Lin [:gasolin] from comment #16)
> If user cleared all requests, he might want to start over again. Why we need
> to just keep the current recording state?
I think that auto-start is what we want (also to avoid case where the user forget about it and is confused that the net monitor doesn't work). Btw. Chrome DT is doing the same. 

(In reply to Ricky Chien [:rickychien] from comment #18)
> How about registerListeners / setupListeners
I kept 'add/removeListeners' since searching existing DT code base shows that this is more consistent


(In reply to :Harald Kirschner :digitarald from comment #15)
> Honza, LGTM on a UI level! Just to confirm an affordance requirement: If the
> panel is in paused state, would it start recording when on user-initiated
> refresh?
Yes

@Ricky, Fred, Nicolas: thanks for the review!

Honza
(In reply to Jan Honza Odvarko [:Honza] from comment #24)
> (In reply to Fred Lin [:gasolin] from comment #16)
> > If user cleared all requests, he might want to start over again. Why we need
> > to just keep the current recording state?
> I think that auto-start is what we want (also to avoid case where the user
> forget about it and is confused that the net monitor doesn't work). Btw.
> Chrome DT is doing the same. 
Sorry, my comment was related to re-load. Not sure if clear should also auto-start
and at least Chrome DT doesn't do it.

Honz
Comment on attachment 8913638 [details]
Bug 1005755 - Introduce pause/resume button;

https://reviewboard.mozilla.org/r/185032/#review191718

::: devtools/client/netmonitor/src/actions/requests.js:14
(Diff revision 2)
>    ADD_REQUEST,
>    CLEAR_REQUESTS,
>    CLONE_SELECTED_REQUEST,
>    REMOVE_SELECTED_CUSTOM_REQUEST,
>    SEND_CUSTOM_REQUEST,
> +  TOGGLE_RECORDING

trail comma is required,
(In reply to Fred Lin [:gasolin] from comment #28)
> > +  TOGGLE_RECORDING
> 
> trail comma is required,
Fixed

Honza
I am planning two follow-ups for the current work:

* Resume automatically after page reload
* Test for 'View source in Debugger' feature (Console panel).

Honza
Comment on attachment 8914263 [details]
Bug 1005755 - Test for pause/stop button;

https://reviewboard.mozilla.org/r/185582/#review194344

::: devtools/client/netmonitor/test/browser_net_pause.js:11
(Diff revision 3)
> +/**
> + * Tests if the pause/resume button works.
> + */
> +add_task(function* () {
> +  let { tab, monitor } = yield initNetMonitor(CUSTOM_GET_URL);
> +  info("Starting test... ");

nit: s/.../…

::: devtools/client/netmonitor/test/browser_net_pause.js:14
(Diff revision 3)
> +add_task(function* () {
> +  let { tab, monitor } = yield initNetMonitor(CUSTOM_GET_URL);
> +  info("Starting test... ");
> +
> +  let { document, store, windowRequire } = monitor.panelWin;
> +  let Actions = windowRequire("devtools/client/netmonitor/src/actions/index");

nit: feels a bit weird to have a variable capitalized without being a component/class

::: devtools/client/netmonitor/test/browser_net_pause.js:31
(Diff revision 3)
> +  // Click pause again to resume monitoring. Load a third request
> +  // and make sure they will show up.
> +  EventUtils.sendMouseEvent({ type: "click" }, pauseButton);
> +  yield performRequestAndWait(tab, monitor);
> +  assertRequestCount(store, 2);

maybe it would make sense to resume, test that we still have only 1 request (i.e. that the second one was indeed not listened), then do the third request and ensure we have 2 requests in the store.

::: devtools/client/netmonitor/test/browser_net_pause.js:67
(Diff revision 3)
> +  // A new request would definitely appear in 500 ms in the UI.
> +  // Declare victory if there is no request in this amount of time.
> +  yield waitForTime(500);

this looks error prone to me. don't we have other events that we can still listen to even when paused that would assert the request was sent back ? something like adding a one time listener on a client for example.


i acknowledge it might not be doable at all :) but it would be nice to be able to rely on something else than waiting for some time
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #37)
> > +  info("Starting test... ");
> nit: s/.../…
> nit: feels a bit weird to have a variable capitalized without being a
> component/class
I kept this the same since all the other tests use it too. Let's have
it consistent for now. But, I am not against to fix all at once as
part of another bug.

> maybe it would make sense to resume, test that we still have only 1 request
> (i.e. that the second one was indeed not listened), then do the third
> request and ensure we have 2 requests in the store.
I added on new assert.

> this looks error prone to me.
You are right, Timeouts are always evil. I found another solution
see the new version.

Thanks!
Honza
Comment on attachment 8914263 [details]
Bug 1005755 - Test for pause/stop button;

https://reviewboard.mozilla.org/r/185582/#review194732

Looks good, thanks Honza

::: devtools/client/netmonitor/test/browser_net_pause.js:44
(Diff revision 4)
> +  assertRequestCount(store, 1);
> +
> +  // Click pause again to resume monitoring. Load a third request
> +  // and make sure they will show up.
> +  EventUtils.sendMouseEvent({ type: "click" }, pauseButton);
> +  assertRequestCount(store, 1);

not sure if we really need this one, we already have the test in l.39

::: devtools/client/netmonitor/test/browser_net_pause.js:85
(Diff revision 4)
> +
> +/**
> + * Listen for events fired by the console client since the Firefox
> + * connector (data provider) is paused.
> + */
> +function waitForPausedRequest(connector) {

nit: could we find a better name, i find this one to be a bit misleading
Attachment #8914263 - Flags: review?(nchevobbe) → review+
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #45)
> Comment on attachment 8914263 [details]
> Bug 1005755 - Test for pause/stop button;
> not sure if we really need this one, we already have the test in l.39
Removed

> nit: could we find a better name, i find this one to be a bit misleading
I renamed it to `waitForWebConsoleNetworkEvent`

Thanks,
Honza
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6f56f7ba748d
Introduce pause/resume button; r=rickychien
https://hg.mozilla.org/integration/autoland/rev/2bc7565e6890
Stop using Connector as a singleton; r=rickychien
https://hg.mozilla.org/integration/autoland/rev/a417d046f2f9
Update tests; r=rickychien
https://hg.mozilla.org/integration/autoland/rev/99b7794639c8
Update Net logging in the Console; r=nchevobbe
https://hg.mozilla.org/integration/autoland/rev/1d56f718253d
Test for pause/stop button; r=nchevobbe
Two follow ups:
Bug 1408964 - The Network panel should be auto resumed on reload
Bug 1408970 - Introduce test for view source in debugger

Honza
Backed out for failing clipboard's devtools/client/netmonitor/src/har/test/browser_net_har_copy_all_as_har.js:

https://hg.mozilla.org/integration/autoland/rev/8468f493cf232c317677feb6a063643ad87aaade
https://hg.mozilla.org/integration/autoland/rev/b62adcd475fe96a7b028d6b63214cd7dc837cadb
https://hg.mozilla.org/integration/autoland/rev/af10498153bcaa4dd770eac98b658761e7038b73
https://hg.mozilla.org/integration/autoland/rev/b8bbfb934bc3f0d3d936718720ffbd4640da20c9
https://hg.mozilla.org/integration/autoland/rev/19322b88b4fc477316e1ff2f9fc8a8a98d21ad9a

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=1d56f718253d6b74af4b73b65a3b8c8ce221be86&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
The test run on more platforms for the next push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=806b195734ee21a234ddf861cad843f215dc60f5&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=137207635&repo=autoland

[task 2017-10-16T10:53:13.967Z] 10:53:13     INFO - > Network events progress: 12/13, 0/0, got NetMonitor:NetworkEventUpdating:ResponseContent for server1.conn0.child1/netEvent28
[task 2017-10-16T10:53:13.968Z] 10:53:13     INFO - > Network events progress: 13/13, 0/0, got NetMonitor:NetworkEventUpdated:ResponseContent for server1.conn0.child1/netEvent28
[task 2017-10-16T10:53:13.969Z] 10:53:13     INFO - > Network events progress: 13/13, 0/0, got NetMonitor:PayloadReady for server1.conn0.child1/netEvent28
[task 2017-10-16T10:53:13.970Z] 10:53:13     INFO - Buffered messages finished
[task 2017-10-16T10:53:13.971Z] 10:53:13     INFO - TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/src/har/test/browser_net_har_copy_all_as_har.js | Uncaught exception - at resource://devtools/shared/base-loader.js -> resource://devtools/client/netmonitor/src/request-list-context-menu.js:390 - TypeError: this.getTabTarget is not a function
[task 2017-10-16T10:53:13.971Z] 10:53:13     INFO - Stack trace:
[task 2017-10-16T10:53:13.972Z] 10:53:13     INFO -     getDefaultHarOptions@resource://devtools/shared/base-loader.js -> resource://devtools/client/netmonitor/src/request-list-context-menu.js:390:16
[task 2017-10-16T10:53:13.973Z] 10:53:13     INFO -     copyAllAsHar@resource://devtools/shared/base-loader.js -> resource://devtools/client/netmonitor/src/request-list-context-menu.js:375:29
[task 2017-10-16T10:53:13.974Z] 10:53:13     INFO -     @chrome://mochitests/content/browser/devtools/client/netmonitor/src/har/test/browser_net_har_copy_all_as_har.js:30:9
[task 2017-10-16T10:53:13.975Z] 10:53:13     INFO -     Tester_execTest@chrome://mochikit/content/browser-test.js:798:9
[task 2017-10-16T10:53:13.975Z] 10:53:13     INFO -     Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:697:9
[task 2017-10-16T10:53:13.976Z] 10:53:13     INFO -     SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:795:59
[task 2017-10-16T10:53:13.977Z] 10:53:13     INFO -     Tester_execTest@chrome://mochikit/content/browser-test.js:798:9
[task 2017-10-16T10:53:13.977Z] 10:53:13     INFO -     Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:697:9
[task 2017-10-16T10:53:13.978Z] 10:53:13     INFO -     SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:795:59
[task 2017-10-16T10:53:13.979Z] 10:53:13     INFO - Leaving test bound 
[task 2017-10-16T10:53:13.980Z] 10:53:13     INFO - Removing tab.
[task 2017-10-16T10:53:13.981Z] 10:53:13     INFO - Waiting for event: 'TabClose' on [object XULElement].
[task 2017-10-16T10:53:13.981Z] 10:53:13     INFO - Got event: 'TabClose' on [object XULElement].
[task 2017-10-16T10:53:13.982Z] 10:53:13     INFO - Tab removed and finished closing
Flags: needinfo?(odvarko)
(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #54)
> Backed out for failing clipboard's
> devtools/client/netmonitor/src/har/test/browser_net_har_copy_all_as_har.js:
Ah, sorry about that!

Patch updated, issue fixed. Trying to land again.

Honza
Flags: needinfo?(odvarko)
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c0ccae3172fc
Introduce pause/resume button; r=rickychien
https://hg.mozilla.org/integration/autoland/rev/819c504f437a
Stop using Connector as a singleton; r=rickychien
https://hg.mozilla.org/integration/autoland/rev/4cc8ddaca5bd
Update tests; r=rickychien
https://hg.mozilla.org/integration/autoland/rev/463a6a947862
Update Net logging in the Console; r=nchevobbe
https://hg.mozilla.org/integration/autoland/rev/f384dd566e7f
Test for pause/stop button; r=nchevobbe
Backed out for eslint failures in browser_net_har_post_data.js and browser_net_har_post_data_on_get.js: trailing spaces not allowed:

https://hg.mozilla.org/integration/autoland/rev/816ac915cdea753ad78824382035a656d60029f0
https://hg.mozilla.org/integration/autoland/rev/7847d84440530a046d686d6b5b88b273a345fbd3
https://hg.mozilla.org/integration/autoland/rev/9c6453e78967c93d284ce61601ed96f5a05b5a3b
https://hg.mozilla.org/integration/autoland/rev/136ae00ec9380f5d7a8c8f77ba7aa1b7a2e54d88
https://hg.mozilla.org/integration/autoland/rev/5bad7469726ec3de1cd7ff7e6562f0941672cf27

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=137225251&repo=autoland
 TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/devtools/client/netmonitor/src/har/test/browser_net_har_post_data.js:20:1 | Trailing spaces not allowed. (no-trailing-spaces)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/devtools/client/netmonitor/src/har/test/browser_net_har_post_data_on_get.js:20:1 | Trailing spaces not allowed. (no-trailing-spaces)
Flags: needinfo?(odvarko)
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/34e578e0d20e
Introduce pause/resume button; r=rickychien
https://hg.mozilla.org/integration/autoland/rev/c44d05db5e64
Stop using Connector as a singleton; r=rickychien
https://hg.mozilla.org/integration/autoland/rev/00992de316f0
Update tests; r=rickychien
https://hg.mozilla.org/integration/autoland/rev/0d8950ca1882
Update Net logging in the Console; r=nchevobbe
https://hg.mozilla.org/integration/autoland/rev/f22d51b93cf8
Test for pause/stop button; r=nchevobbe
Flags: needinfo?(odvarko)
Please don't forget to set the dev-doc-needed keyword for new features like this, so they are considered in the documentation.

Sebastian
Keywords: dev-doc-needed
I've documented this, adding a section here:

https://developer.mozilla.org/en-US/docs/Tools/Network_Monitor#Pausing_and_resume_network_traffic_recording

And adding a note to the Fx58 rel notes:

https://developer.mozilla.org/en-US/Firefox/Releases/58#Developer_Tools

Let me know if this is OK. I really ought to update all the screenshots to show the new button, but that would be a nightmare and I really don't have the time right now ;-|
Keywords: dev-doc-needed → dev-doc-complete
(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #69)> Let me know if this is OK. I really ought to update all the screenshots to
> show the new button, but that would be a nightmare and I really don't have
> the time right now ;-|

Slightly off-topic, could this be something we automate using headless Firefox ? I guess it would save us quite some time, and we could have ever-green screenshots
(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #69)
> Let me know if this is OK. I really ought to update all the screenshots to
> show the new button, but that would be a nightmare and I really don't have
> the time right now ;-|
Looks good to me.
Thanks for the update!
Honza
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #70)
> (In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #69)>
> Let me know if this is OK. I really ought to update all the screenshots to
> > show the new button, but that would be a nightmare and I really don't have
> > the time right now ;-|
> 
> Slightly off-topic, could this be something we automate using headless
> Firefox ? I guess it would save us quite some time, and we could have
> ever-green screenshots

That is a good idea, certainly worth investigating. I will make a note in my to do list.
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: