Bug 121458 – Layout Test http/tests/security/canvas-remote-read-remote-image-redirect.html is flaky

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Alexey Proskuryakov]

The following layout test is flaky on all platforms: http/tests/security/canvas-remote-read-remote-image-redirect.html. The diff is:

-CONSOLE MESSAGE: Unable to get image data from canvas because the canvas has been tainted by cross-origin data.
-CONSOLE MESSAGE: Unable to get image data from canvas because the canvas has been tainted by cross-origin data.
-CONSOLE MESSAGE: Unable to get image data from canvas because the canvas has been tainted by cross-origin data.
-CONSOLE MESSAGE: Unable to get image data from canvas because the canvas has been tainted by cross-origin data.
+CONSOLE MESSAGE: line 108: Unable to get image data from canvas because the canvas has been tainted by cross-origin data.
+CONSOLE MESSAGE: line 108: Unable to get image data from canvas because the canvas has been tainted by cross-origin data.
+CONSOLE MESSAGE: line 108: Unable to get image data from canvas because the canvas has been tainted by cross-origin data.
+CONSOLE MESSAGE: line 108: Unable to get image data from canvas because the canvas has been tainted by cross-origin data.

Line 108 is the last empty line of the test, after </script>. I think that that this is caused by automatic line number generation introduced in r136657. I could never reproduce the failure locally. In success case, here is what performs the logging:

#0	0x00000001042b6e1e in WebCore::PageConsole::addMessage(WebCore::MessageSource, WebCore::MessageLevel, WTF::String const&, unsigned long, WebCore::Document*) at /Users/ap/Safari/OpenSource/Source/WebCore/page/PageConsole.cpp:141
#1	0x00000001035802be in WebCore::Document::addConsoleMessage(WebCore::MessageSource, WebCore::MessageLevel, WTF::String const&, unsigned long) at /Users/ap/Safari/OpenSource/Source/WebCore/dom/Document.cpp:4819
#2	0x000000010328e1eb in WebCore::CanvasRenderingContext2D::getImageData(WebCore::ImageBuffer::CoordinateSystem, float, float, float, float, int&) const at /Users/ap/Safari/OpenSource/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:1844
#3	0x000000010328e0bb in WebCore::CanvasRenderingContext2D::getImageData(float, float, float, float, int&) const at /Users/ap/Safari/OpenSource/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:1832
#4	0x0000000103c74ccf in WebCore::jsCanvasRenderingContext2DPrototypeFunctionGetImageData(JSC::ExecState*) at /Users/ap/Safari/OpenSource/WebKitBuild/Debug/DerivedSources/WebCore/JSCanvasRenderingContext2D.cpp:2601
#5	0x00002706bb201045 in 0x2706bb201045 ()
#6	0x000000010190c5f7 in JSC::JITCode::execute(JSC::JSStack*, JSC::ExecState*, JSC::VM*) at /Users/ap/Safari/OpenSource/Source/JavaScriptCore/jit/JITCode.cpp:46
#7	0x00000001018efe2f in JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) at /Users/ap/Safari/OpenSource/Source/JavaScriptCore/interpreter/Interpreter.cpp:957
#8	0x000000010167343e in JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) at /Users/ap/Safari/OpenSource/Source/JavaScriptCore/runtime/CallData.cpp:39
#9	0x0000000103c5d4ab in WebCore::JSMainThreadExecState::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) at /Users/ap/Safari/OpenSource/Source/WebCore/bindings/js/JSMainThreadExecState.h:52
#10	0x0000000103d907bf in WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext*, WebCore::Event*) at /Users/ap/Safari/OpenSource/Source/WebCore/bindings/js/JSEventListener.cpp:132
#11	0x00000001037337f2 in WebCore::EventTarget::fireEventListeners(WebCore::Event*, WebCore::EventTargetData*, WTF::Vector<WebCore::RegisteredEventListener, 1ul, WTF::CrashOnOverflow>&) at /Users/ap/Safari/OpenSource/Source/WebCore/dom/EventTarget.cpp:272
#12	0x000000010373321e in WebCore::EventTarget::fireEventListeners(WebCore::Event*) at /Users/ap/Safari/OpenSource/Source/WebCore/dom/EventTarget.cpp:228
#13	0x000000010426fad2 in WebCore::Node::handleLocalEvents(WebCore::Event*) at /Users/ap/Safari/OpenSource/Source/WebCore/dom/Node.cpp:2078
#14	0x00000001036ffd11 in WebCore::EventContext::handleLocalEvents(WebCore::Event*) const at /Users/ap/Safari/OpenSource/Source/WebCore/dom/EventContext.cpp:58
#15	0x0000000103701729 in WebCore::EventDispatcher::dispatchEventAtTarget() at /Users/ap/Safari/OpenSource/Source/WebCore/dom/EventDispatcher.cpp:161
#16	0x0000000103700ee9 in WebCore::EventDispatcher::dispatch() at /Users/ap/Safari/OpenSource/Source/WebCore/dom/EventDispatcher.cpp:118
#17	0x0000000103702c2f in WebCore::EventDispatchMediator::dispatchEvent(WebCore::EventDispatcher*) const at /Users/ap/Safari/OpenSource/Source/WebCore/dom/EventDispatchMediator.cpp:54
#18	0x00000001037004a2 in WebCore::EventDispatcher::dispatchEvent(WebCore::Node*, WTF::PassRefPtr<WebCore::EventDispatchMediator>) at /Users/ap/Safari/OpenSource/Source/WebCore/dom/EventDispatcher.cpp:52
#19	0x000000010426fc4b in WebCore::Node::dispatchEvent(WTF::PassRefPtr<WebCore::Event>) at /Users/ap/Safari/OpenSource/Source/WebCore/dom/Node.cpp:2099
#20	0x00000001039abdb4 in WebCore::HTMLImageLoader::dispatchLoadEvent() at /Users/ap/Safari/OpenSource/Source/WebCore/html/HTMLImageLoader.cpp:58
#21	0x0000000103a9aefc in WebCore::ImageLoader::dispatchPendingLoadEvent() at /Users/ap/Safari/OpenSource/Source/WebCore/loader/ImageLoader.cpp:435
#22	0x0000000103a9ae26 in WebCore::ImageLoader::dispatchPendingEvent(WebCore::EventSender<WebCore::ImageLoader>*) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/ImageLoader.cpp:393
#23	0x0000000103a9b6ac in WebCore::EventSender<WebCore::ImageLoader>::dispatchPendingEvents() at /Users/ap/Safari/OpenSource/Source/WebCore/dom/EventSender.h:106
#24	0x0000000103a9b011 in WebCore::ImageLoader::dispatchPendingLoadEvents() at /Users/ap/Safari/OpenSource/Source/WebCore/loader/ImageLoader.cpp:462
#25	0x0000000103574647 in WebCore::Document::implicitClose() at /Users/ap/Safari/OpenSource/Source/WebCore/dom/Document.cpp:2430
#26	0x00000001037f69cb in WebCore::FrameLoader::checkCallImplicitClose() at /Users/ap/Safari/OpenSource/Source/WebCore/loader/FrameLoader.cpp:850
#27	0x00000001037f6646 in WebCore::FrameLoader::checkCompleted() at /Users/ap/Safari/OpenSource/Source/WebCore/loader/FrameLoader.cpp:793
#28	0x00000001037f6815 in WebCore::FrameLoader::loadDone() at /Users/ap/Safari/OpenSource/Source/WebCore/loader/FrameLoader.cpp:739
#29	0x00000001032764d2 in WebCore::CachedResourceLoader::loadDone(WebCore::CachedResource*) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/cache/CachedResourceLoader.cpp:772
#30	0x000000010480ba80 in WebCore::SubresourceLoader::notifyDone() at /Users/ap/Safari/OpenSource/Source/WebCore/loader/SubresourceLoader.cpp:351
#31	0x000000010480b99c in WebCore::SubresourceLoader::didFinishLoading(double) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/SubresourceLoader.cpp:290
#32	0x0000000104637db5 in WebCore::ResourceLoader::didFinishLoading(WebCore::ResourceHandle*, double) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/ResourceLoader.cpp:489

[Alexey Proskuryakov]

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.

[Alexey Proskuryakov]

This is pretty much the same as bug 105280.

[Alexey Proskuryakov]

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!

[Alexey Proskuryakov]

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

[Mike West]

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

[Alexey Proskuryakov]

Created attachment 211956 [details]
patch for EWS

Not trying to fix everything, just flakiness. Let's see what happens (not tested locally).

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Alexey Proskuryakov]

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.

[Alexey Proskuryakov]

Comment on attachment 212098 [details]
proposed fix

EWS is green, let's see how it goes.

[WebKit Commit Bot]

Comment on attachment 212098 [details]
proposed fix

Clearing flags on attachment: 212098

Committed r156130: <http://trac.webkit.org/changeset/156130>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.