From 5922dada8b4925aab554dbe340b3a5f24318987c Mon Sep 17 00:00:00 2001 From: "commit-queue@webkit.org" Date: Fri, 24 Feb 2012 04:58:34 +0000 Subject: [PATCH] Don't clear IntentRequest callback pointers on stop() This causes re-entry into ScriptExecutionContext when the ActiveDOMCallback objects get deleted, which crashes. Instead, just de-activate the object and wait for context destruction to clean up. Test crashes consistently without fix and passes with fix. Added some test infrastructure to support this test. https://bugs.webkit.org/show_bug.cgi?id=78638 Patch by Greg Billock on 2012-02-23 Reviewed by Adam Barth. * Modules/intents/IntentRequest.cpp: (WebCore::IntentRequest::IntentRequest): (WebCore::IntentRequest::stop): (WebCore::IntentRequest::postResult): (WebCore::IntentRequest::postFailure): * Modules/intents/IntentRequest.h: (IntentRequest): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@108724 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- LayoutTests/webintents/resources/pass.html | 7 ++++ .../resources/web-intents-reload-orig.html | 30 +++++++++++++ .../webintents/web-intents-reload-expected.txt | 10 +++++ LayoutTests/webintents/web-intents-reload.html | 49 ++++++++++++++++++++++ Source/WebCore/ChangeLog | 23 ++++++++++ Source/WebCore/Modules/intents/IntentRequest.cpp | 13 ++++-- Source/WebCore/Modules/intents/IntentRequest.h | 1 + Tools/DumpRenderTree/chromium/WebViewHost.cpp | 9 ++++ Tools/DumpRenderTree/chromium/WebViewHost.h | 5 +++ 9 files changed, 143 insertions(+), 4 deletions(-) create mode 100644 LayoutTests/webintents/resources/pass.html create mode 100644 LayoutTests/webintents/resources/web-intents-reload-orig.html create mode 100644 LayoutTests/webintents/web-intents-reload-expected.txt create mode 100644 LayoutTests/webintents/web-intents-reload.html diff --git a/LayoutTests/webintents/resources/pass.html b/LayoutTests/webintents/resources/pass.html new file mode 100644 index 0000000..4de21e0 --- /dev/null +++ b/LayoutTests/webintents/resources/pass.html @@ -0,0 +1,7 @@ + + + + +

PASS

+ + diff --git a/LayoutTests/webintents/resources/web-intents-reload-orig.html b/LayoutTests/webintents/resources/web-intents-reload-orig.html new file mode 100644 index 0000000..d7c80f0 --- /dev/null +++ b/LayoutTests/webintents/resources/web-intents-reload-orig.html @@ -0,0 +1,30 @@ + + + + + + +

Original content

+ + diff --git a/LayoutTests/webintents/web-intents-reload-expected.txt b/LayoutTests/webintents/web-intents-reload-expected.txt new file mode 100644 index 0000000..782ec5a --- /dev/null +++ b/LayoutTests/webintents/web-intents-reload-expected.txt @@ -0,0 +1,10 @@ +Received Web Intent: action=action type=type +* loaded +* sent mouseup +* loaded replacement page + + +-------- +Frame: 'frame' +-------- +PASS diff --git a/LayoutTests/webintents/web-intents-reload.html b/LayoutTests/webintents/web-intents-reload.html new file mode 100644 index 0000000..e23e136 --- /dev/null +++ b/LayoutTests/webintents/web-intents-reload.html @@ -0,0 +1,49 @@ + + + + + + + + + + diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 7606307..6fafc48 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,26 @@ +2012-02-23 Greg Billock + + Don't clear IntentRequest callback pointers on stop() + + This causes re-entry into ScriptExecutionContext when + the ActiveDOMCallback objects get deleted, which crashes. + Instead, just de-activate the object and wait for + context destruction to clean up. + + Test crashes consistently without fix and passes with fix. + Added some test infrastructure to support this test. + https://bugs.webkit.org/show_bug.cgi?id=78638 + + Reviewed by Adam Barth. + + * Modules/intents/IntentRequest.cpp: + (WebCore::IntentRequest::IntentRequest): + (WebCore::IntentRequest::stop): + (WebCore::IntentRequest::postResult): + (WebCore::IntentRequest::postFailure): + * Modules/intents/IntentRequest.h: + (IntentRequest): + 2012-02-23 Konrad Piascik Upstream BlackBerry Cookie Management Classes diff --git a/Source/WebCore/Modules/intents/IntentRequest.cpp b/Source/WebCore/Modules/intents/IntentRequest.cpp index 468b51f..486caef 100644 --- a/Source/WebCore/Modules/intents/IntentRequest.cpp +++ b/Source/WebCore/Modules/intents/IntentRequest.cpp @@ -54,24 +54,26 @@ IntentRequest::IntentRequest(ScriptExecutionContext* context, , m_intent(intent) , m_successCallback(successCallback) , m_errorCallback(errorCallback) + , m_stopped(false) { } void IntentRequest::contextDestroyed() { ContextDestructionObserver::contextDestroyed(); - m_successCallback.clear(); - m_errorCallback.clear(); + m_stopped = true; } void IntentRequest::stop() { - m_successCallback.clear(); - m_errorCallback.clear(); + m_stopped = true; } void IntentRequest::postResult(SerializedScriptValue* data) { + if (m_stopped) + return; + // Callback could lead to deletion of this. RefPtr protector(this); @@ -86,6 +88,9 @@ void IntentRequest::postResult(SerializedScriptValue* data) void IntentRequest::postFailure(SerializedScriptValue* data) { + if (m_stopped) + return; + // Callback could lead to deletion of this. RefPtr protector(this); diff --git a/Source/WebCore/Modules/intents/IntentRequest.h b/Source/WebCore/Modules/intents/IntentRequest.h index 7a809a9..3edc0ce 100644 --- a/Source/WebCore/Modules/intents/IntentRequest.h +++ b/Source/WebCore/Modules/intents/IntentRequest.h @@ -60,6 +60,7 @@ private: RefPtr m_intent; RefPtr m_successCallback; RefPtr m_errorCallback; + bool m_stopped; }; } // namespace WebCore diff --git a/Tools/DumpRenderTree/chromium/WebViewHost.cpp b/Tools/DumpRenderTree/chromium/WebViewHost.cpp index 5dc39d7..81603b3 100644 --- a/Tools/DumpRenderTree/chromium/WebViewHost.cpp +++ b/Tools/DumpRenderTree/chromium/WebViewHost.cpp @@ -47,6 +47,7 @@ #include "WebFrame.h" #include "WebGeolocationClientMock.h" #include "WebHistoryItem.h" +#include "WebIntent.h" #include "WebKit.h" #include "WebNode.h" #include "WebPluginParams.h" @@ -1313,6 +1314,14 @@ bool WebViewHost::willCheckAndDispatchMessageEvent(WebFrame* source, WebSecurity return false; } +void WebViewHost::dispatchIntent(WebFrame* source, const WebIntentRequest& request) +{ + printf("Received Web Intent: action=%s type=%s\n", + request.intent().action().utf8().data(), + request.intent().type().utf8().data()); + m_currentRequest = request; +} + // Public functions ----------------------------------------------------------- WebViewHost::WebViewHost(TestShell* shell) diff --git a/Tools/DumpRenderTree/chromium/WebViewHost.h b/Tools/DumpRenderTree/chromium/WebViewHost.h index 78b53d5..f80f438 100644 --- a/Tools/DumpRenderTree/chromium/WebViewHost.h +++ b/Tools/DumpRenderTree/chromium/WebViewHost.h @@ -37,6 +37,7 @@ #include "WebAccessibilityNotification.h" #include "WebCursorInfo.h" #include "WebFrameClient.h" +#include "WebIntentRequest.h" #include "WebSpellCheckClient.h" #include "WebViewClient.h" #include @@ -237,6 +238,7 @@ class WebViewHost : public WebKit::WebSpellCheckClient, public WebKit::WebViewCl virtual void didDetectXSS(WebKit::WebFrame*, const WebKit::WebURL&, bool didBlockEntirePage); virtual void openFileSystem(WebKit::WebFrame*, WebKit::WebFileSystem::Type, long long size, bool create, WebKit::WebFileSystemCallbacks*); virtual bool willCheckAndDispatchMessageEvent(WebKit::WebFrame* source, WebKit::WebSecurityOrigin target, WebKit::WebDOMMessageEvent); + virtual void dispatchIntent(WebKit::WebFrame* source, const WebKit::WebIntentRequest&); WebKit::WebDeviceOrientationClientMock* deviceOrientationClientMock(); @@ -410,6 +412,9 @@ private: PointerLockWillFailSync } m_pointerLockPlannedResult; #endif + + // For web intents: holds the current request, if any. + WebKit::WebIntentRequest m_currentRequest; }; #endif // WebViewHost_h -- 2.7.4