1186739 - Stop using Promise.jsm in browser/base/content/test/plugins/head.js

Status: RESOLVED FIXED

(Core :: Plug-ins, defect)

Description

[Trevor Rowbotham [:rowbot]]

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

Comment 1

[Trevor Rowbotham [:rowbot]]

Attachment: bug1186739_replace_promise.jsm_with_dom_promises.diff

If no one has any objections, let's do it!

Comment 2

[Mike Conley (: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?

Comment 3

[Trevor Rowbotham [:rowbot]]

Attachment: bug1186739_replace_promise.jsm_with_dom_promises.diff

This one axes startTime as requested.

Comment 4

[Mike Conley (: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!

Comment 5

[Trevor Rowbotham [:rowbot]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c16eb0b067da

Comment 6

[Trevor Rowbotham [:rowbot]]

Try looks good.  If we're lucky, this might fix some other intermittents.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bef4a3abbeb2

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/bef4a3abbeb2