Closed Bug 1439471 Opened 7 years ago Closed 7 years ago

devtools/client/netmonitor/test/browser_net_status-bar.js fails after bug 1193394

Categories

(DevTools :: Netmonitor, defect)

defect
Not set
normal

Tracking

(firefox60 fixed)

RESOLVED FIXED
Firefox 60
Tracking Status
firefox60 --- fixed

People

(Reporter: arai, Assigned: arai)

References

Details

Attachments

(1 file, 1 obsolete file)

devtools/client/netmonitor/test/browser_net_status-bar.js https://treeherder.mozilla.org/#/jobs?repo=try&revision=c9653afa89f067d4a8146ffab41835e9153808fd&group_state=expanded&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=running&filter-resultStatus=pending&filter-resultStatus=runnable&selectedJob=163037154 > 16:14:04 ERROR - 540 INFO TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_status-bar.js | There must be load label - > 16:14:04 INFO - Stack trace: > 16:14:04 INFO - chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_status-bar.js:null:35 > 16:14:04 INFO - 541 INFO TEST-PASS | devtools/client/netmonitor/test/browser_net_status-bar.js | There must be request count label text - > 16:14:04 INFO - 542 INFO TEST-PASS | devtools/client/netmonitor/test/browser_net_status-bar.js | There must be size label text - > 16:14:04 INFO - 543 INFO TEST-PASS | devtools/client/netmonitor/test/browser_net_status-bar.js | There must be DOMContentLoaded label text - > 16:14:04 INFO - Not taking screenshot here: see the one that was previously logged > 16:14:04 ERROR - 544 INFO TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_status-bar.js | Uncaught exception - at chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_status-bar.js:48 - TypeError: onLoad is null > 16:14:04 INFO - Stack trace: > 16:14:04 INFO - @chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_status-bar.js:48:25 > 16:14:04 INFO - waitUntil@chrome://mochitests/content/browser/devtools/client/framework/test/shared-head.js:471:7 > 16:14:04 INFO - @chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_status-bar.js:48:9 > 16:14:04 INFO - Async*Tester_execTest/<@chrome://mochikit/content/browser-test.js:1067:21 > 16:14:04 INFO - Tester_execTest@chrome://mochikit/content/browser-test.js:1058:9 > 16:14:04 INFO - Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:958:9 > 16:14:04 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:795:59
reproduced locally. taking.
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
waitForTimelineMarkers waits for EVENTS.TIMELINE_EVENT, that is emitted in the 1st line, but StatusBar is updated in the 2nd line below: https://searchfox.org/mozilla-central/rev/0c0ddaa7e859a2b76a56a0e2e9c0de88af166812/devtools/client/netmonitor/src/connector/firefox-connector.js#215-216 > onDocEvent(type, event) { > window.emit(EVENTS.TIMELINE_EVENT, event); > this.actions.addTimingMarker(event); > } Once bug 1193394 is fixed, promise resolution handler for the promise resolved inside the 1st line can be executed before StatusBar is updated inside the 2nd line, if there's a MicroTask checkpoint somewhere between the promise resolution and StatusBar update. I haven't yet found the exact place of the MicroTask checkpoint tho, I suspect it's related to XBL (see bug 1416153 comment #4, basically there shouldn't be a MicroTask checkpoint inside sync JS code, but XBL accessors contain it) Anyway, currently the promise returned by waitForTimelineMarkers is resolved before the StatusBar is updated, and the remaining part after `await Promise.all([requestsDone, markersDone]);` can be executed before `onLoad` element is created once bug 1193394 is fixed. I added `await new Promise(resolve => executeSoon(resolve));` after that to wait for an event tick, to make sure StatusBar is updated before getting `onLoad` element.
Attachment #8952296 - Flags: review?(odvarko)
Comment on attachment 8952296 [details] [diff] [review] Wait for the next event tick after doc-complete marker to make sure the StatusBar is updated. Review of attachment 8952296 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for working on this! I would prefer change both `onDocLoadingMarker` and `onDocEvent` as follows, which sounds more correct to me. Honza diff --git a/devtools/client/netmonitor/src/connector/firefox-connector.js b/devtools/client/netmonitor/src/connector/firefox-connector.js --- a/devtools/client/netmonitor/src/connector/firefox-connector.js +++ b/devtools/client/netmonitor/src/connector/firefox-connector.js @@ -195,30 +195,30 @@ class FirefoxConnector { onDocLoadingMarker(marker) { // Translate marker into event similar to newer "docEvent" event sent by the console // actor let event = { name: marker.name == "document::DOMContentLoaded" ? "dom-interactive" : "dom-complete", time: marker.unixTime / 1000 }; + this.actions.addTimingMarker(event); window.emit(EVENTS.TIMELINE_EVENT, event); - this.actions.addTimingMarker(event); } /** * The "DOMContentLoaded" and "Load" events sent by the console actor. * * Only used by FF60+. * * @param {object} marker */ onDocEvent(type, event) { + this.actions.addTimingMarker(event); window.emit(EVENTS.TIMELINE_EVENT, event); - this.actions.addTimingMarker(event); } /** * Send a HTTP request data payload * * @param {object} data data payload would like to sent to backend * @param {function} callback callback will be invoked after the request finished */
Attachment #8952296 - Flags: review?(odvarko)
Thanks! it solves the issue.
Attachment #8952296 - Attachment is obsolete: true
Attachment #8952941 - Flags: review?(odvarko)
Comment on attachment 8952941 [details] [diff] [review] Emit EVENTS.TIMELINE_EVENT after adding timing marker. Review of attachment 8952941 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the patch! R+ Honza
Attachment #8952941 - Flags: review?(odvarko) → review+
Pushed by arai_a@mac.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/4b263e3c54ee Emit EVENTS.TIMELINE_EVENT after adding timing marker. r=Honza
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: