Closed Bug 1407405 Opened 3 years ago Closed 3 years ago

browser_bug553455.js relies on broken Promise scheduling

Categories

(Firefox :: General, defect)

Product:
Firefox
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 58
Tracking Flags:
Tracking Status
firefox58 --- fixed

People

(Reporter: smaug, Assigned: smaug)

References

Details

Attachments

(1 file)

test_fix_for_browser_bug553455.js
2.37 KB, patch
florian
: review+
Details | Diff | Splinter Review 
See bug 1193394 for the context.

The test should pass with the patch with and without the correct promise handling.
Attachment #8917121 - Flags: review?(aswan)
Comment on attachment 8917121 [details] [diff] [review]
test_fix_for_browser_bug553455.js

It seems reasonable enough to me but somebody more familiar with PopupNotifications should take a look.
Attachment #8917121 - Flags: review?(aswan) → review?(florian)
Attachment #8917121 - Attachment is patch: true
Attachment #8917121 - Attachment mime type: application/x-javascript → text/plain
Comment on attachment 8917121 [details] [diff] [review]
test_fix_for_browser_bug553455.js

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

Looks harmless. I haven't tested locally that the test passes, I assume you have verified or will verify with try before landing.

::: browser/base/content/test/general/browser_bug553455.js
@@ +25,5 @@
>  var gApp = document.getElementById("bundle_brand").getString("brandShortName");
>  var gVersion = Services.appinfo.version;
>  
> +function waitForTick() {
> +  return new Promise(resolve => executeSoon(resolve));

Bug 1407408 landed so it seems you no longer need this.
Attachment #8917121 - Flags: review?(florian) → review+
Couldn't immediately get TestUtils.jsm work in this test file, so I'll just land the patch.
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ace9603cadb8
browser_bug553455.js relies on broken Promise scheduling, r=florian
https://hg.mozilla.org/mozilla-central/rev/ace9603cadb8
Status: NEW → RESOLVED
Closed: 3 years ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
You need to log in before you can comment on or make changes to this bug.