Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

COEP tests where iframe fails to load #21300

Closed
annevk opened this issue Jan 21, 2020 · 4 comments · Fixed by #21344
Closed

COEP tests where iframe fails to load #21300

annevk opened this issue Jan 21, 2020 · 4 comments · Fixed by #21344
Labels

Comments

@annevk
Copy link
Member

annevk commented Jan 21, 2020

@ParisMeuleman @ArthurSonzogni html/cross-origin-embedder-policy/require-corp-about-blank.html and equivalent tests rely on the iframe element firing a load event when there's a network error. There's currently no agreement on that being the right thing: whatwg/html#125 (there's various issues around this).

We need to fix these tests to not rely on that until there's agreement on that behavior.

Also, when you use promise_test you do need t.step_func wrappers for any event handling as otherwise exceptions fired as part of those events end up failing the test harness as is happening in Firefox. That's problematic. Documentation on this was wrong, but is now corrected at https://web-platform-tests.org/writing-tests/testharness-api.html#promise-tests.

@ParisMeuleman
Copy link
Contributor

Would replacing
let iframe_B_loaded = new Promise(resolve => iframe_B.onload = resolve);
by
let iframe_B_loaded = new Promise(resolve => { iframe_B.onload = resolve; iframe_B.onerror = resolve; });
make the trick?

Arthur made the following script yesterday and we saw the difference in behavior between FF and CR:

let w = window.open();
let iframe = w.document.createElement("iframe");
iframe.src = "https://error-page"
iframe.onerror = () => console.log("onerror")
iframe.onload = () => console.log("onload")
w.document.body.appendChild(iframe);

@annevk
Copy link
Member Author

annevk commented Jan 22, 2020

No, you need to wait for a timeout. And it would be better not to do anything with load as per the specification it's not expected to be dispatched at this point. (Adding a comment pointing to the whatwg/html issue might be good though, so we can clean this up once that gets resolved.)

@ParisMeuleman
Copy link
Contributor

Indeed onerror is not always called! thanks.

I have a prototype that I think works:

waitForFrameLoadedOrBlocked = (frame) => {
  return new Promise(resolve => {
    let interval;
    done = () => {
      clearInterval(interval);
      resolve();
    }
    interval = setInterval(() => {
      try{ frame.contentWindow.a } catch(e) {
        done(interval, resolve);
      }
    }, 1);
    frame.onload = done;
  });
}
[...]
let iframe_B_loaded = waitForFrameLoadedOrBlocked(iframe_B);
let iframe_C_loaded = waitForFrameLoadedOrBlocked(iframe_C);

onload is kept here, supposing that we accept both a load or an error for the frame, which allows for the completion of test even if COEP was mishandled.

@annevk
Copy link
Member Author

annevk commented Jan 22, 2020

I'd rather only use load if we expect it to not result in a network error. And the precedent in other tests is to use t.step_timeout with 500/1500 as timeout. Using interval might be interesting to let it resolve more quickly, but I think we also want an ultimate timeout for individual tests and not let those result in a harness failure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants