1090759 - browserElementTestHelpers.js cleanup

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Kan-Ru Chen [:kanru] (UTC+9)]

Something I think we could do:

1. Use SpecialPowers.pushPermissions instead of addPermission
2. Use DOM Promise instead of copy Promise.js from addon SDK

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Note, a patch in bug 832700 tried to change the file to use normal Promises.

Comment 2

[Kan-Ru Chen [:kanru] (UTC+9)]

(In reply to Olli Pettay [:smaug] from comment #1)
> Note, a patch in bug 832700 tried to change the file to use normal Promises.

Thanks for noticing. Looks like it doesn't land on any branch.

I have a patch for that too. We can't simply remove the boilerplate because DOM Promise behave slightly differently. The pattern used in priority/

 expectOnlyOneProcessCreated().then(function(chid) {
    childID = chid;
    return Promise.all(
      [expectPriorityChange(childID, 'FOREGROUND'),
       expectMozbrowserEvent(iframe, 'loadend'),
       expectMozbrowserEvent(iframe, 'showmodalprompt').then(function(e) {
         is(e.detail.message, 'onplay', 'showmodalprompt message');
       })]
    );
  }).then(function() {
  ...

doesn't work because the priority change event is fired before we create the expectPriorityChange promise.

I think this is because the DOM Promise uses microtasks so the observer code gets run first.

Comment 3

[Kan-Ru Chen [:kanru] (UTC+9)]

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=31db2b96ec60

Comment 4

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 1. Use pushPermissions

Comment 5

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 2. Use DOM Promise

Comment 6

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 3. Fix tests broken by DOM Promise's asyncness

Comment 7

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 4. Use "disabled" in mochitest.ini

Comment 8

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 5. Use setAttribute to set mozbrowser attr

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8514078 [details] [diff] [review]
Part 1. Use pushPermissions

Note sure why we need lockTestReady/unlockTestReady but I guess those make the
tests more reliable, since they are used to prefs etc.

Comment 10

[Kan-Ru Chen [:kanru] (UTC+9)]

remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/4613379d97a8
remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/94f7c7974cec
remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/4e2555692111
remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/95e50227bb98
remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/ffdc97783d5a

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4613379d97a8
https://hg.mozilla.org/mozilla-central/rev/94f7c7974cec
https://hg.mozilla.org/mozilla-central/rev/4e2555692111
https://hg.mozilla.org/mozilla-central/rev/95e50227bb98
https://hg.mozilla.org/mozilla-central/rev/ffdc97783d5a