Don't clear IntentRequest callback pointers on stop()
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 24 Feb 2012 04:58:34 +0000 (04:58 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 24 Feb 2012 04:58:34 +0000 (04:58 +0000)
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):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@108724 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/webintents/resources/pass.html [new file with mode: 0644]
LayoutTests/webintents/resources/web-intents-reload-orig.html [new file with mode: 0644]
LayoutTests/webintents/web-intents-reload-expected.txt [new file with mode: 0644]
LayoutTests/webintents/web-intents-reload.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Modules/intents/IntentRequest.cpp
Source/WebCore/Modules/intents/IntentRequest.h
Tools/DumpRenderTree/chromium/WebViewHost.cpp
Tools/DumpRenderTree/chromium/WebViewHost.h

diff --git a/LayoutTests/webintents/resources/pass.html b/LayoutTests/webintents/resources/pass.html
new file mode 100644 (file)
index 0000000..4de21e0
--- /dev/null
@@ -0,0 +1,7 @@
+<html>
+  <head>
+  </head>
+  <body>
+    <p>PASS</p>
+  </body>
+</html>
diff --git a/LayoutTests/webintents/resources/web-intents-reload-orig.html b/LayoutTests/webintents/resources/web-intents-reload-orig.html
new file mode 100644 (file)
index 0000000..d7c80f0
--- /dev/null
@@ -0,0 +1,30 @@
+<html>
+  <head>
+    <script src="../../fast/js/resources/js-test-pre.js"></script>
+    <script>
+      function onSuccess() {
+      }
+
+      function onFailure() {
+      }
+
+      function startIntent() {
+        debug("* launching intent action/type");
+        var intent = new Intent("action", "type");
+        try {
+          navigator.startActivity(intent, onSuccess, onFailure);
+        } catch (e) {
+          testFailed("startActivity should not throw exception");
+        }
+
+        debug("* navigating after startActivity");
+
+        // This should not crash.
+        window.location = "resources/pass.html";
+      }
+    </script>
+  </head>
+  <body>
+    <p>Original content</p>
+  </body>
+</html>
diff --git a/LayoutTests/webintents/web-intents-reload-expected.txt b/LayoutTests/webintents/web-intents-reload-expected.txt
new file mode 100644 (file)
index 0000000..782ec5a
--- /dev/null
@@ -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 (file)
index 0000000..e23e136
--- /dev/null
@@ -0,0 +1,49 @@
+<html>
+  <head>
+    <script src="../fast/js/resources/js-test-pre.js"></script>
+    <script>
+      var latch = true;
+
+      function buttonClicked() {
+        frames[0].startIntent();
+      }
+
+      function frameloaded() {
+        if (latch) {
+          latch = false;
+          startTest();
+          return;
+        }
+
+        debug("* loaded replacement page");
+
+        if (window.layoutTestController) {
+          window.layoutTestController.notifyDone();
+        }
+      }
+
+      function startTest() {
+        if (window.layoutTestController) {
+          window.layoutTestController.waitUntilDone();
+          window.layoutTestController.dumpChildFramesAsText();
+        }
+
+        debug("* loaded");
+
+        // We must simulate a button press with eventSender because intents
+        // require a user gesture.
+        var button = document.getElementById("button");
+        if (eventSender) {
+          eventSender.mouseMoveTo(button.getBoundingClientRect().left + 2, button.getBoundingClientRect().top + 12);
+          eventSender.mouseDown();
+          eventSender.mouseUp();
+          debug("* sent mouseup");
+        }
+      }
+    </script>
+  </head>
+<body>
+<input type="button" id="button" value="Start Web Intent" onmouseup="buttonClicked()">
+<iframe id="frame" onload="frameloaded()" src="resources/web-intents-reload-orig.html"></iframe>
+</body>
+</html>
index 7606307..6fafc48 100644 (file)
@@ -1,3 +1,26 @@
+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>
 
         Upstream BlackBerry Cookie Management Classes
index 468b51f..486caef 100644 (file)
@@ -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<IntentRequest> 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<IntentRequest> protector(this);
 
index 7a809a9..3edc0ce 100644 (file)
@@ -60,6 +60,7 @@ private:
     RefPtr<Intent> m_intent;
     RefPtr<IntentResultCallback> m_successCallback;
     RefPtr<IntentResultCallback> m_errorCallback;
+    bool m_stopped;
 };
 
 } // namespace WebCore
index 5dc39d7..81603b3 100644 (file)
@@ -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)
index 78b53d5..f80f438 100644 (file)
@@ -37,6 +37,7 @@
 #include "WebAccessibilityNotification.h"
 #include "WebCursorInfo.h"
 #include "WebFrameClient.h"
+#include "WebIntentRequest.h"
 #include "WebSpellCheckClient.h"
 #include "WebViewClient.h"
 #include <wtf/HashMap.h>
@@ -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