Bug 62955

Created attachment 97727 [details] Patch

Comment on attachment 97727 [details] Patch Clearing flags on attachment: 97727 Committed r89223: <http://trac.webkit.org/changeset/89223>

All reviewed patches have been landed. Closing bug.

This patch appears to have broken builds on almost all ports/platforms.

Rolled out in r89224 due to the mass breakage that was caused.

Oops, sorry. Thanks for rolling it out. No big rush to fix these, I guess I’ll do it later.

Here's error: http://build.webkit.org/builders/SnowLeopard%20Intel%20Debug%20%28Build%29/builds/831/steps/compile-webkit/logs/stdio InspectorBackendDispatcher.cpp was failing to compile because InspectorBackendDispatcher::CSS_setPropertyText was passing RefPtr to setPropertyText, which now takes a raw pointer. InspectorBackendDispatcher.cpp is generated by CodeGeneratorInspector.pm and the function called typeTraits seems to be responsible for generating type-specific argument but I couldn't figure out how to fix it. If anything the code that generated InspectorBackendDispatcher::CSS_setPropertyText is around line 460 in CodeGeneratorInspector.pm.

I didn’t realize there was a code generator for the inspector. It’s not urgent to fix this. I can land all the non-inspector parts of this some time.

(In reply to comment #8) > I didn’t realize there was a code generator for the inspector. It’s not urgent to fix this. I can land all the non-inspector parts of this some time. The inspector also has its very own cookie parser, and javascript object model... just in case.

Created attachment 98091 [details] Patch

Removed the inspector part of the patch.

The commit-queue encountered the following flaky tests while processing attachment 98091 [details]: fast/forms/checkValidity-cancel.html bug 63111 (author: tkent@chromium.org) The commit-queue is continuing to process your patch.

Comment on attachment 98091 [details] Patch Clearing flags on attachment: 98091 Committed r89402: <http://trac.webkit.org/changeset/89402>

All reviewed patches have been landed. Closing bug.

It made 6 tests crash: http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20%28Tests%29/r89403%20%2830612%29/results.html and rolled out by http://trac.webkit.org/changeset/89420

Comment on attachment 98091 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=98091&action=review > Source/WebCore/websockets/WorkerThreadableWebSocketChannel.cpp:268 > -void WorkerThreadableWebSocketChannel::Bridge::mainThreadCreateWebSocketChannel(ScriptExecutionContext* context, Bridge* thisPtr, RefPtr<ThreadableWebSocketChannelClientWrapper> clientWrapper, const String& taskMode, const KURL& url, const String& protocol) > +void WorkerThreadableWebSocketChannel::Bridge::mainThreadCreateWebSocketChannel(ScriptExecutionContext* context, Bridge* thisPtr, PassRefPtr<ThreadableWebSocketChannelClientWrapper> clientWrapper, const String& taskMode, const KURL& url, const String& protocol) Now I see the problem. clientWrapper is passed to Peer::create and createCallbackTask.

(In reply to comment #16) > > +void WorkerThreadableWebSocketChannel::Bridge::mainThreadCreateWebSocketChannel(ScriptExecutionContext* context, Bridge* thisPtr, PassRefPtr<ThreadableWebSocketChannelClientWrapper> clientWrapper, const String& taskMode, const KURL& url, const String& protocol) > > Now I see the problem. clientWrapper is passed to Peer::create and createCallbackTask. So that function needs to use the prp idiom.

Anyone know why the commit-bot and the cr-linux EWS bot did not catch that?

(In reply to comment #18) > Anyone know why the commit-bot and the cr-linux EWS bot did not catch that? commit-queue uses Chromium Linux bot but Chromium disables workers' tests in DRT because they require multiple processes. commit-queue also won't catch any JSC regressions because Chromium uses V8 although I had never seen JSC folks using commit-queue before the switch anyway.

(In reply to comment #19) > I had never seen JSC folks using commit-queue before the switch anyway. What? If by "JSC folks" you mean "anyone not working on Chromium" then there have been 100s of JSC folks using commit-queue. I guess you mean people working directly on the JavaScript engine.

(In reply to comment #20) > (In reply to comment #19) > > I had never seen JSC folks using commit-queue before the switch anyway. > > What? If by "JSC folks" you mean "anyone not working on Chromium" then there have been 100s of JSC folks using commit-queue. > > I guess you mean people working directly on the JavaScript engine. I meant people working on JavaScriptCore.

Created attachment 98385 [details] Patch

New patch fixes the prp mistake in WorkerThreadableWebSocketChannel::Bridge::mainThreadCreateWebSocketChannel that Ryosuke spotted for me and is the reason this broke the build.

Comment on attachment 98385 [details] Patch Clearing flags on attachment: 98385 Committed r89613: <http://trac.webkit.org/changeset/89613>

All reviewed patches have been landed. Closing bug.