Closed
Bug 1186739
Opened 7 years ago
Closed 7 years ago
Stop using Promise.jsm in browser/base/content/test/plugins/head.js
Categories
(Core Graveyard :: Plug-ins, defect)
Core Graveyard
Plug-ins
Tracking
(firefox42 fixed)
RESOLVED
FIXED
mozilla42
Tracking | Status | |
---|---|---|
firefox42 | --- | fixed |
People
(Reporter: rowbot, Assigned: rowbot)
Details
Attachments
(1 file, 1 obsolete file)
8.41 KB,
patch
|
mconley
:
review+
|
Details | Diff | Splinter Review |
Switch all the Promises in head.js[1] to use native DOM Promises instead of Promise.jsm. All tests appear to be passing with flying colors on my Win7 x64 machine when using DOM promises despite the comment in the file stating that DOM Promises were failing for unknown reasons[2]. [1] https://dxr.mozilla.org/mozilla-central/source/browser/base/content/test/plugins/head.js?from=browser/base/content/test/plugins/head.js [2] https://dxr.mozilla.org/mozilla-central/source/browser/base/content/test/plugins/head.js?from=browser/base/content/test/plugins/head.js#48
Assignee | ||
Comment 1•7 years ago
|
||
If no one has any objections, let's do it!
Assignee: nobody → smokey101stair
Status: NEW → ASSIGNED
Attachment #8637604 -
Flags: review?(mconley)
Comment 2•7 years ago
|
||
Comment on attachment 8637604 [details] [diff] [review] bug1186739_replace_promise.jsm_with_dom_promises.diff Review of attachment 8637604 [details] [diff] [review]: ----------------------------------------------------------------- This looks good to me! Just minusing to get startTime removed. The next one will be a plus! Thanks Trevor! ::: browser/base/content/test/plugins/head.js @@ +33,5 @@ > * @returns a Promise that resolves to true after the time has elapsed > */ > function waitForMs(aMs) { > + return new Promise((resolve) => { > + let startTime = Date.now(); I don't think startTime is used for anything. Let's axe it. @@ -45,5 @@ > } > > - > -// DOM Promise fails for unknown reasons here, so we're using > -// resource://gre/modules/Promise.jsm. I guess this is no longer the case?
Attachment #8637604 -
Flags: review?(mconley) → review-
Assignee | ||
Comment 3•7 years ago
|
||
This one axes startTime as requested.
Attachment #8637604 -
Attachment is obsolete: true
Attachment #8638606 -
Flags: review?(mconley)
Comment 4•7 years ago
|
||
Comment on attachment 8638606 [details] [diff] [review] bug1186739_replace_promise.jsm_with_dom_promises.diff Review of attachment 8638606 [details] [diff] [review]: ----------------------------------------------------------------- Looks great! Thanks Trevor!
Attachment #8638606 -
Flags: review?(mconley) → review+
Assignee | ||
Comment 5•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c16eb0b067da
Assignee | ||
Comment 6•7 years ago
|
||
Try looks good. If we're lucky, this might fix some other intermittents.
Keywords: checkin-needed
Comment 8•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/bef4a3abbeb2
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Updated•3 months ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•