1561712 - Write a test for downloads with COOP enabled

Status: RESOLVED FIXED

(Core :: Networking, task, P2)

Description

[Neha Kochar [:neha]]

We need to test if downloads with COOP enabled and if not, fix that. And also write a test for it.

Comment 1

[Valentin Gosu [:valentin] (he/him)]

Attachment: test.zip

I performed a manual verification that downloads work OK with coop headers
Steps:

Edit /etc/hosts and add 127.0.0.1 other.host
Extract attached archive.
Run python server.py
./mach run --setpref browser.tabs.remote.useCrossOriginOpenerPolicy=true
Go to http://other.host:8000/ and try to download all the files.
Go to http://other.host:8000/?coop=same-origin and try to download all the files.
Go to http://localhost:8000/?coop=same-origin and try to download all the files.
Make sure all downloads succeeded.

Neha, do we want something more than this?
We can eventually turn it into an automated test if needed.

Comment 2

[Neha Kochar [:neha]]

Thanks for the manual test, Valentin. Changing the bug title to writing a test for this.

ni'ing Nhi to find someone to work on this.

Comment 3

[Nhi Nguyen (:nhi)]

assigned

Comment 4

[Junior [inactive]]

(In reply to PTO until September 9th from comment #1)

Created attachment 9074888 [details]
test.zip

nit in server.py

self.send_header("Cross-Origin-Opener-Policy", "same-origin")
s/"same-origin"/coop

Comment 5

[Junior [inactive]]

Attachment: Bug 1561712 - test for downloads with COOP enabled

Comment 6

[Junior [inactive]]

Hello Nika,
Two observations in the test:
(a) If the coop mismatch between current page and download link, clicking the download link will make the page empty (no navigation, so no history.length++) and popup the download window. Is that a bug or on purpose?

(b) If the coop of top-level window is same-origin, this code can't work.

      ContentTask.spawn(_browser, coop, coop => {
        content.document.getElementById(coop).click();
      });

Is that a bug or on purpose?
If it's on purpose, any suggestion to get the dom object and click?

Comment 7

[Junior [inactive]]

(

(b) If the coop of top-level window is same-origin, this code can't work.

      ContentTask.spawn(_browser, coop, coop => {
        content.document.getElementById(coop).click();
      });

Is that a bug or on purpose?
If it's on purpose, any suggestion to get the dom object and click?

Mm... I set a super long timeout and it works. Looks like I need to wait for some event.
Based on this, I can have a test given issue (a) exists

Comment 8

[Nika Layzell [:nika] (ni? for response)]

(In reply to Junior [:junior] from comment #6)

Hello Nika,
Two observations in the test:
(a) If the coop mismatch between current page and download link, clicking the download link will make the page empty (no navigation, so no history.length++) and popup the download window. Is that a bug or on purpose?

That's a known bug. Ideally this could be fixed by some combination of the DocumentChannel work and your work.

(b) If the coop of top-level window is same-origin, this code can't work.

      ContentTask.spawn(_browser, coop, coop => {
        content.document.getElementById(coop).click();
      });

Is that a bug or on purpose?
If it's on purpose, any suggestion to get the dom object and click?

You need to wait for the process switch to finish using something like BrowserTestUtils.browserLoaded before trying to spawn into it, as a process switch will mean the target of your spawn is wrong.

Comment 9

[Junior [inactive]]

That's a known bug. Ideally this could be fixed by some combination of the DocumentChannel work and your work.

Thanks. I load the initial page every time I download to work around this bug.

You need to wait for the process switch to finish using something like BrowserTestUtils.browserLoaded before trying to spawn into it, as a process switch will mean the target of your spawn is wrong.

Need to wait SSTabRestored if it's coop mismatch for the the loading page and about:blank.
(I didn't aware I need to take about:blank into account for BrowserTestUtils.withNewTab

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2a9efa4e6b40
test for downloads with COOP enabled r=nika

Comment 11

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out changeset 2a9efa4e6b40 (Bug 1561712) for browser_httpCrossOriginOpenerPolicy.js failures

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&searchStr=windows%2C10%2Cx64%2Cquantumrender%2Cdebug%2Cmochitests&fromchange=b401901b9e7c5692645698dc327f3d3323a7925f&tochange=4abf73b4dc0d8f2ab139c4d72c8fcad5776a1d1d&selectedJob=256832769

Backout link: https://hg.mozilla.org/integration/autoland/rev/235f30e7ccbfb8b3c1e1fcbd408282e5c904a407

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=256832769&repo=autoland&lineNumber=52629

01:21:08 INFO - test_download: Test tab ready: https://example.com/browser/toolkit/components/remotebrowserutils/tests/browser/coop_header.sjs?downloadPage&
01:21:08 INFO - Downloading same-origin%20unsafe-allow-outgoing
01:21:08 INFO - Download window shown...
01:21:08 INFO - TEST-PASS | toolkit/components/remotebrowserutils/tests/browser/browser_httpCrossOriginOpenerPolicy.js | Download page appeared -
01:21:08 INFO - Buffered messages logged at 01:15:18
01:21:08 INFO - test_download: Test tab ready: https://example.com/browser/toolkit/components/remotebrowserutils/tests/browser/coop_header.sjs?downloadPage&same-site
01:21:08 INFO - Buffered messages logged at 01:16:38
01:21:08 INFO - Longer timeout required, waiting longer... Remaining timeouts: 3
01:21:08 INFO - Buffered messages logged at 01:18:08
01:21:08 INFO - Longer timeout required, waiting longer... Remaining timeouts: 2
01:21:08 INFO - Buffered messages logged at 01:19:38
01:21:08 INFO - Longer timeout required, waiting longer... Remaining timeouts: 1
01:21:08 INFO - Buffered messages logged at 01:20:03
01:21:08 INFO - Console message: [JavaScript Error: "getScreenshot(https://example.com/browser/toolkit/components/remotebrowserutils/tests/browser/coop_header.sjs?downloadPage&) failed: TypeError: NetworkError when attempting to fetch resource." {file: "resource://activity-stream/lib/Screenshots.jsm" line: 59}]
01:21:08 INFO - getScreenshotForURL@resource://activity-stream/lib/Screenshots.jsm:59:10
01:21:08 INFO - asyncmaybeCacheScreenshot@resource://activity-stream/lib/Screenshots.jsm:112:37
01:21:08 INFO - _fetchScreenshot@resource://activity-stream/lib/TopSitesFeed.jsm:527:23
01:21:08 INFO - _fetchIcon@resource://activity-stream/lib/TopSitesFeed.jsm:515:16
01:21:08 INFO - getLinksWithDefaults@resource://activity-stream/lib/TopSitesFeed.jsm:408:16
01:21:08 INFO - asyncrefresh@resource://activity-stream/lib/TopSitesFeed.jsm:431:30
01:21:08 INFO - onAction@resource://activity-stream/lib/TopSitesFeed.jsm:771:14
01:21:08 INFO - _middleware/</<@resource://activity-stream/lib/Store.jsm:63:17
01:21:08 INFO - Store/this[method]@resource://activity-stream/lib/Store.jsm:39:54
01:21:08 INFO - init/this.intervalId<@resource://activity-stream/lib/SystemTickFeed.jsm:27:24
01:21:08 INFO - notify@resource://gre/modules/Timer.jsm:62:17
01:21:08 INFO -
01:21:08 INFO - Buffered messages finished
01:21:08 INFO - TEST-UNEXPECTED-FAIL | toolkit/components/remotebrowserutils/tests/browser/browser_httpCrossOriginOpenerPolicy.js | Test timed out -
01:21:08 INFO - GECKO(5240) | MEMORY STAT | vsize 2104204MB | vsizeMaxContiguous 70387764MB | residentFast 246MB | heapAllocated 74MB
01:21:08 INFO - TEST-OK | toolkit/components/remotebrowserutils/tests/browser/browser_httpCrossOriginOpenerPolicy.js | took 360076ms
01:21:08 INFO - Not taking screenshot here: see the one that was previously logged
01:21:08 INFO - TEST-UNEXPECTED-FAIL | toolkit/components/remotebrowserutils/tests/browser/browser_httpCrossOriginOpenerPolicy.js | Found a tab after previous test timed out: https://example.com/browser/toolkit/components/remotebrowserutils/tests/browser/coop_header.sjs?downloadPage&same-site -
01:21:08 INFO - GECKO(5240) | [Parent 11532, Main Thread] WARNING: Constructing RangeBoundary with invalid value: 'mRef || aOffset == 0', file z:/build/build/src/obj-firefox/dist/include\mozilla/RangeBoundary.h, line 79
01:21:08 INFO - GECKO(5240) | [Parent 11532, Main Thread] WARNING: Constructing RangeBoundary with invalid value: 'mRef || aOffset == 0', file z:/build/build/src/obj-firefox/dist/include\mozilla/RangeBoundary.h, line 79
01:21:08 INFO - GECKO(5240) | [Child 9668, Main Thread] WARNING: No active window: file z:/build/build/src/js/xpconnect/src/XPCJSContext.cpp, line 662
01:21:08 INFO - GECKO(5240) | ++DOCSHELL 000002A40823D800 == 1 [pid = 10372] [id = {65c32696-43b7-4f29-9713-b3dd98ad4c2f}]
01:21:08 INFO - GECKO(5240) | ++DOMWINDOW == 1 (000002A40EE0E020) [pid = 10372] [serial = 45] [outer = 0000000000000000]
01:21:08 INFO - GECKO(5240) | [Child 10372, Main Thread] WARNING: No active window: file z:/build/build/src/js/xpconnect/src/XPCJSContext.cpp, line 662
01:21:08 INFO - GECKO(5240) | ++DOMWINDOW == 2 (000002A4082AB800) [pid = 10372] [serial = 46] [outer = 000002A40EE0E020]
01:21:08 INFO - checking window state
01:21:08 INFO - GECKO(5240) | JavaScript error: resource://testing-common/PromiseTestUtils.jsm, line 112: uncaught exception: Object
01:21:08 INFO - TEST-START | toolkit/components/remotebrowserutils/tests/browser/browser_httpResponseProcessSelection.js

Comment 12

[Junior [inactive]]

"SSTabRestored" can't be catch in treeherder. Need to add listener before load new tab.

Comment 13

[Junior [inactive]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a8123c2545dc9401d7af22a8d7d3ac1391c57280&selectedJob=257049948

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f849276c790b
test for downloads with COOP enabled r=nika

Comment 15

[Cosmin Sabou [:CosminS]]

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