1048098 - Remove SpecialPowers.flushPermissions, flushPrefEnv from tearing down procedure

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Vicamo Yang [:vicamo][:vyang]]

"testing/specialpowers/content/specialpowersAPI.js" has specified that permissions pushed by pushPermissions() and preference settings pushed by pushPrefEnv() will be reverted when the test finishes.  Therefore there is no reason to include any of them in test case tearing down procedure.

Besides, I observed some failures due to timeouts in waiting for flushPermissions/flushPrefEnv returns.

Known affected are:

dom/mobilemessage/tests/marionette/head.js:571:    SpecialPowers.flushPermissions(function() {
dom/mobilemessage/tests/marionette/head.js:574:      SpecialPowers.flushPrefEnv(function() {
dom/mobileconnection/tests/marionette/head.js:1135:    SpecialPowers.flushPermissions(function() {
dom/bluetooth/tests/marionette/head.js:712:    SpecialPowers.flushPermissions(function() {
dom/bluetooth2/tests/marionette/head.js:633:    SpecialPowers.flushPermissions(function() {
dom/cellbroadcast/tests/marionette/head.js:324:    SpecialPowers.flushPermissions(function() {

Comment 1

[Vicamo Yang [:vicamo][:vyang]]

Attachment: patch

Comment 2

[Vicamo Yang [:vicamo][:vyang]]

Attachment: bug-1048098-remove-redundant-flushPermissions-flushPrefEnv.patch

Sorry, wrong file attached.

Comment 4

[Edgar Chen [:edgar]]

Comment on attachment 8466869 [details] [diff] [review]
bug-1048098-remove-redundant-flushPermissions-flushPrefEnv.patch

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

r=me on the cellbroadcast/mobileconnection/mobilemessage changes.

::: dom/cellbroadcast/tests/marionette/head.js
@@ +316,5 @@
>    return Promise.all(promises).then(aResults => aResults[0].message);
>  }
>  
>  /**
>   * Flush permission settings and call |finish()|.

nit: Since we don't flush permission any more, could you help to revise the comments? Thank you.

::: dom/mobileconnection/tests/marionette/head.js
@@ +1127,5 @@
>    return _numOfRadioInterfaces;
>  }
>  
>  /**
>   * Flush permission settings and call |finish()|.

Ditto

::: dom/mobilemessage/tests/marionette/head.js
@@ +561,4 @@
>  }
>  
>  /**
>   * Flush permission settings and call |finish()|.

Ditto

Comment 5

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Edgar Chen [:edgar][:echen] from comment #4)
> nit: Since we don't flush permission any more, could you help to revise the
> comments? Thank you.

Ah, yes. Didn't notice them.

Comment 6

[Ben Tian (inactive)]

Comment on attachment 8466869 [details] [diff] [review]
bug-1048098-remove-redundant-flushPermissions-flushPrefEnv.patch

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

r=me with comment addressed. Thanks.

::: dom/bluetooth/tests/marionette/head.js
@@ +704,5 @@
>    return deferred.promise;
>  }
>  
>  /**
>   * Flush permission settings and call |finish()|.

Please revise comment as other files.

::: dom/bluetooth2/tests/marionette/head.js
@@ +625,5 @@
>    return true;
>  }
>  
>  /**
>   * Flush permission settings and call |finish()|.

Ditto.

Comment 7

[Vicamo Yang [:vicamo][:vyang]]

Attachment: patch : v2

Correct function documentation to reflect the changes. Address review comment 4 and comment 6.

Comment 8

[Vicamo Yang [:vicamo][:vyang]]

https://hg.mozilla.org/integration/b2g-inbound/rev/9281f9896ce0

Comment 9

[Ed Morley [:emorley]]

Backed out for webapi test failures:
https://tbpl.mozilla.org/php/getParsedLog.php?id=45234928&tree=B2g-Inbound

remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/f37209067340

Comment 10

[Vicamo Yang [:vicamo][:vyang]]

One more flushPermissions in dom/events/test, but that should not be a problem anyway. Included in https://tbpl.mozilla.org/?tree=Try&rev=ed773e1fbdea

Comment 11

[Vicamo Yang [:vicamo][:vyang]]

Attachment: patch : v3

Include patch to dom/events/test .

Comment 12

[Vicamo Yang [:vicamo][:vyang]]

https://hg.mozilla.org/integration/b2g-inbound/rev/95aa9681d4f8

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/95aa9681d4f8