Closed Bug 1586189 Opened 2 months ago Closed 10 days ago

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

Categories

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

task

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox71 --- affected
firefox72 --- fixed

People

(Reporter: tt, Assigned: tt)

References

(Blocks 1 open bug)

Details

Attachments

(4 files, 1 obsolete file)

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

Type: defect → task
Priority: -- → P2
Assignee: nobody → sgiesecke

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.

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.

Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/aac323190bcc
Provide useful assertion messages. r=ttung

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

Status: NEW → RESOLVED
Closed: Last month
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
Status: RESOLVED → REOPENED
Keywords: leave-open
Resolution: FIXED → ---
Summary: Reopen skipped tests in indexedDB for fission → Reenable indexedDB tests that are skipped or expected to fail
Depends on: 1588193
No longer depends on: 1585777
Assignee: sgiesecke → ttung

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

Duplicate of this bug: 1586190
Status: REOPENED → ASSIGNED
Summary: Reenable indexedDB tests that are skipped or expected to fail → Reenable quota indexedDB tests that are skipped or expected to fail
Attached file Some idea for 9102549 (obsolete) —

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)

See Also: → 1590696
See Also: 1590696
Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e48111fd8ea2
Re-implement waitForMessage based on waitForContentEvent. r=mccr8
Attachment #9103602 - Attachment mime type: application/octet-stream → text/plain

Will merge "Some idea for 9102549" into D49752

Flags: needinfo?(ttung)

(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.

(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!

Attachment #9102549 - Attachment description: Bug 1586189 - Re-implement waitForMessage based on waitForContentEvent. r=nika → Bug 1586189 - Re-implement waitForMessage based on waitForContentEvent;
Pushed by ttung@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/617567f745c2
Re-implement waitForMessage based on waitForContentEvent; r=mccr8,janv

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

Attachment #9103602 - Attachment is obsolete: true
Pushed by ttung@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d7c5b5d38982
Fix behaviour of ThirdPartyUtil::IsThirdPartyWindow with Fission. r=ttung,Ehsan,kmag
Summary: Reenable quota indexedDB tests that are skipped or expected to fail → Reenable quota and indexedDB tests that are skipped or expected to fail
Status: ASSIGNED → RESOLVED
Closed: Last month10 days ago
Resolution: --- → FIXED
Target Milestone: mozilla71 → mozilla72
You need to log in before you can comment on or make changes to this bug.