Summary: | Layout Test http/tests/security/canvas-remote-read-remote-image-redirect.html is flaky | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexey Proskuryakov <ap> | ||||||||||||
Component: | WebCore Misc. | Assignee: | Alexey Proskuryakov <ap> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | buildbot, commit-queue, eric.carlson, glenn, jer.noble, joepeck, mkwst, rniwa | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
Alexey Proskuryakov
2013-09-16 15:33:02 PDT
I can reproduce reliably if I convert this test to a .cgi, and add a sleep() at the last line. Makes good sense - all the checks in PageConsole::addMessage() pass, so we generate a line number from current parser line. I can't make sense of the checks though. Why do we generate a line number for the script when _not_ running a script? This seems opposite to what it should be. This is pretty much the same as bug 105280. So here is the reason why the check is what it is. This code is meant to produce line numbers for parser generated console messages (like e.g. frame sandbox attribute parsing errors). In the case of this particular test, I think that parser->isExecutingScript() may be giving a wrong result. But then there are tons of addConsoleMessage calls everywhere in WebKit that are neither. Consider for example WebSocketChannel::fail(), which is triggered by network level errors. If this happens during parsing, then current parser location will be randomly assigned as error line! > I think that parser->isExecutingScript() may be giving a wrong result.
This function only tells you if parser is executing a script, not if a script is being executed for any other reason.
(In reply to comment #4) > > I think that parser->isExecutingScript() may be giving a wrong result. > > This function only tells you if parser is executing a script, not if a script is being executed for any other reason. You're entirely correct; this code is broken as written. My apologies. :/ I plan to resolve it in Blink by distinguishing between console messages that relate to parsing, and those that don't. After a not-terribly-deep audit, it looks like SecurityMessageSource is the only MessageSource that refers both to parsing errors and non-parsing errors. An initial attempt to separate those out is up at https://codereview.chromium.org/20890003/. I've abandoned that patch after discussion with pfeldman@, and I'd now like to just split SecurityMessageSource into two sources: ParsingSecurityMessageSource and NonparsingSecurityMessageSource (not with those names, of course :) ). Created attachment 211956 [details]
patch for EWS
Not trying to fix everything, just flakiness. Let's see what happens (not tested locally).
Comment on attachment 211956 [details] patch for EWS Attachment 211956 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1928203 New failing tests: fast/frames/sandboxed-iframe-attribute-parsing-14.html http/tests/security/contentSecurityPolicy/sandbox-empty.html http/tests/security/isolatedWorld/sandboxed-iframe.html fast/frames/sandboxed-iframe-attribute-parsing-07.html fast/frames/sandboxed-iframe-attribute-parsing-10.html http/tests/security/contentSecurityPolicy/sandbox-in-http-header.html http/tests/security/contentSecurityPolicy/sandbox-invalid-header.html fast/frames/sandboxed-iframe-attribute-parsing-06.html http/tests/security/mixedContent/insecure-script-in-iframe.html fast/frames/sandboxed-iframe-attribute-parsing-12.html fast/frames/sandboxed-iframe-scripting-04.html fast/frames/sandboxed-iframe-parsing-space-characters.html fast/frames/sandboxed-iframe-attribute-parsing-13.html fast/frames/sandboxed-iframe-attribute-parsing-09.html fast/frames/sandboxed-iframe-attribute-parsing-11.html fast/frames/sandboxed-iframe-attribute-parsing-08.html http/tests/security/contentSecurityPolicy/sandbox-empty-subframe.html http/tests/security/mixedContent/redirect-http-to-https-script-in-iframe.html media/video-controls-no-scripting.html http/tests/security/contentSecurityPolicy/sandbox-in-http-header-control.html Created attachment 212006 [details]
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-03 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 211956 [details] patch for EWS Attachment 211956 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/1835193 New failing tests: fast/frames/sandboxed-iframe-attribute-parsing-14.html http/tests/security/contentSecurityPolicy/sandbox-empty.html http/tests/security/isolatedWorld/sandboxed-iframe.html fast/frames/sandboxed-iframe-attribute-parsing-07.html fast/frames/sandboxed-iframe-attribute-parsing-10.html http/tests/security/contentSecurityPolicy/sandbox-in-http-header.html http/tests/security/contentSecurityPolicy/sandbox-invalid-header.html fast/frames/sandboxed-iframe-attribute-parsing-06.html http/tests/security/mixedContent/insecure-script-in-iframe.html fast/frames/sandboxed-iframe-attribute-parsing-12.html fast/frames/sandboxed-iframe-scripting-04.html fast/frames/sandboxed-iframe-parsing-space-characters.html fast/frames/sandboxed-iframe-attribute-parsing-13.html fast/frames/sandboxed-iframe-attribute-parsing-09.html fast/frames/sandboxed-iframe-attribute-parsing-11.html fast/frames/sandboxed-iframe-attribute-parsing-08.html http/tests/security/contentSecurityPolicy/sandbox-empty-subframe.html http/tests/security/mixedContent/redirect-http-to-https-script-in-iframe.html media/video-controls-no-scripting.html http/tests/security/contentSecurityPolicy/sandbox-in-http-header-control.html Created attachment 212008 [details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-15 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 211956 [details] patch for EWS Attachment 211956 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1888294 New failing tests: fast/frames/sandboxed-iframe-attribute-parsing-14.html http/tests/security/contentSecurityPolicy/sandbox-empty.html http/tests/security/isolatedWorld/sandboxed-iframe.html fast/frames/sandboxed-iframe-attribute-parsing-07.html fast/frames/sandboxed-iframe-attribute-parsing-10.html http/tests/security/contentSecurityPolicy/sandbox-in-http-header.html http/tests/security/contentSecurityPolicy/sandbox-invalid-header.html fast/frames/sandboxed-iframe-attribute-parsing-06.html http/tests/security/mixedContent/insecure-script-in-iframe.html fast/frames/sandboxed-iframe-attribute-parsing-12.html fast/frames/sandboxed-iframe-scripting-04.html fast/frames/sandboxed-iframe-parsing-space-characters.html fast/frames/sandboxed-iframe-attribute-parsing-13.html fast/frames/sandboxed-iframe-attribute-parsing-09.html fast/frames/sandboxed-iframe-attribute-parsing-11.html fast/frames/sandboxed-iframe-attribute-parsing-08.html http/tests/security/contentSecurityPolicy/sandbox-empty-subframe.html http/tests/security/mixedContent/redirect-http-to-https-script-in-iframe.html media/video-controls-no-scripting.html http/tests/security/contentSecurityPolicy/sandbox-in-http-header-control.html Created attachment 212011 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Created attachment 212098 [details]
proposed fix
This is a very targeted fix. I expect it to fix flakiness on this test, but not necessarily everywhere. It also happened to make us get line numbers where we didn't use to before, which is nice.
Comment on attachment 212098 [details]
proposed fix
EWS is green, let's see how it goes.
Comment on attachment 212098 [details] proposed fix Clearing flags on attachment: 212098 Committed r156130: <http://trac.webkit.org/changeset/156130> All reviewed patches have been landed. Closing bug. |