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 :: Plug-ins, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox42 --- fixed

People

(Reporter: rowbot, Assigned: rowbot)

Details

Attachments

(1 file, 1 obsolete file)

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
If no one has any objections, let's do it!
Assignee: nobody → smokey101stair
Status: NEW → ASSIGNED
Attachment #8637604 - Flags: review?(mconley)
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-
This one axes startTime as requested.
Attachment #8637604 - Attachment is obsolete: true
Attachment #8638606 - Flags: review?(mconley)
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+
Try looks good.  If we're lucky, this might fix some other intermittents.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/bef4a3abbeb2
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.