1419336 - HTTP inspector do not update the Response panel if it was opened before the Request completed

Status: RESOLVED FIXED

(DevTools :: Console, defect, P1)

Description

[Nicolas Chevobbe [:nchevobbe]]

Steps to reproduce:
1. Go to data:text/html,Test long XHR<script>const headers = new Headers(); headers.append("Accept", "application/json");fetch("https://httpstat.us/200?sleep=20000&test=true", {method: "POST", headers})</script>
2. Open the console
3. Make sure you have the XHR filter turned on
4. Reload the page
5. You should see an XHR request being logged, click on it to expand it
6. Go to the Response tab

Expected results:
The request is resolved after 20s, and at that time, the response tab panel content should be updated with the response (200 OK). This is what is done in the netmonitor

Actual results:
The panel content stays empty

Comment 1

[gvl]

Duplicate of: https://bugzilla.mozilla.org/show_bug.cgi?id=1419300
Should have CC-ed you also.

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: Bug 1419336 - Properly update HTTPi UI if a request is opened before finish;

Review commit: https://reviewboard.mozilla.org/r/204252/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/204252/

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8933335 [details]
Bug 1419336 - Properly update HTTPi UI if a request is opened before finish;

https://reviewboard.mozilla.org/r/204252/#review209870

A couple of small nits, but this looks good.
This should be covered by a test (but I know you are working on it)

::: devtools/client/webconsole/new-console-output/store.js:231
(Diff revision 1)
>            });
>          }
>        }
>  
> -      // Process all incoming HTTP details packets.
> +      // Process all incoming HTTP details packets. Note that
> +      // Network event update packets and sent in batches from:

packets and sent → packets are sent

::: devtools/client/webconsole/new-console-output/store.js:243
(Diff revision 1)
>          let open = getAllMessagesUiById(state).includes(actor);
>          if (open) {
> -          dataProvider.onNetworkEventUpdate(null, action.response);
> +          let message = getMessage(state, actor);
> +          message.updates.forEach(updateType => {
> +            dataProvider.onNetworkEventUpdate(null, {
> +              packet: { updateType: updateType },

nit: we could write `packet: { updateType }`

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: Bug 1419336 - Update tests;

Review commit: https://reviewboard.mozilla.org/r/204566/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/204566/

Comment 6

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8933335 [details]
Bug 1419336 - Properly update HTTPi UI if a request is opened before finish;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/204252/diff/1-2/

Comment 7

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #4)
> Comment on attachment 8933335 [details]
> Bug 1419336 - Properly update HTTPi UI if a request is opened before finish;
Fixed thanks Nicolas!

Honza

Comment 8

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8933619 [details]
Bug 1419336 - Update tests;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/204566/diff/1-2/

Comment 9

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8933619 [details]
Bug 1419336 - Update tests;

https://reviewboard.mozilla.org/r/204566/#review210158

Thanks Honza for the updated test. I have questions though, see the inline comments :)

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_network_messages_expand.js:67
(Diff revision 2)
> +
> +async function openRequestBeforeUpdates(target, hud) {
> +  let toolbox = gDevTools.getToolbox(target);
> +
> +  // Fire an XHR POST request.
> +  await ContentTask.spawn(gBrowser.selectedBrowser, null, function () {

I think we can remove await here, since we already wait later for the network message to be shown

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_network_messages_expand.js:77
(Diff revision 2)
> +
> +  let xhrUrl = TEST_PATH + "sjs_slow-response-test-server.sjs";
> +  let messageNode = await waitFor(() => findMessage(hud, xhrUrl));
> +  let urlNode = messageNode.querySelector(".url");
> +  info("Network message found.");
> +

here, can we make sure that we don't have anything shown in the panel ? I'd like the test to fail if it's not the case, since it's the STR that we are trying to cover

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_network_messages_expand.js:86
(Diff revision 2)
> +  // Expand network log
> +  urlNode.click();
> +
> +  await updates;
> +  await payload;
> +  await testNetworkMessage(messageNode);

so here, we are only testing the str for the headers panel.

In testNetworkMessage we do click on the other panels, but I think it would be nice to check if each panel is updated with the following pattern : 

- fire long xhr
- select request
- click on panel
- when request finishes, make sure panel is updated
- make sure other panels have expected data

This will be longer and verbose for sure, but since we are currently making everything lazy, I think it woulb be good to cover all the possible failures.

What do you think about this Honza ? Does it look like an overkill ?

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_network_messages_expand.js:102
(Diff revision 2)
> -  // Headers tab should be selected by default, so just check its content.
> +  // Check Headers tab.
> +  headersTab.click();

why do we have to make this change ?

Comment 10

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8933335 [details]
Bug 1419336 - Properly update HTTPi UI if a request is opened before finish;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/204252/diff/2-3/

Comment 11

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8933619 [details]
Bug 1419336 - Update tests;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/204566/diff/2-3/

Comment 12

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8933619 [details]
Bug 1419336 - Update tests;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/204566/diff/3-4/

Comment 13

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #9)
> > +async function openRequestBeforeUpdates(target, hud) {
> > +  let toolbox = gDevTools.getToolbox(target);
> > +
> > +  // Fire an XHR POST request.
> > +  await ContentTask.spawn(gBrowser.selectedBrowser, null, function () {
> 
> I think we can remove await here, since we already wait later for the
> network message to be shown
Done

> :::
> devtools/client/webconsole/new-console-output/test/mochitest/
> browser_webconsole_network_messages_expand.js:77
> (Diff revision 2)
> > +
> > +  let xhrUrl = TEST_PATH + "sjs_slow-response-test-server.sjs";
> > +  let messageNode = await waitFor(() => findMessage(hud, xhrUrl));
> > +  let urlNode = messageNode.querySelector(".url");
> > +  info("Network message found.");
> > +
> 
> here, can we make sure that we don't have anything shown in the panel ? I'd
> like the test to fail if it's not the case, since it's the STR that we are
> trying to cover
Done

> :::
> devtools/client/webconsole/new-console-output/test/mochitest/
> browser_webconsole_network_messages_expand.js:86
> (Diff revision 2)
> > +  // Expand network log
> > +  urlNode.click();
> > +
> > +  await updates;
> > +  await payload;
> > +  await testNetworkMessage(messageNode);
> 
> so here, we are only testing the str for the headers panel.
> 
> In testNetworkMessage we do click on the other panels, but I think it would
> be nice to check if each panel is updated with the following pattern : 
> 
> - fire long xhr
> - select request
> - click on panel
> - when request finishes, make sure panel is updated
> - make sure other panels have expected data
Also done

> This will be longer and verbose for sure, but since we are currently making
> everything lazy, I think it woulb be good to cover all the possible failures.
> 
> What do you think about this Honza ? Does it look like an overkill ?
No, I think it's nice to have one test that checks functionality
of the entire HTTPi feature in the Console.

> :::
> devtools/client/webconsole/new-console-output/test/mochitest/
> browser_webconsole_network_messages_expand.js:102
> (Diff revision 2)
> > -  // Headers tab should be selected by default, so just check its content.
> > +  // Check Headers tab.
> > +  headersTab.click();
> 
> why do we have to make this change ?
The test is now quite different, but essentially this ensures
that the right tab is selected (since HTTPi is remembering the last 
selected tab).

Thanks for review!
Honza

Comment 14

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8933619 [details]
Bug 1419336 - Update tests;

https://reviewboard.mozilla.org/r/204566/#review211318

a few nits, but this test is now extensive, i like that :D

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_network_messages_expand.js:66
(Diff revision 4)
> +  // 1. Execute long-time request
> +  // 2. Expand the net log before the request finishes (set default tab)
> +  // 3. Check the default tab empty content
> +  // 4. Wait till the request finishes
> +  // 5. Check content of all tabs
> +  for (let i = 0; i < tabs.length; i++) {

nit : use a for of loop

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_network_messages_expand.js:76
(Diff revision 4)
> +async function openRequestAfterUpdates(target, hud) {
>    let toolbox = gDevTools.getToolbox(target);
>  
>    // Fire an XHR POST request.
> -  await ContentTask.spawn(gBrowser.selectedBrowser, null, function () {
> -    content.wrappedJSObject.testXhrPost();
> +  ContentTask.spawn(gBrowser.selectedBrowser, null, function () {
> +    content.wrappedJSObject.testXhrPostSlowResponse();

do we need to use the slow response here ? Since we wait for the update before expanding, it should not be necessary

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_network_messages_expand.js:109
(Diff revision 4)
> +  let xhrUrl = TEST_PATH + "sjs_slow-response-test-server.sjs";
> +  let messageNode = await waitFor(() => findMessage(hud, xhrUrl));

can we use waitForMessage , and then fire the xhr ? waitFor uses polling, and since the response is slow, there is a risk of timeout (by default, it polls 500 times, on a 50ms interval)

we should only use waitFor when we can't predict when messages will be logged.

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_network_messages_expand.js:167
(Diff revision 4)
> +  await waitUntil(() => {
> +    return !!messageNode.querySelector("#headers-panel .headers-overview");
> +  });

can we use waitOnMutation here instead ?

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_network_messages_expand.js:214
(Diff revision 4)
> +function testEmptyResponse(messageNode) {
> +  let panel = messageNode.querySelector("#response-panel .tab-panel");
> +  is(panel.textContent, "", "Cookies tab is empty");
> +}

I was going to suggest that we could have only one function to test panel emptiness, but some have an .empty-notice element, others have nothing.

It might be good to open a bug to unify the behaviour of those panels.
Also , i think we should differentiate loading state and empty ones. Here, we should have a "loading" message, until the data is loaded, and then, we can say if the panel does not have data. For long request, it would make sense, since now, the user sees an "empty" message, and then some data when the response resolves.

this could be a css rule we apply on :empty panel (https://codepen.io/oslego/pen/XdvWmd)

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_network_messages_expand.js:266
(Diff revision 4)
> +  await waitUntil(() => {
> +    return !!messageNode.querySelector("#stack-trace-panel .frame-link");
> +  });

could we use waitformutation ?

Comment 15

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #14)
> Comment on attachment 8933619 [details]
> Bug 1419336 - Update tests;
> 
> https://reviewboard.mozilla.org/r/204566/#review211318
> 
> a few nits, but this test is now extensive, i like that :D
> 
> :::
> devtools/client/webconsole/new-console-output/test/mochitest/
> browser_webconsole_network_messages_expand.js:66
> (Diff revision 4)
> > +  // 1. Execute long-time request
> > +  // 2. Expand the net log before the request finishes (set default tab)
> > +  // 3. Check the default tab empty content
> > +  // 4. Wait till the request finishes
> > +  // 5. Check content of all tabs
> > +  for (let i = 0; i < tabs.length; i++) {
> 
> nit : use a for of loop
Done

> 
> :::
> devtools/client/webconsole/new-console-output/test/mochitest/
> browser_webconsole_network_messages_expand.js:76
> (Diff revision 4)
> > +async function openRequestAfterUpdates(target, hud) {
> >    let toolbox = gDevTools.getToolbox(target);
> >  
> >    // Fire an XHR POST request.
> > -  await ContentTask.spawn(gBrowser.selectedBrowser, null, function () {
> > -    content.wrappedJSObject.testXhrPost();
> > +  ContentTask.spawn(gBrowser.selectedBrowser, null, function () {
> > +    content.wrappedJSObject.testXhrPostSlowResponse();
> 
> do we need to use the slow response here ? Since we wait for the update
> before expanding, it should not be necessary
The test needs it because of cookies (the other response doesn't have cookies)

> 
> :::
> devtools/client/webconsole/new-console-output/test/mochitest/
> browser_webconsole_network_messages_expand.js:109
> (Diff revision 4)
> > +  let xhrUrl = TEST_PATH + "sjs_slow-response-test-server.sjs";
> > +  let messageNode = await waitFor(() => findMessage(hud, xhrUrl));
> 
> can we use waitForMessage , and then fire the xhr ? waitFor uses polling,
> and since the response is slow, there is a risk of timeout (by default, it
> polls 500 times, on a 50ms interval)
> 
> we should only use waitFor when we can't predict when messages will be
> logged.
Done

> 
> :::
> devtools/client/webconsole/new-console-output/test/mochitest/
> browser_webconsole_network_messages_expand.js:167
> (Diff revision 4)
> > +  await waitUntil(() => {
> > +    return !!messageNode.querySelector("#headers-panel .headers-overview");
> > +  });
> 
> can we use waitOnMutation here instead ?

What's wrong with `waitUntil(() => query-selector);` it's nicely waiting for an element to appear. `waitForNodeMutation` needs the node and so it's more verbose to use, `waitUntil` works with selector only.

From Slack:
jdescottes > (fwiw I'm in the process of removing all the mutation based tests in about:debugging because they are so fragile they race all the time)
especially true in react based apps, where you don't actually control the mutations that will happen on the page

> 
> :::
> devtools/client/webconsole/new-console-output/test/mochitest/
> browser_webconsole_network_messages_expand.js:214
> (Diff revision 4)
> > +function testEmptyResponse(messageNode) {
> > +  let panel = messageNode.querySelector("#response-panel .tab-panel");
> > +  is(panel.textContent, "", "Cookies tab is empty");
> > +}
> 
> I was going to suggest that we could have only one function to test panel
> emptiness, but some have an .empty-notice element, others have nothing.
> 
> It might be good to open a bug to unify the behaviour of those panels.
> Also , i think we should differentiate loading state and empty ones. Here,
> we should have a "loading" message, until the data is loaded, and then, we
> can say if the panel does not have data. For long request, it would make
> sense, since now, the user sees an "empty" message, and then some data when
> the response resolves.
> 
> this could be a css rule we apply on :empty panel
> (https://codepen.io/oslego/pen/XdvWmd)
Sounds good to me, can you please file a bug for this. You'll describe better
what you have in mind.

> 
> :::
> devtools/client/webconsole/new-console-output/test/mochitest/
> browser_webconsole_network_messages_expand.js:266
> (Diff revision 4)
> > +  await waitUntil(() => {
> > +    return !!messageNode.querySelector("#stack-trace-panel .frame-link");
> > +  });
> 
> could we use waitformutation ?
See above

Thanks for the review Nicolas!
Honza

Comment 16

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8933335 [details]
Bug 1419336 - Properly update HTTPi UI if a request is opened before finish;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/204252/diff/3-4/

Comment 17

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8933619 [details]
Bug 1419336 - Update tests;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/204566/diff/4-5/

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1cd28aa89b3e
Properly update HTTPi UI if a request is opened before finish; r=nchevobbe
https://hg.mozilla.org/integration/autoland/rev/572bb0e72616
Update tests; r=nchevobbe

Comment 19

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for build bustage on Linux, OS X and mingw32 and failing modified devtools test browser_webconsole_network_messages_expand.js on Windows:

https://hg.mozilla.org/integration/autoland/rev/5fcb3d79de4abfb081b9aa255f7dc5fda8653cc9

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=572bb0e726162c903bfb9e9c67226a43a7988eee&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception

Build bustage: https://treeherder.mozilla.org/logviewer.html#?job_id=150220723&repo=autoland

[task 2017-12-06T16:24:11.706Z] 16:24:11     INFO -  mozpack.errors.ErrorMessage: Symlink target path does not exist: /builds/worker/workspace/build/src/devtools/client/webconsole/new-console-output/test/mochitest/sjs_slow-response-test-server.sjs
[task 2017-12-06T16:24:11.706Z] 16:24:11     INFO -  Makefile:188: recipe for target 'install-test-files' failed
[task 2017-12-06T16:24:11.706Z] 16:24:11     INFO -  gmake[2]: *** [install-test-files] Error 1
[task 2017-12-06T16:24:11.706Z] 16:24:11     INFO -  gmake[2]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox'
[task 2017-12-06T16:24:11.706Z] 16:24:11     INFO -  /builds/worker/workspace/build/src/build/moz-automation.mk:98: recipe for target 'automation/package-tests' failed
[task 2017-12-06T16:24:11.706Z] 16:24:11     INFO -  gmake[1]: *** [automation/package-tests] Error 2
[task 2017-12-06T16:24:11.706Z] 16:24:11     INFO -  gmake[1]: *** Waiting for unfinished jobs....

Devtools log: https://treeherder.mozilla.org/logviewer.html#?job_id=150234353&repo=autoland
17:10:17     INFO -  2 INFO TEST-START | devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_network_messages_expand.js
17:10:33     INFO -  GECKO(3104) | [DEBUG SHUTDOWN] Shutdown: decoder=0DA26280 state machine=0D9A5800
17:10:33     INFO -  GECKO(3104) | [DEBUG SHUTDOWN] Enter: state machine=05DE5420 reader=08E087B0
17:10:33     INFO -  GECKO(3104) | [DEBUG SHUTDOWN] Shutdown: reader=010CC000 shutdown demuxer=05DE52B8
17:10:33     INFO -  GECKO(3104) | [DEBUG SHUTDOWN] Shutdown: pool=08EF8A60 count=1
17:10:33     INFO -  GECKO(3104) | [DEBUG SHUTDOWN] operator (): pool=08EF8A60 shutdown=true count=0
17:10:33     INFO -  GECKO(3104) | [DEBUG SHUTDOWN] TearDownDecoders: reader=010CC000 shut down audio task queue
17:10:33     INFO -  GECKO(3104) | [DEBUG SHUTDOWN] TearDownDecoders: reader=010CC000 shut down video task queue
17:10:33     INFO -  GECKO(3104) | [DEBUG SHUTDOWN] FinishShutdown: state machine=0D9A5800
17:10:33     INFO -  GECKO(3104) | [DEBUG SHUTDOWN] Unregister: decoder=0DA26280, count=0
17:11:02     INFO -  TEST-INFO | started process screenshot
17:11:02     INFO -  TEST-INFO | screenshot: exit 0
17:11:02     INFO -  Buffered messages logged at 17:10:17
17:11:02     INFO -  3 INFO Entering test bound task
17:11:02     INFO -  4 INFO Adding a new tab with URL: http://example.com/browser/devtools/client/webconsole/new-console-output/test/mochitest/test-network-request.html
17:11:02     INFO -  5 INFO Console message: [JavaScript Warning: "Use of nsIFile in content process is deprecated." {file: "resource://gre/modules/FileUtils.jsm" line: 174}]
17:11:02     INFO -  6 INFO Console message: [JavaScript Warning: "Use of nsIFile in content process is deprecated." {file: "resource://gre/modules/FileUtils.jsm" line: 174}]
17:11:02     INFO -  7 INFO Tab added and finished loading
17:11:02     INFO -  8 INFO Opening the toolbox
17:11:02     INFO -  Buffered messages logged at 17:10:18
17:11:02     INFO -  9 INFO Console message: [JavaScript Warning: "Unknown property ‘user-select’.  Declaration dropped." {file: "resource://devtools/client/shared/components/reps/reps.css" line: 251 column: 13 source: "  user-select: none;"}]
17:11:02     INFO -  10 INFO Console message: [JavaScript Warning: "Unknown pseudo-class or pseudo-element ‘selection’.  Ruleset ignored due to bad selector." {file: "chrome://devtools/content/sourceeditor/codemirror/lib/codemirror.css" line: 232 column: 29 source: ".CodeMirror-gutter-wrapper ::selection { background-color: transparent }"}]
17:11:02     INFO -  11 INFO Console message: [JavaScript Warning: "Unknown property ‘-moz-border-radius’.  Declaration dropped." {file: "chrome://devtools/content/sourceeditor/codemirror/lib/codemirror.css" line: 241 column: 20 source: "  -moz-border-radius: 0; -webkit-border-radius: 0; border-radius: 0;"}]
17:11:02     INFO -  12 INFO Console message: [JavaScript Warning: "Unknown pseudo-class or pseudo-element ‘selection’.  Ruleset ignored due to bad selector." {file: "chrome://devtools/content/sourceeditor/codemirror/lib/codemirror.css" line: 324 column: 18 source: ".CodeMirror-line::selection, .CodeMirror-line > span::selection, .CodeMirror-line > span > span::selection { background: #d7d4f0; }"}]
17:11:02     INFO -  13 INFO Console message: [JavaScript Warning: "Unknown property ‘user-select’.  Declaration dropped." {file: "resource://devtools/client/shared/components/reps/reps.css" line: 251 column: 13 source: "  user-select: none;"}]
17:11:02     INFO -  14 INFO Toolbox opened and focused
17:11:02     INFO -  15 INFO Matched a message with text: "http://example.com/browser/devtools/client/webconsole/new-console-output/test/mochitest/sjs_slow-response-test-server.sjs", all messages received.
17:11:02     INFO -  16 INFO Network message found.
17:11:02     INFO -  Buffered messages finished
17:11:02    ERROR -  17 INFO TEST-UNEXPECTED-FAIL | devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_network_messages_expand.js | Test timed out -

Comment 20

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8933335 [details]
Bug 1419336 - Properly update HTTPi UI if a request is opened before finish;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/204252/diff/4-5/

Comment 21

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8933619 [details]
Bug 1419336 - Update tests;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/204566/diff/5-6/

Comment 22

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Missing file added.

Try looks good:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2f1016c5648f2669cf5e056c7c95dfcc644a1565

Trying to land again.

Honza

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c7ecad9b8aad
Properly update HTTPi UI if a request is opened before finish; r=nchevobbe
https://hg.mozilla.org/integration/autoland/rev/18135f76a2bc
Update tests; r=nchevobbe

Comment 24

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ee34a2acdc48
Backed out 2 changesets for failing devtools tests  devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_network_messages_expand.js r=backout on a CLOSED TREE

Comment 25

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for failing devtools tests devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_network_messages_expand.js:

https://hg.mozilla.org/integration/autoland/rev/ee34a2acdc489510596e000eacd30f84f1d98726

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=18135f76a2bc928689d542fd54c9760755b266d3&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&selectedJob=150475812
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=150475812&repo=autoland

12:44:11     INFO -  776 INFO TEST-PASS | devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_network_messages_expand.js | Timings text is available -
12:44:11     INFO -  777 INFO TEST-PASS | devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_network_messages_expand.js | StackTrace tab is available -
12:44:11     INFO -  778 INFO Leaving test bound task
12:44:11     INFO -  779 INFO Console message: [JavaScript Error: "TypeError: this.webConsoleFrame is null" {file: "resource://devtools/shared/base-loader.js -> resource://devtools/client/webconsole/webconsole-connection-proxy.js" line: 247}]
12:44:11     INFO -  780 INFO Removing tab.
12:44:11     INFO -  781 INFO Waiting for event: 'TabClose' on [object XULElement].
12:44:11     INFO -  782 INFO Got event: 'TabClose' on [object XULElement].
12:44:11     INFO -  783 INFO Tab removed and finished closing
12:44:11     INFO -  Buffered messages finished
12:44:11    ERROR -  784 INFO TEST-UNEXPECTED-FAIL | devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_network_messages_expand.js | A promise chain failed to handle a rejection: this.webConsoleFrame is null - stack: null
12:44:11     INFO -  Rejection date: Thu Dec 07 2017 12:44:11 GMT+0000 (Coordinated Universal Time) - false == true - JS frame :: resource://testing-common/PromiseTestUtils.jsm :: assertNoUncaughtRejections :: line 253
12:44:11     INFO -  Stack trace:
12:44:11     INFO -      resource://testing-common/PromiseTestUtils.jsm:assertNoUncaughtRejections:253
12:44:11     INFO -      chrome://mochikit/content/browser-test.js:Tester.prototype.nextTest<:703
12:44:11     INFO -      testScope/test_finish/<@chrome://mochikit/content/browser-test.js:1361:11
12:44:11     INFO -      run@chrome://mochikit/content/browser-test.js:1298:9

Comment 26

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8933335 [details]
Bug 1419336 - Properly update HTTPi UI if a request is opened before finish;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/204252/diff/5-6/

Comment 27

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8933619 [details]
Bug 1419336 - Update tests;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/204566/diff/6-7/

Comment 28

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I fixed the problem and mach test --verify works for me locally.
Let's try to land this now!

Honza

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/916157a780b9
Properly update HTTPi UI if a request is opened before finish; r=nchevobbe
https://hg.mozilla.org/integration/autoland/rev/118dd0da02d7
Update tests; r=nchevobbe

Comment 30

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/916157a780b9
https://hg.mozilla.org/mozilla-central/rev/118dd0da02d7

Comment 31

[Mohammed Adam]

I have Tried to Reproduce this same bug with Firefox Beta 59.0b14 (32bit) on Ubuntu Mate (Linux 4.13.0-36-generic) 

But the Bug has been Fixed. 
--------------------------------------------------------------------------------
Firefox Version 59.0b14
Build ID: 20180301022608
User Agent: Mozilla/5.0 (X11; Linux i686; rv:59.0) Gecko/20100101 Firefox/59.0
OS: Linux 4.13.0-36-generic
--------------------------------------------------------------------------------

Status: Fixed & Verified

[testday-20180302]