Bug 94458 – Assertion failure in MessagePort::contextDestroyed in http/tests/security/MessagePort/event-listener-context.html, usually attributed to later tests

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Zan Dobersek]

http/tests/security/XFrameOptions/x-frame-options-deny-meta-tag-in-body.html recently started to crash on debug builds:
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&showAllRuns=true&tests=http%2Ftests%2Fsecurity%2FXFrameOptions%2Fx-frame-options-deny-meta-tag-in-body.html

It only crashed once on the Apple Lion Debug WK1 builder but is crashing almost every second run on the GTK 64-bit debug builder. On the latter, the first crash occurred at build #35611[1], making me suspect r125592[2] as the culprit.

[1] - http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug/builds/35611
[2] - http://trac.webkit.org/changeset/125592

[Zan Dobersek]

Created attachment 159375 [details]
GTK crash log

[Zan Dobersek]

Created attachment 159376 [details]
Mac crash log

[Alexey Proskuryakov]

This is reported against http/tests/security/aboutBlank/security-context-grandchildren-alias.html even more often, but it's unclear yet which test is to blame. I haven't been able to reproduce locally.

We have previously seen this assertion failure in bug 85811, but that test is still skipped, so another test must be also triggering this.

[Alexey Proskuryakov]

It's http/tests/security/MessagePort/event-listener-context.html, I suggest skipping.

[Alexey Proskuryakov]

*** Bug 95079 has been marked as a duplicate of this bug. ***

[Jessie Berlin]

fast/events/message-port-context-destroyed.html is asserting in the same place on ML WK2 Debug and Lion WK2 Debug:

http://build.webkit.org/results/Apple%20MountainLion%20Debug%20WK2%20(Tests)/r127135%20(434)/fast/events/message-port-context-destroyed-crash-log.txt
http://build.webkit.org/results/Apple%20Lion%20Debug%20WK2%20(Tests)/r127136%20(2968)/fast/events/message-port-context-destroyed-crash-log.txt

I am going to add it to the mac-wk2 Skipped list.

Process:         WebProcess [24435]
Path:            /Volumes/VOLUME/*/WebKit2.framework/WebProcess.app/Contents/MacOS/WebProcess
Identifier:      com.apple.WebProcess
Version:         537+ (537.3+)
Code Type:       X86-64 (Native)
Parent Process:  ??? [1]

PlugIn Path:       /Volumes/VOLUME/*/WebKitTestRunnerInjectedBundle.bundle/Contents/MacOS/WebKitTestRunnerInjectedBundle
PlugIn Identifier: WebKitTestRunnerInjectedBundle
PlugIn Version:    ??? (???)

Date/Time:       2012-08-30 08:11:15.336 -0700
OS Version:      Mac OS X 10.7.4 (11E53)
Report Version:  9

Crashed Thread:  0  Dispatch queue: com.apple.main-thread

Exception Type:  EXC_BAD_ACCESS (SIGSEGV)
Exception Codes: KERN_INVALID_ADDRESS at 0x00000000bbadbeef

VM Regions Near 0xbbadbeef:
--> 
    __TEXT                 0000000100c87000-0000000100c88000 [    4K] r-x/rwx SM=COW  /Volumes/VOLUME/*/WebKit2.framework/WebProcess.app/Contents/MacOS/WebProcess

Application Specific Information:
objc[24435]: garbage collection is OFF

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.WebCore             	0x0000000103c3af72 WebCore::MessagePort::contextDestroyed() + 178 (MessagePort.cpp:158)
1   com.apple.WebCore             	0x00000001040ba714 WebCore::ScriptExecutionContext::~ScriptExecutionContext() + 724 (ScriptExecutionContext.cpp:113)
2   com.apple.WebCore             	0x0000000102e7f573 WebCore::Document::~Document() + 3523 (Document.cpp:687)
3   com.apple.WebCore             	0x00000001032c7115 WebCore::HTMLDocument::~HTMLDocument() + 149 (HTMLDocument.cpp:91)
4   com.apple.WebCore             	0x00000001032c6fe5 WebCore::HTMLDocument::~HTMLDocument() + 21 (HTMLDocument.cpp:91)
5   com.apple.WebCore             	0x00000001032c6fb9 WebCore::HTMLDocument::~HTMLDocument() + 25 (HTMLDocument.cpp:90)
6   com.apple.WebCore             	0x0000000102e99139 WebCore::Document::guardDeref() + 201 (Document.h:247)
7   com.apple.WebCore             	0x0000000102e7fb30 WebCore::Document::removedLastRef() + 560 (Document.cpp:736)
8   com.apple.WebCore             	0x0000000103c7ae02 WebCore::Node::removedLastRef() + 50 (Node.cpp:2825)
9   com.apple.WebCore             	0x0000000102a81c1e WebCore::TreeShared<WebCore::Node, WebCore::ContainerNode>::deref() + 494 (TreeShared.h:83)
10  com.apple.WebCore             	0x00000001037771c6 WebCore::JSNode::releaseImpl() + 38 (JSNode.h:69)
11  com.apple.WebCore             	0x00000001038a4491 WebCore::JSNodeOwner::finalize(JSC::Handle<JSC::Unknown>, void*) + 113 (JSNodeCustom.cpp:145)
12  com.apple.JavaScriptCore      	0x00000001020464d7 JSC::WeakBlock::finalize(JSC::WeakImpl*) + 215 (WeakSetInlines.h:53)
13  com.apple.JavaScriptCore      	0x0000000102045e2e JSC::WeakBlock::sweep() + 158 (WeakBlock.cpp:81)
14  com.apple.JavaScriptCore      	0x00000001020467b0 JSC::WeakSet::sweep() + 64 (WeakSet.cpp:45)
15  com.apple.JavaScriptCore      	0x0000000101f0c5c8 JSC::MarkedBlock::sweep(JSC::MarkedBlock::SweepMode) + 40 (MarkedBlock.cpp:108)
16  com.apple.JavaScriptCore      	0x0000000101f0f0f0 JSC::Sweep::operator()(JSC::MarkedBlock*) + 32 (MarkedSpace.h:53)
17  com.apple.JavaScriptCore      	0x0000000101f0f0b2 void JSC::MarkedAllocator::forEachBlock<JSC::Sweep>(JSC::Sweep&) + 82 (MarkedAllocator.h:111)
18  com.apple.JavaScriptCore      	0x0000000101f0efa9 JSC::Sweep::ReturnType JSC::MarkedSpace::forEachBlock<JSC::Sweep>(JSC::Sweep&) + 105 (MarkedSpace.h:207)
19  com.apple.JavaScriptCore      	0x0000000101f0e629 JSC::Sweep::ReturnType JSC::MarkedSpace::forEachBlock<JSC::Sweep>() + 25 (MarkedSpace.h:226)
20  com.apple.JavaScriptCore      	0x0000000101f0dfb4 JSC::MarkedSpace::sweep() + 52 (MarkedSpace.cpp:116)
21  com.apple.JavaScriptCore      	0x0000000101e05ac9 JSC::Heap::collect(JSC::Heap::SweepToggle) + 729 (Heap.cpp:745)
22  com.apple.JavaScriptCore      	0x0000000101e071a4 JSC::Heap::collectAllGarbage() + 52 (Heap.cpp:680)
23  com.apple.WebCore             	0x00000001031f4773 WebCore::GCController::garbageCollectNow() + 83 (GCController.cpp:85)
24  com.apple.WebKit2             	0x0000000100d37779 WebKit::InjectedBundle::garbageCollectJavaScriptObjects() + 25 (InjectedBundle.cpp:435)
25  com.apple.WebKit2             	0x0000000100ff5c5d WKBundleGarbageCollectJavaScriptObjects + 29 (WKBundle.cpp:82)
26  WebKitTestRunnerInjectedBundle	0x000000010a01e511 WTR::GCController::collect() + 33 (GCController.cpp:56)
27  WebKitTestRunnerInjectedBundle	0x000000010a01e8ba WTR::JSGCController::collect(OpaqueJSContext const*, OpaqueJSValue*, OpaqueJSValue*, unsigned long, OpaqueJSValue const* const*, OpaqueJSValue const**) + 90 (JSGCController.cpp:82)
28  com.apple.JavaScriptCore      	0x0000000101e804a1 JSC::JSCallbackFunction::call(JSC::ExecState*) + 497 (JSCallbackFunction.cpp:73)
29  com.apple.JavaScriptCore      	0x0000000102030e20 _ZN3JSC5LLIntL14handleHostCallEPNS_9ExecStateEPNS_11InstructionENS_7JSValueENS_22CodeSpecializationKindE + 352 (LLIntSlowPaths.cpp:1297)
30  com.apple.JavaScriptCore      	0x0000000102031d9d JSC::LLInt::setUpCall(JSC::ExecState*, JSC::Instruction*, JSC::CodeSpecializationKind, JSC::JSValue, JSC::LLIntCallLinkInfo*) + 93 (LLIntSlowPaths.cpp:1341)
31  com.apple.JavaScriptCore      	0x0000000102031d27 JSC::LLInt::genericCall(JSC::ExecState*, JSC::Instruction*, JSC::CodeSpecializationKind) + 263 (LLIntSlowPaths.cpp:1397)
32  com.apple.JavaScriptCore      	0x000000010202ef7c llint_slow_path_call + 60 (LLIntSlowPaths.cpp:1403)
33  com.apple.JavaScriptCore      	0x0000000102036d77 llint_op_call + 185
34  com.apple.JavaScriptCore      	0x0000000101e20d20 JSC::JITCode::execute(JSC::RegisterFile*, JSC::ExecState*, JSC::JSGlobalData*) + 96 (JITCode.h:133)
35  com.apple.JavaScriptCore      	0x0000000101e1d6f4 JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 1908 (Interpreter.cpp:1045)
36  com.apple.JavaScriptCore      	0x0000000101cb5f1b JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 315 (CallData.cpp:39)
37  com.apple.WebCore             	0x00000001035e3cb3 WebCore::JSMainThreadExecState::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 179 (JSMainThreadExecState.h:56)
38  com.apple.WebCore             	0x0000000103733ec0 WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext*, WebCore::Event*) + 1408 (JSEventListener.cpp:126)
39  com.apple.WebCore             	0x00000001030ec5f9 WebCore::EventTarget::fireEventListeners(WebCore::Event*, WebCore::EventTargetData*, WTF::Vector<WebCore::RegisteredEventListener, 1ul>&) + 393 (EventTarget.cpp:232)
40  com.apple.WebCore             	0x00000001030ec43b WebCore::EventTarget::fireEventListeners(WebCore::Event*) + 331 (EventTarget.cpp:200)
41  com.apple.WebCore             	0x0000000103c7952b WebCore::Node::handleLocalEvents(WebCore::Event*) + 155 (Node.cpp:2570)
42  com.apple.WebCore             	0x00000001030b90f5 WebCore::EventContext::handleLocalEvents(WebCore::Event*) const + 293 (EventContext.cpp:55)
43  com.apple.WebCore             	0x00000001030bc1df WebCore::EventDispatcher::dispatchEventAtTarget(WTF::PassRefPtr<WebCore::Event>) + 111 (EventDispatcher.cpp:309)
44  com.apple.WebCore             	0x00000001030bb019 WebCore::EventDispatcher::dispatchEvent(WTF::PassRefPtr<WebCore::Event>) + 1129 (EventDispatcher.cpp:261)
45  com.apple.WebCore             	0x00000001030c119c WebCore::EventDispatchMediator::dispatchEvent(WebCore::EventDispatcher*) const + 76 (EventDispatchMediator.cpp:51)
46  com.apple.WebCore             	0x00000001030b9bda WebCore::EventDispatcher::dispatchEvent(WebCore::Node*, WTF::PassRefPtr<WebCore::EventDispatchMediator>) + 154 (EventDispatcher.cpp:129)
47  com.apple.WebCore             	0x0000000103c79626 WebCore::Node::dispatchEvent(WTF::PassRefPtr<WebCore::Event>) + 70 (Node.cpp:2585)
48  com.apple.WebCore             	0x0000000103037fa6 WebCore::DOMWindow::dispatchLoadEvent() + 758 (DOMWindow.cpp:1639)
49  com.apple.WebCore             	0x0000000102e89202 WebCore::Document::dispatchWindowLoadEvent() + 146 (Document.cpp:4110)
50  com.apple.WebCore             	0x0000000102e86671 WebCore::Document::implicitClose() + 513 (Document.cpp:2536)
51  com.apple.WebCore             	0x00000001031a1a7b WebCore::FrameLoader::checkCallImplicitClose() + 155 (FrameLoader.cpp:804)
52  com.apple.WebCore             	0x00000001031a1745 WebCore::FrameLoader::checkCompleted() + 341 (FrameLoader.cpp:751)
53  com.apple.WebCore             	0x00000001031a04d3 WebCore::FrameLoader::finishedParsing() + 179 (FrameLoader.cpp:684)
54  com.apple.WebCore             	0x0000000102e9319f WebCore::Document::finishedParsing() + 591 (Document.cpp:4886)
55  com.apple.WebCore             	0x00000001033a5f64 WebCore::HTMLTreeBuilder::finished() + 148 (HTMLTreeBuilder.cpp:2696)
56  com.apple.WebCore             	0x00000001032cc243 WebCore::HTMLDocumentParser::end() + 227 (HTMLDocumentParser.cpp:373)
57  com.apple.WebCore             	0x00000001032cb236 WebCore::HTMLDocumentParser::attemptToRunDeferredScriptsAndEnd() + 278 (HTMLDocumentParser.cpp:382)
58  com.apple.WebCore             	0x00000001032cb01c WebCore::HTMLDocumentParser::prepareToStopParsing() + 268 (HTMLDocumentParser.cpp:150)
59  com.apple.WebCore             	0x00000001032cc293 WebCore::HTMLDocumentParser::attemptToEnd() + 67 (HTMLDocumentParser.cpp:394)
60  com.apple.WebCore             	0x00000001032cc2e8 WebCore::HTMLDocumentParser::finish() + 72 (HTMLDocumentParser.cpp:421)
61  com.apple.WebCore             	0x0000000102ef7c17 WebCore::DocumentWriter::end() + 391 (DocumentWriter.cpp:245)
62  com.apple.WebCore             	0x0000000102ed402f WebCore::DocumentLoader::finishedLoading() + 207 (DocumentLoader.cpp:301)
63  com.apple.WebCore             	0x0000000103bd138e WebCore::MainResourceLoader::didFinishLoading(double) + 318 (MainResourceLoader.cpp:526)
64  com.apple.WebCore             	0x0000000104055c25 WebCore::ResourceLoader::didFinishLoading(WebCore::ResourceHandle*, double) + 53 (ResourceLoader.cpp:442)
65  com.apple.WebCore             	0x0000000104052705 -[WebCoreResourceHandleAsDelegate connectionDidFinishLoading:] + 197 (ResourceHandleMac.mm:861)
66  com.apple.Foundation          	0x00007fff90b1f63e ___NSURLConnectionDidFinishLoading_block_invoke_1 + 122
67  com.apple.Foundation          	0x00007fff90b1f5be _NSURLConnectionDidFinishLoading + 81
68  com.apple.CFNetwork           	0x00007fff9505e4fe URLConnectionClient::_clientDidFinishLoading(URLConnectionClient::ClientConnectionEventQueue*) + 296
69  com.apple.CFNetwork           	0x00007fff9510e91e URLConnectionClient::ClientConnectionEventQueue::processAllEventsAndConsumePayload(XConnectionEventInfo<XClientEvent, XClientEventParams>*, long) + 862
70  com.apple.CFNetwork           	0x00007fff9510eb0a URLConnectionClient::ClientConnectionEventQueue::processAllEventsAndConsumePayload(XConnectionEventInfo<XClientEvent, XClientEventParams>*, long) + 1354
71  com.apple.CFNetwork           	0x00007fff95039389 URLConnectionClient::processEvents() + 185
72  com.apple.CFNetwork           	0x00007fff9503922e MultiplexerSource::perform() + 212
73  com.apple.CoreFoundation      	0x00007fff935784f1 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
74  com.apple.CoreFoundation      	0x00007fff93577d5d __CFRunLoopDoSources0 + 253
75  com.apple.CoreFoundation      	0x00007fff9359eb49 __CFRunLoopRun + 905
76  com.apple.CoreFoundation      	0x00007fff9359e486 CFRunLoopRunSpecific + 230
77  com.apple.HIToolbox           	0x00007fff9519b4d3 RunCurrentEventLoopInMode + 277
78  com.apple.HIToolbox           	0x00007fff951a2781 ReceiveNextEventCommon + 355
79  com.apple.HIToolbox           	0x00007fff951a260e BlockUntilNextEventMatchingListInMode + 62
80  com.apple.AppKit              	0x00007fff8e5e4e31 _DPSNextEvent + 659
81  com.apple.AppKit              	0x00007fff8e5e4735 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 135
82  com.apple.AppKit              	0x00007fff8e5e1071 -[NSApplication run] + 470
83  com.apple.WebCore             	0x0000000104080b6c WebCore::RunLoop::run() + 92 (RunLoopMac.mm:37)
84  com.apple.WebKit2             	0x0000000100fcf472 WebKit::WebProcessMain(WebKit::CommandLine const&) + 3506 (WebProcessMainMac.mm:228)
85  com.apple.WebKit2             	0x0000000100edd49f _ZL10WebKitMainRKN6WebKit11CommandLineE + 239 (WebKitMain.cpp:50)
86  com.apple.WebKit2             	0x0000000100edd393 WebKitMain + 163 (WebKitMain.cpp:74)
87  com.apple.WebProcess          	0x0000000100c87d82 main + 290
88  com.apple.WebProcess          	0x0000000100c87c54 start + 52

[Alexey Proskuryakov]

> fast/events/message-port-context-destroyed.html is asserting in the same place on ML WK2 Debug and Lion WK2 Debug:

That's probably bug 85811.

[Alexey Proskuryakov]

Strike that previous comment, I was confused about which bug I was looking at.

Jessie, are you sure that the test you are going to skip is guilty? AFAICT, we never skipped event-listener-context.html, so it's still affecting subsequent tests.

[Jessie Berlin]

(In reply to comment #8)
> Strike that previous comment, I was confused about which bug I was looking at.
> 
> Jessie, are you sure that the test you are going to skip is guilty? AFAICT, we never skipped event-listener-context.html, so it's still affecting subsequent tests.

There is an entry in the platform/mac Skipped list already for http/tests/security/MessagePort/event-listener-context.html

[Jessie Berlin]

(In reply to comment #9)
> (In reply to comment #8)
> > Strike that previous comment, I was confused about which bug I was looking at.
> > 
> > Jessie, are you sure that the test you are going to skip is guilty? AFAICT, we never skipped event-listener-context.html, so it's still affecting subsequent tests.
> 
> There is an entry in the platform/mac Skipped list already for http/tests/security/MessagePort/event-listener-context.html

But you are right that fast/events/message-port-context-destroyed.html isn't the culprit.

When I run 

run-webkit-tests -2 LayoutTests/fast/events/message-port-clone.html LayoutTests/fast/events/message-port-close.html LayoutTests/fast/events/message-port-constructor-for-deleted-document.html LayoutTests/fast/events/message-port-context-destroyed.html 

I get the crash 100% of the time

When I run

run-webkit-tests -2 LayoutTests/fast/events/message-port-clone.html LayoutTests/fast/events/message-port-close.html LayoutTests/fast/events/message-port-context-destroyed.html 

I don't get the crash.

I am going to skip LayoutTests/fast/events/message-port-constructor-for-deleted-document.html

[Alexey Proskuryakov]

I really want you to try "--repeat-each 10 -v" too. With multiple runs, it gets blazingly clear which tests are causing trouble.

[Mark Lam]

I found (based on tests I ran) that sometimes you'll have to do "--repeat-each 100 -v" or "--repeat-each 1000 -v" even.  Depending on the test (and the root cause), it may take more reps to reproduce.

[Jessie Berlin]

(In reply to comment #11)
> I really want you to try "--repeat-each 10 -v" too. With multiple runs, it gets blazingly clear which tests are causing trouble.

I did. LayoutTests/fast/events/message-port-constructor-for-deleted-document.html "crashed" in MessagePort::contextDestroyed when I did so. I Skipped it in http://trac.webkit.org/changeset/127166

[Li Yin]

I think the root cause is the same with Bug 85811, I have posted a patch for Bug 85811, hope it can fix this issue as well.

[Dominik Röttsches (drott)]

(In reply to comment #14)
> I think the root cause is the same with Bug 85811, I have posted a patch for Bug 85811, hope it can fix this issue as well.

So, can we close it? At least, http/tests/security/XFrameOptions/x-frame-options-deny-meta-tag-in-body.html seems to reliably pass on EFL now.

[Dominik Röttsches (drott)]

Committed r127447: <http://trac.webkit.org/changeset/127447>

[Dominik Röttsches (drott)]

webkit-patch land closed this prematurely since it took my comment for the bug URL :-(

[Dominik Röttsches (drott)]

And my observation was wrong - the test did not reliably pass on the bot yet. I am sorry for the confusion.

[Gábor Ábrahám]

Still problematic on Qt Debug bots.
On x86-32 Linux Qt Debug http/tests/security/XFrameOptions/x-frame-options-cached.html test crashes:
http://build.webkit.sed.hu/results/x86-32%20Linux%20Qt%20Debug/r151375%20(26085)/results.html

On x86-64 Linux Qt Debug http/tests/security/XFrameOptions/x-frame-options-deny-multiple-clients.html test crashes:
http://build.webkit.sed.hu/results/x86-64%20Linux%20Qt%20Debug/r151375%20(29270)/results.html

I generated backtrace for http/tests/security/XFrameOptions/x-frame-options-invalid.html test on Qt x64 enviroment:
https://gist.github.com/abrhm/5748258

Could you check it please?

[Alexey Proskuryakov]

Per bug 155373, this is still happening.

[Ryan Haddad]

*** Bug 155373 has been marked as a duplicate of this bug. ***

[Ryan Haddad]

Additional tests that hit this assertion on ios-simulator debug:
http/tests/security/XFrameOptions/x-frame-options-deny-meta-tag-parent-same-origin-allow.html
http/tests/security/aboutBlank/window-open-self-about-blank.html

Both have been marked as flaky (r198295 and r198344)

[Alexey Proskuryakov]

More gardening in <http://trac.webkit.org/r199183>. Still happens, of course.

[Brady Eidson]

*** Bug 181017 has been marked as a duplicate of this bug. ***

[Brady Eidson]

Found this independently while working on MessagePort refactoring (https://bugs.webkit.org/show_bug.cgi?id=181017)

I have a fix, but first I'm trying to make a single test reproduce the crash.

[Brady Eidson]

(In reply to Brady Eidson from comment #25)
> Found this independently while working on MessagePort refactoring
> (https://bugs.webkit.org/show_bug.cgi?id=181017)
> 
> I have a fix, but first I'm trying to make a single test reproduce the crash.

And that was easy. Patch on its way.

[Brady Eidson]

Created attachment 329900 [details]
Patch

[Brady Eidson]

Besides working with the test in question, I haven't hammered on this patch much on my own. Relying on EWS for that.

Goodnight now, and will revisit in the AM.

[Alexey Proskuryakov]

See also <rdar://problem/11533338>

[EWS Watchlist]

Comment on attachment 329900 [details]
Patch

Attachment 329900 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/5769304

New failing tests:
fast/events/message-port-deleted-document.html
fast/workers/worker-cloneport.html
fast/events/message-channel-gc-4.html
fast/events/message-port-inactive-document.html
fast/events/message-port-clone.html
fast/events/mouse-cursor-no-mousemove.html
fast/events/message-port-deleted-frame.html

[EWS Watchlist]

Created attachment 329901 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[EWS Watchlist]

Comment on attachment 329900 [details]
Patch

Attachment 329900 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/5769307

New failing tests:
fast/events/message-channel-gc.html
fast/workers/worker-close.html
fast/events/message-port-multi.html
fast/events/message-port-deleted-frame.html
imported/w3c/web-platform-tests/service-workers/service-worker/clients-matchall.https.html
imported/w3c/web-platform-tests/service-workers/service-worker/clients-matchall-on-evaluation.https.html

[EWS Watchlist]

Created attachment 329902 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[EWS Watchlist]

Comment on attachment 329900 [details]
Patch

Attachment 329900 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/5769330

New failing tests:
imported/w3c/web-platform-tests/service-workers/service-worker/ServiceWorkerGlobalScope/postmessage.https.html
imported/w3c/web-platform-tests/service-workers/service-worker/referrer-policy-header.https.html
imported/w3c/web-platform-tests/service-workers/service-worker/clients-matchall.https.html
imported/w3c/web-platform-tests/service-workers/service-worker/fetch-response-taint.https.html
fast/events/message-channel-gc.html

[EWS Watchlist]

Created attachment 329903 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 329900 [details]
Patch

Attachment 329900 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/5769370

New failing tests:
fast/workers/worker-multi-port.html
fast/workers/worker-context-multi-port.html
fast/workers/termination-with-port-messages.html
imported/w3c/web-platform-tests/html/browsers/origin/relaxing-the-same-origin-restriction/document_domain_setter_srcdoc.html
fast/workers/worker-cloneport.html
fast/dom/Window/window-postmessage-args.html
imported/w3c/web-platform-tests/workers/postMessage_clone_port.htm
imported/w3c/web-platform-tests/FileAPI/blob/Blob-constructor.html
fast/events/message-port-inactive-document.html
fast/workers/worker-close-more.html
fast/events/message-port-deleted-document.html
fast/events/message-port-multi.html
fast/workers/worker-messageport.html
imported/w3c/web-platform-tests/workers/postMessage_ports_readonly_array.htm
fast/events/message-port-deleted-frame.html
fast/events/message-port-clone.html
fast/events/message-port.html
fast/workers/worker-messageport-gc.html
fast/events/message-channel-gc-4.html

[EWS Watchlist]

Created attachment 329904 [details]
Archive of layout-test-results from ews116 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Brady Eidson]

While this patch fixes the test in question, clearly some additional work to do. Exploring locally.

[Brady Eidson]

Created attachment 329919 [details]
Patch

[Chris Dumez]

Comment on attachment 329919 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=329919&action=review

> Source/WebCore/dom/MessagePort.h:76
> +    bool canSuspendForDocumentSuspension() const final { return false; }

This will likely regress PageCache substantially. can we at the very least return true if !hasPendingActivity() or if not m_started or if m_stopped?

[Brady Eidson]

Created attachment 329947 [details]
Patch

[Brady Eidson]

(In reply to Chris Dumez from comment #40)
> Comment on attachment 329919 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=329919&action=review
> 
> > Source/WebCore/dom/MessagePort.h:76
> > +    bool canSuspendForDocumentSuspension() const final { return false; }
> 
> This will likely regress PageCache substantially. can we at the very least
> return true if !hasPendingActivity() or if not m_started or if m_stopped?

Updated patch does this

[Chris Dumez]

Comment on attachment 329947 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=329947&action=review

r=me

> Source/WebCore/dom/MessagePort.cpp:232
> +    return !hasPendingActivity() || (!m_started  || m_closed);

nit: Looks like there is an extra space in there.

[Brady Eidson]

Created attachment 329948 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 329948 [details]
Patch

Clearing flags on attachment: 329948

Committed r226202: <https://trac.webkit.org/changeset/226202>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/36165042>

[Darin Adler]

Comment on attachment 329948 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=329948&action=review

> Source/WebCore/ChangeLog:10
> +        There was already a glaring FIXME that said "MessagePorts should be ActiveDOMObjects"

I remember this. Alexey and I added it while working on bug 52719.

> Source/WebCore/dom/MessagePort.cpp:41
> +    suspendIfNeeded();

Normally it’s not safe to call this in the constructor; if we reenter we can run into problems because we haven’t yet adopted the reference count. Can we move this function call to MessagePort::create? Might want to move the call to createdMessagePort too for the same reason, although I don’t think it’s needed.

We’d want to move MessagePort::create into the .cpp file, which is probably good anyway.

> Source/WebCore/dom/MessagePort.h:44
> +class MessagePort final : public RefCounted<MessagePort>, public ActiveDOMObject, public EventTargetWithInlineData {

I would have preferred to inherit privately from ActiveDOMObject if possible. Does it need to be public inheritance?

> Source/WebCore/dom/MessagePort.h:85
> +    // ActiveDOMObject
> +    const char* activeDOMObjectName() const final;
> +    bool canSuspendForDocumentSuspension() const final;
> +    void contextDestroyed() final;
> +    void stop() final { close(); }
> +    bool hasPendingActivity() const final;
>  
> +    // EventTargetWithInlineData.
> +    EventTargetInterface eventTargetInterface() const final { return MessagePortEventTargetInterfaceType; }
> +    ScriptExecutionContext* scriptExecutionContext() const final { return ActiveDOMObject::scriptExecutionContext(); }
>      void refEventTarget() final { ref(); }
>      void derefEventTarget() final { deref(); }

I would have liked it slightly better if these overrides were private, like eventTargetInterface already was; we could have some be public, but only the ones that needed to be. Is there some reason to have them public now? Maybe to keep all the overridden functions together in the same paragraph?

[Brady Eidson]

(In reply to Darin Adler from comment #48)
> Comment on attachment 329948 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=329948&action=review
> 
> > Source/WebCore/ChangeLog:10
> > +        There was already a glaring FIXME that said "MessagePorts should be ActiveDOMObjects"
> 
> I remember this. Alexey and I added it while working on bug 52719.
> 
> > Source/WebCore/dom/MessagePort.cpp:41
> > +    suspendIfNeeded();
> 
> Normally it’s not safe to call this in the constructor; if we reenter we can
> run into problems because we haven’t yet adopted the reference count. Can we
> move this function call to MessagePort::create? Might want to move the call
> to createdMessagePort too for the same reason, although I don’t think it’s
> needed.

This can be done later, sure.

> 
> We’d want to move MessagePort::create into the .cpp file, which is probably
> good anyway.
> 
> > Source/WebCore/dom/MessagePort.h:44
> > +class MessagePort final : public RefCounted<MessagePort>, public ActiveDOMObject, public EventTargetWithInlineData {
> 
> I would have preferred to inherit privately from ActiveDOMObject if
> possible. Does it need to be public inheritance?

Nope.

> 
> > Source/WebCore/dom/MessagePort.h:85
> > +    // ActiveDOMObject
> > +    const char* activeDOMObjectName() const final;
> > +    bool canSuspendForDocumentSuspension() const final;
> > +    void contextDestroyed() final;
> > +    void stop() final { close(); }
> > +    bool hasPendingActivity() const final;
> >  
> > +    // EventTargetWithInlineData.
> > +    EventTargetInterface eventTargetInterface() const final { return MessagePortEventTargetInterfaceType; }
> > +    ScriptExecutionContext* scriptExecutionContext() const final { return ActiveDOMObject::scriptExecutionContext(); }
> >      void refEventTarget() final { ref(); }
> >      void derefEventTarget() final { deref(); }
> 
> I would have liked it slightly better if these overrides were private, like
> eventTargetInterface already was; we could have some be public, but only the
> ones that needed to be. Is there some reason to have them public now? Maybe
> to keep all the overridden functions together in the same paragraph?

No good reason.

[Alexey Proskuryakov]

We still have this test skipped.

LayoutTests/TestExpectations:2104:webkit.org/b/94458 http/tests/security/MessagePort/event-listener-context.html [ Skip ]