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)
DevTools
Netmonitor
Tracking
(firefox60 fixed)
RESOLVED
FIXED
Firefox 60
Tracking | Status | |
---|---|---|
firefox60 | --- | fixed |
People
(Reporter: arai, Assigned: arai)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Honza
:
review+
|
Details | Diff | Splinter Review |
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
Assignee | ||
Comment 1•7 years ago
|
||
reproduced locally.
taking.
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•7 years ago
|
||
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 3•7 years ago
|
||
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)
Assignee | ||
Comment 4•7 years ago
|
||
Thanks!
it solves the issue.
Attachment #8952296 -
Attachment is obsolete: true
Attachment #8952941 -
Flags: review?(odvarko)
Comment 5•7 years ago
|
||
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
Assignee | ||
Comment 7•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/4b263e3c54ee64f59dc0e33633590fca3fc614e3
Bug 1439471 - Emit EVENTS.TIMELINE_EVENT after adding timing marker. r=Honza
Comment 8•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•