1118799 - Using async promises causes several tests in the timeline to fail

Status: RESOLVED FIXED

(DevTools :: Performance Tools (Profiler/Timeline), defect)

Description

[Eddy Bruel [:ejpbruel]]

In bug 1096490, I'm replacing the deprecated-sync-thenables promises, which are synchronous, with Promise.jsm promises, which are asynchronous. Doing so causes several tests in the timeline to fail.

To reproduce this issue, simply apply the attached patch.

Comment 1

[Eddy Bruel [:ejpbruel]]

Attachment: STR

Comment 2

[Victor Porof [:vporof][:vp]]

Only test that's failing is browser_timeline_overview-update.js

Comment 3

[Eddy Bruel [:ejpbruel]]

According to Victor, the problem is that browser_timeline_overview-update.js assumes that using an about:blank page does not cause any markers to be recorded. However, this assumption turns out to be false. In particular, Styles markers are being recorded. I'm not exactly sure why this didn't happen with the deprecated-sync-thenables, but it must be some timing issue.

What's more, simply changing the test to ignore Style markers is not sufficient: doing so makes the test pass locally, but not on try. Apparently, we cannot rely on the behavior of an about:blank page wrt recording, which means that these assertions are currently useless.

I'm going to file a patch in this bug that disables the offending assertions, with a follow up bug to re-enable them once we figure out a way around this.

Comment 4

[Eddy Bruel [:ejpbruel]]

Attachment: Disable faulty assertions in browser_timeline_overview-update.js

Comment 5

[Patrick Brosset <:pbro>]

Comment on attachment 8548131 [details] [diff] [review]
Disable faulty assertions in browser_timeline_overview-update.js

Review of attachment 8548131 [details] [diff] [review]:
-----------------------------------------------------------------

Ok, disabling these assertions and filing a follow-up bug to re-enable them later sounds fine to me. We need to move away from the sync promises.

Comment 6

[Eddy Bruel [:ejpbruel]]

Try run for the above patch:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=79d7e501d382

Comment 7

[Eddy Bruel [:ejpbruel]]

Try run looks green, pushing to fx-team:
https://hg.mozilla.org/integration/fx-team/rev/910019565e35

Comment 8

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

https://hg.mozilla.org/mozilla-central/rev/910019565e35

Comment 9

[David Lawrence [:dkl]]

Sorry for the spam. Moving bugs to Firefox :: Developer Tools: Performance Tools (Profiler/Timeline).

dkl