1404138 - New web console http inspection completely broken

Status: RESOLVED FIXED

(DevTools :: Console, defect, P1)

Description

[Ian Moody [:Kwan] (UTC+1)]

Attachment: Broken console HTTP inspection.png

Bug 1371721 completely broke HTTP request inspection in the new web console
Confirmed via mozregression and local backout of
https://hg.mozilla.org/mozilla-central/rev/ac6de5490c42
https://hg.mozilla.org/mozilla-central/rev/707d860b4e2f
https://hg.mozilla.org/mozilla-central/rev/5997563964e1
https://hg.mozilla.org/mozilla-central/rev/48478dfbe1f8
see attached image

old console is unaffected.

Comment 1

[Ian Moody [:Kwan] (UTC+1)]

Attachment: Console before 1371721 or after backout.png

Comment 2

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

Alex, I don't understand exactly the entire patch, but it seems that the problem is that created action objects are not dispatched.

See the diff bellow that fixes it for me.

Also, emitting "network-request-payload-ready" should be done just once probably outside of the cycle.

Honza

---

diff --git a/devtools/client/webconsole/new-console-output/new-console-output-wrapper.js b/devtools/client/webconsole/new-console-output/new-console-output-wrapper.js
--- a/devtools/client/webconsole/new-console-output/new-console-output-wrapper.js
+++ b/devtools/client/webconsole/new-console-output/new-console-output-wrapper.js
@@ -261,24 +261,24 @@ NewConsoleOutputWrapper.prototype = {
     this.throttledDispatchTimeout = setTimeout(() => {
       this.throttledDispatchTimeout = null;
 
       store.dispatch(actions.messagesAdd(this.queuedMessageAdds));
       this.queuedMessageAdds = [];
 
       if (this.queuedMessageUpdates.length > 0) {
         this.queuedMessageUpdates.forEach(({ message, res }) => {
-          actions.networkMessageUpdate(message);
+          store.dispatch(actions.networkMessageUpdate(message));
           this.jsterm.hud.emit("network-message-updated", res);
         });
         this.queuedMessageUpdates = [];
       }
       if (this.queuedRequestUpdates.length > 0) {
         this.queuedRequestUpdates.forEach(({ id, data}) => {
-          actions.networkUpdateRequest(id, data);
+          store.dispatch(actions.networkUpdateRequest(id, data));
           // Fire an event indicating that all data fetched from
           // the backend has been received. This is based on
           // 'FirefoxDataProvider.isQueuePayloadReady', see more
           // comments in that method.
           // (netmonitor/src/connector/firefox-data-provider).
           // This event might be utilized in tests to find the right
           // time when to finish.
           this.jsterm.hud.emit("network-request-payload-ready", {id, data});

Comment 3

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

I have also filled bug 1404227 that should improve the existing HTTPi test, so we can avoid regressions like this one.

Honza

Comment 4

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

This seems to be a duplicate of bug 1403946.

Comment 6

[Alexandre Poirot [:ochameau]]

(In reply to Jan Honza Odvarko [:Honza] from comment #2)
> Alex, I don't understand exactly the entire patch, but it seems that the
> problem is that created action objects are not dispatched.

Yes... do not hesitate to submit a patch and r? nicolas.

> Also, emitting "network-request-payload-ready" should be done just once
> probably outside of the cycle.

network-request-payload-ready used to send request id and data,
so it should be sent for each request. But I haven't looked at the listeners,
may be they do not need such details...

Looks like there is only a test, which doesn't use arguments:
http://searchfox.org/mozilla-central/source/devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_network_messages_expand.js#63

So looks like you are right.

Comment 7

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

Attachment: Bug 1404138 - Properly dispatch actions;

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

Comment 8

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

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

Honza

Comment 9

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8914683 [details]
Bug 1404138 - Properly dispatch actions;

https://reviewboard.mozilla.org/r/185998/#review191026

Looks good, thanks honza

::: devtools/client/webconsole/new-console-output/new-console-output-wrapper.js
(Diff revision 1)
> -          // (netmonitor/src/connector/firefox-data-provider).
> +        // (netmonitor/src/connector/firefox-data-provider).
> -          // This event might be utilized in tests to find the right
> +        // This event might be utilized in tests to find the right
> -          // time when to finish.
> +        // time when to finish.
> -          this.jsterm.hud.emit("network-request-payload-ready", {id, data});
> +        this.jsterm.hud.emit("network-request-payload-ready");
> -        });
> -        this.queuedRequestUpdates = [];

oh

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9bade53e420b
Properly dispatch actions; r=nchevobbe

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9bade53e420b