-[WKWebView evaluateJavaScript] provides a misleading error when the return cannot...
authorap@apple.com <ap@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Jun 2015 16:57:01 +0000 (16:57 +0000)
committerap@apple.com <ap@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Jun 2015 16:57:01 +0000 (16:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=145900

Reviewed by Sam Weinig.

Source/WebCore:

* English.lproj/Localizable.strings:

Source/WebKit2:

We used to conflate to cases: (1) when there was a JavaScript exception, and
(2) when the result could not be serialized for IPC. Turns out that it was quite
misleading!

* UIProcess/API/C/WKPage.cpp:
(WKPageRunJavaScriptInMainFrame):
* UIProcess/API/Cocoa/WKError.h:
* UIProcess/API/Cocoa/WKError.mm:
(localizedDescriptionForErrorCode):
* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView evaluateJavaScript:completionHandler:]):
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::runJavaScriptInMainFrame):
(WebKit::WebPageProxy::scriptValueCallback):
* UIProcess/WebPageProxy.h:
* UIProcess/WebPageProxy.messages.in:
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::runJavaScriptInMainFrame):
* UIProcess/API/gtk/WebKitWebView.cpp:
(webkit_web_view_run_javascript):
(resourcesStreamReadCallback):

Tools:

* TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewEvaluateJavaScript.mm:
(TEST): Added tests.

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

14 files changed:
Source/WebCore/ChangeLog
Source/WebCore/English.lproj/Localizable.strings
Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/API/C/WKPage.cpp
Source/WebKit2/UIProcess/API/Cocoa/WKError.h
Source/WebKit2/UIProcess/API/Cocoa/WKError.mm
Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp
Source/WebKit2/UIProcess/WebPageProxy.cpp
Source/WebKit2/UIProcess/WebPageProxy.h
Source/WebKit2/UIProcess/WebPageProxy.messages.in
Source/WebKit2/WebProcess/WebPage/WebPage.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewEvaluateJavaScript.mm

index 3956f4f..4caf9b0 100644 (file)
@@ -1,3 +1,12 @@
+2015-06-12  Alexey Proskuryakov  <ap@apple.com>
+
+        -[WKWebView evaluateJavaScript] provides a misleading error when the return cannot be serialized
+        https://bugs.webkit.org/show_bug.cgi?id=145900
+
+        Reviewed by Sam Weinig.
+
+        * English.lproj/Localizable.strings:
+
 2015-06-15  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [SOUP] Custom URI schemes don't work for requests containing a fragment identifier
index 0281142..204c41d 100644 (file)
@@ -91,6 +91,9 @@
 /* WKErrorJavaScriptExceptionOccurred description */
 "A JavaScript exception occurred" = "A JavaScript exception occurred";
 
+/* WKErrorJavaScriptResultTypeIsUnsupported */
+"JavaScript execution returned a result of an unsupported type" = "JavaScript execution returned a result of an unsupported type";
+
 /* action menu item */
 "Add to Photos (action menu item)" = "Add to Photos";
 
index be3c729..f548221 100644 (file)
@@ -1,3 +1,32 @@
+2015-06-12  Alexey Proskuryakov  <ap@apple.com>
+
+        -[WKWebView evaluateJavaScript] provides a misleading error when the return cannot be serialized
+        https://bugs.webkit.org/show_bug.cgi?id=145900
+
+        Reviewed by Sam Weinig.
+
+        We used to conflate to cases: (1) when there was a JavaScript exception, and
+        (2) when the result could not be serialized for IPC. Turns out that it was quite
+        misleading!
+
+        * UIProcess/API/C/WKPage.cpp:
+        (WKPageRunJavaScriptInMainFrame):
+        * UIProcess/API/Cocoa/WKError.h:
+        * UIProcess/API/Cocoa/WKError.mm:
+        (localizedDescriptionForErrorCode):
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView evaluateJavaScript:completionHandler:]):
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::runJavaScriptInMainFrame):
+        (WebKit::WebPageProxy::scriptValueCallback):
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/WebPageProxy.messages.in:
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::runJavaScriptInMainFrame):
+        * UIProcess/API/gtk/WebKitWebView.cpp:
+        (webkit_web_view_run_javascript):
+        (resourcesStreamReadCallback):
+
 2015-06-15  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [SOUP] Custom URI schemes don't work for requests containing a fragment identifier
index 1447888..ccec099 100644 (file)
@@ -1944,7 +1944,9 @@ void WKPageSetSession(WKPageRef pageRef, WKSessionRef session)
 
 void WKPageRunJavaScriptInMainFrame(WKPageRef pageRef, WKStringRef scriptRef, void* context, WKPageRunJavaScriptFunction callback)
 {
-    toImpl(pageRef)->runJavaScriptInMainFrame(toImpl(scriptRef)->string(), toGenericCallbackFunction(context, callback));
+    toImpl(pageRef)->runJavaScriptInMainFrame(toImpl(scriptRef)->string(), [context, callback](API::SerializedScriptValue* returnValue, bool, CallbackBase::Error error) {
+        callback(toAPI(returnValue), (error != CallbackBase::Error::None) ? toAPI(API::Error::create().ptr()) : 0, context);
+    });
 }
 
 #ifdef __BLOCKS__
index 9bba913..233c27e 100644 (file)
@@ -36,16 +36,18 @@ WK_EXTERN NSString * const WKErrorDomain WK_AVAILABLE(10_10, 8_0);
 
 /*! @enum WKErrorCode
  @abstract Constants used by NSError to indicate errors in the WebKit domain.
- @constant WKErrorUnknown                       Indicates that an unknown error occurred.
- @constant WKErrorWebContentProcessTerminated   Indicates that the Web Content process was terminated.
- @constant WKErrorWebViewInvalidated            Indicates that the WKWebView was invalidated.
- @constant WKErrorJavaScriptExceptionOccurred   Indicates that a JavaScript exception occurred.
+ @constant WKErrorUnknown                           Indicates that an unknown error occurred.
+ @constant WKErrorWebContentProcessTerminated       Indicates that the Web Content process was terminated.
+ @constant WKErrorWebViewInvalidated                Indicates that the WKWebView was invalidated.
+ @constant WKErrorJavaScriptExceptionOccurred       Indicates that a JavaScript exception occurred.
+ @constant WKErrorJavaScriptResultTypeIsUnsupported Indicates that the result of JavaScript execution could not be returned.
  */
 typedef NS_ENUM(NSInteger, WKErrorCode) {
     WKErrorUnknown = 1,
     WKErrorWebContentProcessTerminated,
     WKErrorWebViewInvalidated,
     WKErrorJavaScriptExceptionOccurred,
+    WKErrorJavaScriptResultTypeIsUnsupported WK_ENUM_AVAILABLE(WK_MAC_TBA, WK_IOS_TBA),
 } WK_ENUM_AVAILABLE(10_10, 8_0);
 
 WK_ASSUME_NONNULL_END
index f2f6011..34f6245 100644 (file)
@@ -49,6 +49,9 @@ static NSString *localizedDescriptionForErrorCode(WKErrorCode errorCode)
 
     case WKErrorJavaScriptExceptionOccurred:
         return WEB_UI_STRING("A JavaScript exception occurred", "WKErrorJavaScriptExceptionOccurred description");
+
+    case WKErrorJavaScriptResultTypeIsUnsupported:
+        return WEB_UI_STRING("JavaScript execution returned a result of an unsupported type", "WKErrorJavaScriptResultTypeIsUnsupported description");
     }
 }
 
index f9845a8..5ea4268 100644 (file)
@@ -590,7 +590,7 @@ static WKErrorCode callbackErrorCode(WebKit::CallbackBase::Error error)
 {
     auto handler = adoptNS([completionHandler copy]);
 
-    _page->runJavaScriptInMainFrame(javaScriptString, [handler](API::SerializedScriptValue* serializedScriptValue, WebKit::ScriptValueCallback::Error errorCode) {
+    _page->runJavaScriptInMainFrame(javaScriptString, [handler](API::SerializedScriptValue* serializedScriptValue, bool hadException, WebKit::ScriptValueCallback::Error errorCode) {
         if (!handler)
             return;
 
@@ -613,11 +613,17 @@ static WKErrorCode callbackErrorCode(WebKit::CallbackBase::Error error)
         }
 
         auto rawHandler = (void (^)(id, NSError *))handler.get();
-        if (!serializedScriptValue) {
+        if (hadException) {
+            ASSERT(!serializedScriptValue);
             rawHandler(nil, createNSError(WKErrorJavaScriptExceptionOccurred).get());
             return;
         }
 
+        if (!serializedScriptValue) {
+            rawHandler(nil, createNSError(WKErrorJavaScriptResultTypeIsUnsupported).get());
+            return;
+        }
+
         auto context = adoptNS([[JSContext alloc] init]);
         JSValueRef valueRef = serializedScriptValue->deserialize([context JSGlobalContextRef], 0);
         JSValue *value = [JSValue valueWithJSValueRef:valueRef inContext:context.get()];
index cfe81e1..73bfbc4 100644 (file)
@@ -3008,7 +3008,7 @@ void webkit_web_view_run_javascript(WebKitWebView* webView, const gchar* script,
     g_return_if_fail(script);
 
     GTask* task = g_task_new(webView, cancellable, callback, userData);
-    getPage(webView)->runJavaScriptInMainFrame(String::fromUTF8(script), [task](API::SerializedScriptValue* serializedScriptValue, WebKit::CallbackBase::Error) {
+    getPage(webView)->runJavaScriptInMainFrame(String::fromUTF8(script), [task](API::SerializedScriptValue* serializedScriptValue, bool, WebKit::CallbackBase::Error) {
         webkitWebViewRunJavaScriptCallback(serializedScriptValue, adoptGRef(task).get());
     });
 }
@@ -3099,7 +3099,7 @@ static void resourcesStreamReadCallback(GObject* object, GAsyncResult* result, g
     WebKitWebView* webView = WEBKIT_WEB_VIEW(g_task_get_source_object(task.get()));
     gpointer outputStreamData = g_memory_output_stream_get_data(G_MEMORY_OUTPUT_STREAM(object));
     getPage(webView)->runJavaScriptInMainFrame(String::fromUTF8(reinterpret_cast<const gchar*>(outputStreamData)),
-        [task](API::SerializedScriptValue* serializedScriptValue, WebKit::CallbackBase::Error) {
+        [task](API::SerializedScriptValue* serializedScriptValue, bool, WebKit::CallbackBase::Error) {
             webkitWebViewRunJavaScriptCallback(serializedScriptValue, task.get());
         });
 }
index 232642b..c1a0ca9 100644 (file)
@@ -2574,10 +2574,10 @@ void WebPageProxy::countStringMatches(const String& string, FindOptions options,
     m_process->send(Messages::WebPage::CountStringMatches(string, options, maxMatchCount), m_pageID);
 }
 
-void WebPageProxy::runJavaScriptInMainFrame(const String& script, std::function<void (API::SerializedScriptValue*, CallbackBase::Error)> callbackFunction)
+void WebPageProxy::runJavaScriptInMainFrame(const String& script, std::function<void (API::SerializedScriptValue*, bool hadException, CallbackBase::Error)> callbackFunction)
 {
     if (!isValid()) {
-        callbackFunction(nullptr, CallbackBase::Error::Unknown);
+        callbackFunction(nullptr, false, CallbackBase::Error::Unknown);
         return;
     }
 
@@ -4644,7 +4644,7 @@ void WebPageProxy::stringCallback(const String& resultString, uint64_t callbackI
     callback->performCallbackWithReturnValue(resultString.impl());
 }
 
-void WebPageProxy::scriptValueCallback(const IPC::DataReference& dataReference, uint64_t callbackID)
+void WebPageProxy::scriptValueCallback(const IPC::DataReference& dataReference, bool hadException, uint64_t callbackID)
 {
     auto callback = m_callbacks.take<ScriptValueCallback>(callbackID);
     if (!callback) {
@@ -4656,7 +4656,7 @@ void WebPageProxy::scriptValueCallback(const IPC::DataReference& dataReference,
     data.reserveInitialCapacity(dataReference.size());
     data.append(dataReference.data(), dataReference.size());
 
-    callback->performCallbackWithReturnValue(data.size() ? API::SerializedScriptValue::adopt(data).get() : 0);
+    callback->performCallbackWithReturnValue(data.size() ? API::SerializedScriptValue::adopt(data).get() : 0, hadException);
 }
 
 void WebPageProxy::computedPagesCallback(const Vector<IntRect>& pageRects, double totalScaleFactorForPrinting, uint64_t callbackID)
index 076182a..d6dc5b2 100644 (file)
@@ -213,7 +213,7 @@ class QuickLookDocumentData;
 typedef GenericCallback<uint64_t> UnsignedCallback;
 typedef GenericCallback<EditingRange> EditingRangeCallback;
 typedef GenericCallback<const String&> StringCallback;
-typedef GenericCallback<API::SerializedScriptValue*> ScriptValueCallback;
+typedef GenericCallback<API::SerializedScriptValue*, bool> ScriptValueCallback;
 
 #if PLATFORM(GTK)
 typedef GenericCallback<API::Error*> PrintFinishedCallback;
@@ -763,7 +763,7 @@ public:
     void getSelectionAsWebArchiveData(std::function<void (API::Data*, CallbackBase::Error)>);
     void getSourceForFrame(WebFrameProxy*, std::function<void (const String&, CallbackBase::Error)>);
     void getWebArchiveOfFrame(WebFrameProxy*, std::function<void (API::Data*, CallbackBase::Error)>);
-    void runJavaScriptInMainFrame(const String&, std::function<void (API::SerializedScriptValue*, CallbackBase::Error)> callbackFunction);
+    void runJavaScriptInMainFrame(const String&, std::function<void (API::SerializedScriptValue*, bool hadException, CallbackBase::Error)> callbackFunction);
     void forceRepaint(PassRefPtr<VoidCallback>);
 
     float headerHeight(WebFrameProxy*);
@@ -1322,7 +1322,7 @@ private:
     void dataCallback(const IPC::DataReference&, uint64_t);
     void imageCallback(const ShareableBitmap::Handle&, uint64_t);
     void stringCallback(const String&, uint64_t);
-    void scriptValueCallback(const IPC::DataReference&, uint64_t);
+    void scriptValueCallback(const IPC::DataReference&, bool hadException, uint64_t);
     void computedPagesCallback(const Vector<WebCore::IntRect>&, double totalScaleFactorForPrinting, uint64_t);
     void validateCommandCallback(const String&, bool, int, uint64_t);
     void unsignedCallback(uint64_t, uint64_t);
index 9db9755..3fc2df5 100644 (file)
@@ -165,7 +165,7 @@ messages -> WebPageProxy {
     DataCallback(IPC::DataReference resultData, uint64_t callbackID)
     ImageCallback(WebKit::ShareableBitmap::Handle bitmapHandle, uint64_t callbackID)
     StringCallback(String resultString, uint64_t callbackID)
-    ScriptValueCallback(IPC::DataReference resultData, uint64_t callbackID)
+    ScriptValueCallback(IPC::DataReference resultData, bool hadException, uint64_t callbackID)
     ComputedPagesCallback(Vector<WebCore::IntRect> pageRects, double totalScaleFactorForPrinting, uint64_t callbackID)
     ValidateCommandCallback(String command, bool isEnabled, int32_t state, uint64_t callbackID)
     EditingRangeCallback(struct WebKit::EditingRange range, uint64_t callbackID)
index 358e1c0..49e3913 100644 (file)
@@ -2529,7 +2529,9 @@ void WebPage::runJavaScriptInMainFrame(const String& script, uint64_t callbackID
 
     RefPtr<SerializedScriptValue> serializedResultValue;
     JSLockHolder lock(JSDOMWindow::commonVM());
+    bool hadException = true;
     if (JSValue resultValue = m_mainFrame->coreFrame()->script().executeScript(script, true).jsValue()) {
+        hadException = false;
         serializedResultValue = SerializedScriptValue::create(m_mainFrame->jsContext(),
             toRef(m_mainFrame->coreFrame()->script().globalObject(mainThreadNormalWorld())->globalExec(), resultValue), nullptr);
     }
@@ -2537,7 +2539,7 @@ void WebPage::runJavaScriptInMainFrame(const String& script, uint64_t callbackID
     IPC::DataReference dataReference;
     if (serializedResultValue)
         dataReference = serializedResultValue->data();
-    send(Messages::WebPageProxy::ScriptValueCallback(dataReference, callbackID));
+    send(Messages::WebPageProxy::ScriptValueCallback(dataReference, hadException, callbackID));
 }
 
 void WebPage::getContentsAsString(uint64_t callbackID)
index 7f054ad..fd08237 100644 (file)
@@ -1,3 +1,13 @@
+2015-06-12  Alexey Proskuryakov  <ap@apple.com>
+
+        -[WKWebView evaluateJavaScript] provides a misleading error when the return cannot be serialized
+        https://bugs.webkit.org/show_bug.cgi?id=145900
+
+        Reviewed by Sam Weinig.
+
+        * TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewEvaluateJavaScript.mm:
+        (TEST): Added tests.
+
 2015-06-15  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [SOUP] Custom URI schemes don't work for requests containing a fragment identifier
index b8a646b..20b2977 100644 (file)
@@ -60,4 +60,34 @@ TEST(WKWebView, EvaluateJavaScriptBlockCrash)
     TestWebKitAPI::Util::run(&isDone);
 }
 
+TEST(WKWebView, EvaluateJavaScriptErrorCases)
+{
+    RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
+
+    NSURLRequest *request = [NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"simple" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]];
+    [webView loadRequest:request];
+
+    [webView evaluateJavaScript:@"document.body" completionHandler:^(id result, NSError *error) {
+        EXPECT_NULL(result);
+        EXPECT_WK_STREQ(@"WKErrorDomain", [error domain]);
+        EXPECT_EQ(5, [error code]);
+
+        isDone = true;
+    }];
+
+    isDone = false;
+    TestWebKitAPI::Util::run(&isDone);
+
+    [webView evaluateJavaScript:@"document.body.insertBefore(document, document)" completionHandler:^(id result, NSError *error) {
+        EXPECT_NULL(result);
+        EXPECT_WK_STREQ(@"WKErrorDomain", [error domain]);
+        EXPECT_EQ(4, [error code]);
+
+        isDone = true;
+    }];
+
+    isDone = false;
+    TestWebKitAPI::Util::run(&isDone);
+}
+
 #endif