Changeset 108724 in webkit

Timestamp:

Feb 23, 2012 8:58:34 PM (12 years ago)

Author:

commit-queue@webkit.org

Message:

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 <​gbillock@google.com> 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):

Location:

trunk

Files:

• LayoutTests/webintents/resources (added)
• LayoutTests/webintents/resources/pass.html (added)
• LayoutTests/webintents/resources/web-intents-reload-orig.html (added)
• LayoutTests/webintents/web-intents-reload-expected.txt (added)
• LayoutTests/webintents/web-intents-reload.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/Modules/intents/IntentRequest.cpp (modified) (3 diffs)
• Source/WebCore/Modules/intents/IntentRequest.h (modified) (1 diff)
• Tools/DumpRenderTree/chromium/WebViewHost.cpp (modified) (2 diffs)
• Tools/DumpRenderTree/chromium/WebViewHost.h (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-02-23  Greg Billock  <gbillock@google.com>
+
+        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  <kpiascik@rim.com>
 

trunk/Source/WebCore/Modules/intents/IntentRequest.cpp

@@ -55 +55 @@
     , m_successCallback(successCallback)
     , m_errorCallback(errorCallback)
+    , m_stopped(false)
 {
 }
@@ -61 +62 @@
 {
     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<IntentRequest> protector(this);
@@ -87 +89 @@
 void IntentRequest::postFailure(SerializedScriptValue* data)
 {
+    if (m_stopped)
+        return;
+
     // Callback could lead to deletion of this.
     RefPtr<IntentRequest> protector(this);

trunk/Source/WebCore/Modules/intents/IntentRequest.h

@@ -61 +61 @@
     RefPtr<IntentResultCallback> m_successCallback;
     RefPtr<IntentResultCallback> m_errorCallback;
+    bool m_stopped;
 };
 

trunk/Tools/DumpRenderTree/chromium/WebViewHost.cpp

@@ -48 +48 @@
 #include "WebGeolocationClientMock.h"
 #include "WebHistoryItem.h"
+#include "WebIntent.h"
 #include "WebKit.h"
 #include "WebNode.h"
@@ -1312 +1313 @@
 
     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;
 }
 

trunk/Tools/DumpRenderTree/chromium/WebViewHost.h

@@ -38 +38 @@
 #include "WebCursorInfo.h"
 #include "WebFrameClient.h"
+#include "WebIntentRequest.h"
 #include "WebSpellCheckClient.h"
 #include "WebViewClient.h"
@@ -238 +239 @@
     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();
@@ -411 +413 @@
     } m_pointerLockPlannedResult;
 #endif
+
+    // For web intents: holds the current request, if any.
+    WebKit::WebIntentRequest m_currentRequest;
 };