Closed Bug 1626286 Opened 5 years ago Closed 5 years ago

Cached messages may be rendered in the wrong order

Categories

(DevTools :: Console, defect, P1)

defect

Tracking

(Fission Milestone:M6, firefox-esr68 unaffected, firefox74 unaffected, firefox75 unaffected, firefox76 fixed)

RESOLVED FIXED
Firefox 76
Fission Milestone M6
Tracking Status
firefox-esr68 --- unaffected
firefox74 --- unaffected
firefox75 --- unaffected
firefox76 --- fixed

People

(Reporter: nchevobbe, Assigned: nchevobbe)

References

(Regression)

Details

(Keywords: regression, Whiteboard: dt-fission-m2-mvp)

Attachments

(3 files)

Steps to reproduce

  1. Go to data:text/html,<meta charset=utf8><script>(async function() { console.log("first"); await new Promise(res => setTimeout(res, 1000)); x.sdfsdf.sdf; })()</script>
  2. Wait 2s and open the console

Expected results

I see 2 messages in the console:

  • first
  • ReferenceError: x is not defined

Actual results

I do see 2 messages in the console, but they're in the wrong order:

  • ReferenceError: x is not defined
  • first

Since Bug 1620234, we now have 2 different places where we retrieve the cached messages, and we first render the cached error messages, and then the cached console messages.

we need to fix this and find a solution that won't harm performances.

In the console redux store, we assume that all the messages are added in chronological order, and we put them in a Map so we can guarantee the order to then build an array of the visible messages.

The first solution that comes to mind is that we should wait until we have all the cached messages, then sort them by their timestamps, and only then, add them to the redux store.
The issue here is how we can detect that we have all the cached messages; with the ResourceWatcher, every message, from the cache or not, will be handled as a single unit by WebConsoleUI#_onResourceAvailable. We can pass extra data to the resource so we know the message comes from the cache for example. The hard part still is how to detect that we retrieved all the caches, since some of them can be empty (thus, won't trigger onResourceAvailable).

Another solution could be to reorder the messages in the reducer if a cached message comes in. I'm afraid this will be quite costly because of the map holding all the messages (we'd have to re-create it for each new cache message). Or we could represent the order of those messages in a different store property, like we do for visibleMessages for example.

Since code freeze is tomorrow, I may simply revert the retrieval of cached console api messages to the same place as we're getting the cached page errors.
I'll file a follow up bugs to handle cached message with the resource watcher.

Tracking Fission DevTools bugs for Fission Nightly (M6) milestone

Fission Milestone: --- → M6

We need to figure out how to efficiently resolve the issue
of the order of cached messages of different types before
using the resourceWatcher.
This patch revert some parts of Bug 1620234 so the console
will work as it was before.

Blocks: 1627167
Pushed by nchevobbe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/47f82e89f357 Revert usage of ResourceWatcher for ConsoleAPI messages. r=Honza.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 76

Since we're going to re-order the packets in the webconsole messages
reducer based on the messages timestamps, the node tests are going
to fail, as stubs timeStamps are generated and we don't have much
control on them.
Assigning the same timestamp means we'll defer to order based on the
message id, which should follow the order of insertion. The only exception
is network messages, where the id needs to be the actor, which is a string.
For the tests that are using network messages and assert some order, we
simply reassign the timestamps so we can guarantee the order.

Depends on D69004

QA Whiteboard: [qa-76b-p2]
Pushed by csabou@mozilla.com: https://hg.mozilla.org/mozilla-central/rev/cc342696a392 Assign the same timestamp to all WebConsole stubs. r=Honza. https://hg.mozilla.org/mozilla-central/rev/2e804f3812b9 Fix order of cached messages in the reducer. r=jlast.
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: