updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks doesn't work with remote frames |
|||||
Issue description
Repro steps via layout tests at ToT (d2c8b422):
$ ninja -C out/gn blink_tests
$ third_party/WebKit/Tools/Scripts/run-webkit-tests \
--additional-drt-flag=--site-per-process --no-retry-failures -v -t gn \
http/tests/security/mixedContent/insecure-plugin-in-iframe.html
Note that without --site-per-process cmdline switch the test passes.
Expected test output:
CONSOLE WARNING: line 10: Mixed Content: The page at 'https://127.0.0.1:8443/security/mixedContent/resources/frame-with-insecure-plugin.html' was loaded over HTTPS, but requested an insecure plugin resource 'http://example.test:8000/security/mixedContent/resources/dummy.swf'. This content should also be served over HTTPS.
CONSOLE MESSAGE: Blink Test Plugin: initializing
This test loads a secure iframe that loads an insecure plugin. We should get a mixed content callback because the insecure plugin can script the secure origin.
Actual test output:
CONSOLE WARNING: Mixed Content: The page at 'https://127.0.0.1:8443/security/mixedContent/resources/frame-with-insecure-plugin.html' was loaded over HTTPS, but requested an insecure plugin resource 'http://example.test:8000/security/mixedContent/resources/dummy.swf'. This content should also be served over HTTPS.
CONSOLE MESSAGE: Blink Test Plugin: initializing
CONSOLE ERROR: line 10: Uncaught TypeError: Failed to execute 'updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks' on 'Internals': The node provided is neither a document nor an IFrame.
This test loads a secure iframe that loads an insecure plugin. We should get a mixed content callback because the insecure plugin can script the secure origin.
Diff:
No "line 10: " in the first line of test output.
Extra line of test output: CONSOLE ERROR: line 10: Uncaught TypeError: Failed to execute 'updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks' on 'Internals': The node provided is neither a document nor an IFrame.
,
Oct 31 2016
The first difference (lack of "line 10: " with OOPIFs) is kind of WAI.
I believe the second difference indicates that updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks doesn't work with remote frames (aka OOPIFs - out of process iframes):
third_party/WebKit/Source/core/testing/Internals.cpp:
void Internals::updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks(
Node* node,
ExceptionState& exceptionState) {
Document* document = nullptr;
if (!node) {
document = contextDocument();
} else if (node->isDocumentNode()) {
document = toDocument(node);
} else if (isHTMLIFrameElement(*node)) {
// WE HAVE A PROBLEM HERE? AFAIK contentDocument will be null for an OOPIF.
document = toHTMLIFrameElement(*node).contentDocument();
}
if (!document) {
exceptionState.throwTypeError(
"The node provided is neither a document nor an IFrame.");
return;
}
document->updateStyleAndLayoutIgnorePendingStylesheets(
Document::RunPostLayoutTasksSynchronously);
}
,
Oct 31 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0c709a831773d24b82e64888eae63e3dda9c9d66 commit 0c709a831773d24b82e64888eae63e3dda9c9d66 Author: lukasza <lukasza@chromium.org> Date: Mon Oct 31 16:59:12 2016 Updating --site-per-process expectations for insecure-plugin-in-iframe.html The test stopped crashing in --site-per-process mode after r428312. A new bug has been opened for the remaining text diff in test output. BUG= 582494 , 660872 NOTRY=true Review-Url: https://codereview.chromium.org/2466593002 Cr-Commit-Position: refs/heads/master@{#428745} [modify] https://crrev.com/0c709a831773d24b82e64888eae63e3dda9c9d66/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process
,
Oct 31 2016
I don't understand why we'd need to call updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks() explicitly in the test. It's done on begin-frame regardlessly? Isn't that enough?
,
Oct 31 2016
tkent@ - would you remember why you've added the call to updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks in third_party/WebKit/LayoutTests/http/tests/security/mixedContent/insecure-plugin-in-iframe.html in https://codereview.chromium.org/61763016? As rune@ points out it does seem unneeded (i.e. the mixed content message appears even without this call). Would it be okay to just remove the call from this particular test?
,
Oct 31 2016
Comment 5: It was necessary to load a plugin in the IFRAME immediately. I'm not sure if it is unnecessary now. At worst, removing it might make the test flaky.
,
Nov 1 2016
lukasza@: If you wrap the internals call in a try/catch, you will run the updateLayoutIgnore... in the LocalFrame case, and get the same expected output for the OOPIF case (it won't have an effect regardlessly).
,
Nov 1 2016
For http/tests/security/mixedContent/insecure-plugin-in-iframe.html, I think I'll just move the call to updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks from the parent into the child frame (so that the call always happens on a local document). This is a cowardly^H^H^Hlow-risk CL that will hopefully pave way toward enabling this test with --site-per-process (although I agree that in principle we might want to investigate getting rid of that call altogether). Once the above is done, I can try to dust off the CL (https://codereview.chromium.org/2191143002) that allows layout tests to strip/ignore console line numbers (tracked broadly by issue 619662 ).
,
Nov 1 2016
Fix proposed at https://codereview.chromium.org/2468953002 (it turns out that console line numbers can be tackled without going back to https://codereview.chromium.org/2191143002 for help).
,
Nov 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cd8d931dc59e273273701feae744adff8714c27f commit cd8d931dc59e273273701feae744adff8714c27f Author: lukasza <lukasza@chromium.org> Date: Tue Nov 01 23:15:21 2016 Make mixedContent/insecure-plugin-in-iframe.html test OOPIF-compatible. The CL makes the following changes: - Moves the call to internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks from the parent frame into the child frame. This ensures that this method works on a local document - this is important because the method doesn't support OOPIFs. - Delays the call to internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks via window.setTimeout. This ensures that the mixed content message does not include the line number of the call (and so appears the same with an without OOPIFs). The line number used to indicate which line called internals.updateLayoutIgnore...heetsAndRunPostLayoutTasks and this piece of information is artificial (and not essential to test for) - no real page will call this test-only window.internals method. - To make sure the test finishes *after* the delayed call to internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks a pair of testRunner.waitUntilDone / testRunner.notifyDone calls has been added. Note that the subframe used to post a "done" message back to the opener, but this message was never listened to (and so the postMessage call can be deleted). BUG= 660872 Review-Url: https://codereview.chromium.org/2468953002 Cr-Commit-Position: refs/heads/master@{#429147} [modify] https://crrev.com/cd8d931dc59e273273701feae744adff8714c27f/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process [modify] https://crrev.com/cd8d931dc59e273273701feae744adff8714c27f/third_party/WebKit/LayoutTests/http/tests/security/mixedContent/insecure-plugin-in-iframe-expected.txt [modify] https://crrev.com/cd8d931dc59e273273701feae744adff8714c27f/third_party/WebKit/LayoutTests/http/tests/security/mixedContent/insecure-plugin-in-iframe.html [modify] https://crrev.com/cd8d931dc59e273273701feae744adff8714c27f/third_party/WebKit/LayoutTests/http/tests/security/mixedContent/resources/frame-with-insecure-plugin.html
,
Apr 27 2017
Hmmm... it seems that http/tests/security/mixedContent/insecure-plugin-in-iframe.html runs as expected now. I am guessing that the CL from #c10 above fixed the problem and I just forgot to mark the bug as fixed. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by lukasza@chromium.org
, Oct 31 2016