1586189 - Reenable quota and indexedDB tests that are skipped or expected to fail

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, task, P2)

Description

[Tom Tung [:tt, :ttung]]

dom/indexedDB/test/browser_forgetThisSite.js
test1() ~ test4() involve running cross origin tabs

dom/indexedDB/test/browser_permissionsPromptAllow.js
dom/indexedDB/test/browser_permissionsPromptDeny.js
dom/indexedDB/test/browser_permissionsPromptWorker.js
Might be related to helper functions in BrowserTestUtils or issues in Permission

dom/indexedDB/test/browser_perwindow_privateBrowsing.js
Might be related to helper functions in BrowserTestUtils

dom/indexedDB/test/test_third_party.html
The test runs cross-origin windows

Comment 1

[Simon Giesecke [:sg] [he/him]]

dom/indexedDB/test/test_third_party.html has assertion failures for four of the test cases:

Unexpected Results
------------------
dom/indexedDB/test/test_third_party.html
  FAIL Good result for testData[9] == {"host":"http://example.com","cookieBehavior":1,"expectedResult":false} - got true, expected false
    SimpleTest.is@SimpleTest/SimpleTest.js:322:16
    messageListener@dom/indexedDB/test/test_third_party.html:80:9
    EventListener.handleEvent*runTest@dom/indexedDB/test/test_third_party.html:98:14
    onload@dom/indexedDB/test/test_third_party.html:1:1
  FAIL Good result for testData[10] == {"host":"http://sub1.test2.example.org:8000","cookieBehavior":1,"expectedResult":false} - got true, expected false
    SimpleTest.is@SimpleTest/SimpleTest.js:322:16
    messageListener@dom/indexedDB/test/test_third_party.html:80:9
    EventListener.handleEvent*runTest@dom/indexedDB/test/test_third_party.html:98:14
    onload@dom/indexedDB/test/test_third_party.html:1:1
  FAIL Good result for testData[13] == {"host":"http://example.com","cookieBehavior":3,"expectedResult":false} - got true, expected false
    SimpleTest.is@SimpleTest/SimpleTest.js:322:16
    messageListener@dom/indexedDB/test/test_third_party.html:80:9
    EventListener.handleEvent*runTest@dom/indexedDB/test/test_third_party.html:98:14
    onload@dom/indexedDB/test/test_third_party.html:1:1
  FAIL Good result for testData[14] == {"host":"http://sub1.test2.example.org:8000","cookieBehavior":3,"expectedResult":false} - got true, expected false
    SimpleTest.is@SimpleTest/SimpleTest.js:322:16
    messageListener@dom/indexedDB/test/test_third_party.html:80:9
    EventListener.handleEvent*runTest@dom/indexedDB/test/test_third_party.html:98:14
    onload@dom/indexedDB/test/test_third_party.html:1:1

All other mentioned tests are timing out.

Comment 2

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1586189 - Provide useful assertion messages. r=ttung

Comment 3

[Simon Giesecke [:sg] [he/him]]

I think the test_third_party.html tests are failing because ThirdPartyUtil::IsThirdPartyWindow wrongly returns false for these cases. There is a loop iterating through towards the parent, but due to https://searchfox.org/mozilla-central/source/dom/base/ThirdPartyUtil.cpp#229, this does not go beyond process boundaries.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aac323190bcc
Provide useful assertion messages. r=ttung

Comment 5

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1586189 - Fix behaviour of ThirdPartyUtil::IsThirdPartyWindow with Fission. r=ttung

Removed expected failure with fission for dom/indexedDB/test/test_third_party.html
and dom/workers/test/test_sharedWorker_thirdparty.html.

Comment 6

[Natalia Csoregi [:nataliaCs]]

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

Comment 7

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1586189 - Re-implement waitForMessage based on waitForContentEvent;

Comment 8

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1586189 - Reenable fission tests.

Depends on D49752

Comment 9

[Simon Giesecke [:sg] [he/him]]

See https://treeherder.mozilla.org/#/jobs?repo=try&revision=db94c4907bb4bbc539ca141644812d05524d24d9 for the test result with the reworked waitForMessage implementation (also for dom/quota tests).

Comment 10

[Tom Tung [:tt, :ttung]]

try for all platforms: https://treeherder.mozilla.org/#/jobs?repo=try&revision=69d21becca31dd37acde5eeeea18c3acd657eb34

Comment 12

[Tom Tung [:tt, :ttung]]

Attachment: Some idea for 9102549

The attachment 9102549 [details] is mostly correct but there is something that needs to be fixed before landing.

There are some nit and some issues on the attachment 9102549 [details]. And this is only a suggestion or suggest for direction (if there is another way to fix the remaining issues, then that would be great.)

So, the major one is that the way it tunnels a value into a javascript closure is incorrect. The existing one doesn't work. And there seems no easy way to do that. (I checked the arguments.callee, but it isn't allowed under the strict mode). Thus, I think the short term way is to hard code the value into checkFun.

Minor stuff (e.g. dump, typo) that need to be fixed before landing.

I will update the current patch under this direction once bug 1588193 is fixed. (I'll recheck if there is an easy way to pass a local variable to a js closure)

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e48111fd8ea2
Re-implement waitForMessage based on waitForContentEvent. r=mccr8

Comment 14

[Razvan Maries]

Backed out as per request: https://hg.mozilla.org/integration/autoland/rev/36b3dc6256ff193525c7419fd470ff39d05be2f4

Comment 15

[Tom Tung [:tt, :ttung]]

Will merge "Some idea for 9102549" into D49752

Comment 16

[Andrew McCreight [:mccr8]]

(In reply to Tom Tung [:tt, :ttung] from comment #12)

So, the major one is that the way it tunnels a value into a javascript closure is incorrect. The existing one doesn't work. And there seems no easy way to do that. (I checked the arguments.callee, but it isn't allowed under the strict mode). Thus, I think the short term way is to hard code the value into checkFun.

Based on something I saw on the toSource() mdn page, one idea might be to pass in a dummy object where you have overriden toSource to just return the string you want. You could use a template literal to splice in the message. That's ugly, but hopefully nicer than hard coding specific types.

Comment 17

[Tom Tung [:tt, :ttung]]

(In reply to Andrew McCreight [:mccr8] from comment #16)

(In reply to Tom Tung [:tt, :ttung] from comment #12)

So, the major one is that the way it tunnels a value into a javascript closure is incorrect. The existing one doesn't work. And there seems no easy way to do that. (I checked the arguments.callee, but it isn't allowed under the strict mode). Thus, I think the short term way is to hard code the value into checkFun.

Based on something I saw on the toSource() mdn page, one idea might be to pass in a dummy object where you have overriden toSource to just return the string you want. You could use a template literal to splice in the message. That's ugly, but hopefully nicer than hard coding specific types.

Thanks! Will make a patch taking this idea!

Comment 18

[Tom Tung [:tt, :ttung]]

try for 9102549 : https://treeherder.mozilla.org/#/jobs?repo=try&revision=098fe9fa94c25aa817fe9ce30fc2fe91f14e27b5

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/617567f745c2
Re-implement waitForMessage based on waitForContentEvent; r=mccr8,janv

Comment 20

[Tom Tung [:tt, :ttung]]

Note that D49753 is expected to be landed once bug 1588193 is fixed.

Comment 21

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/617567f745c2

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d7c5b5d38982
Fix behaviour of ThirdPartyUtil::IsThirdPartyWindow with Fission. r=ttung,Ehsan,kmag

Comment 23

[Cosmin Sabou [:CosminS]]

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

Comment 24

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/617567f745c2

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ba0583a15749
Reenable fission tests. r=ttung

Comment 26

[Alexandru Michis [:malexandru]]

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