Title: [199680] trunk/Source/WebKit2
Revision
199680
Author
bb...@apple.com
Date
2016-04-18 10:46:01 -0700 (Mon, 18 Apr 2016)

Log Message

Web Automation: provide detailed error messages when an automation command fails
https://bugs.webkit.org/show_bug.cgi?id=156635
<rdar://problem/25754051>

Reviewed by Darin Adler and Timothy Hatcher.

Fill in lots of missing error message details now that the remote end knows
how to parse error strings with predefined error names and details.

* UIProcess/Automation/WebAutomationSession.cpp:
Rearrange the error message macros. Make it possible to pass a ErrorMessage
variable or just the enum value name and get out an encoded error message
that optionally includes a free-form details string. The predefined error
name and the details string are joined together with a semicolon.

(WebKit::WebAutomationSession::getBrowsingContext):
(WebKit::WebAutomationSession::createBrowsingContext):
(WebKit::WebAutomationSession::closeBrowsingContext):
(WebKit::WebAutomationSession::switchToBrowsingContext):
(WebKit::WebAutomationSession::resizeWindowOfBrowsingContext):
(WebKit::WebAutomationSession::moveWindowOfBrowsingContext):
(WebKit::WebAutomationSession::navigateBrowsingContext):
(WebKit::WebAutomationSession::goBackInBrowsingContext):
(WebKit::WebAutomationSession::goForwardInBrowsingContext):
(WebKit::WebAutomationSession::reloadBrowsingContext):
(WebKit::WebAutomationSession::evaluateJavaScriptFunction):
(WebKit::WebAutomationSession::didEvaluateJavaScriptFunction):
(WebKit::WebAutomationSession::resolveChildFrameHandle):
(WebKit::WebAutomationSession::didResolveChildFrame):
(WebKit::WebAutomationSession::resolveParentFrameHandle):
(WebKit::WebAutomationSession::didResolveParentFrame):
(WebKit::WebAutomationSession::computeElementLayout):
(WebKit::WebAutomationSession::didComputeElementLayout):
(WebKit::WebAutomationSession::isShowingJavaScriptDialog):
(WebKit::WebAutomationSession::dismissCurrentJavaScriptDialog):
(WebKit::WebAutomationSession::acceptCurrentJavaScriptDialog):
(WebKit::WebAutomationSession::messageOfCurrentJavaScriptDialog):
(WebKit::WebAutomationSession::setUserInputForCurrentJavaScriptPrompt):
(WebKit::WebAutomationSession::getAllCookies):
(WebKit::WebAutomationSession::didGetCookiesForFrame):
(WebKit::WebAutomationSession::deleteSingleCookie):
(WebKit::WebAutomationSession::didDeleteCookie):
(WebKit::WebAutomationSession::addSingleCookie):
(WebKit::WebAutomationSession::deleteAllCookies):
(WebKit::WebAutomationSession::performMouseInteraction):
(WebKit::WebAutomationSession::performKeyboardInteractions):
(WebKit::WebAutomationSession::takeScreenshot):
(WebKit::WebAutomationSession::didTakeScreenshot):
Audit almost all early exits and provide a relevant error details message
if the error itself is ambiguous. Make sure to format asynchronous errors.

Modified Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (199679 => 199680)


--- trunk/Source/WebKit2/ChangeLog	2016-04-18 17:30:36 UTC (rev 199679)
+++ trunk/Source/WebKit2/ChangeLog	2016-04-18 17:46:01 UTC (rev 199680)
@@ -1,3 +1,56 @@
+2016-04-18  Brian Burg  <bb...@apple.com>
+
+        Web Automation: provide detailed error messages when an automation command fails
+        https://bugs.webkit.org/show_bug.cgi?id=156635
+        <rdar://problem/25754051>
+
+        Reviewed by Darin Adler and Timothy Hatcher.
+
+        Fill in lots of missing error message details now that the remote end knows
+        how to parse error strings with predefined error names and details.
+
+        * UIProcess/Automation/WebAutomationSession.cpp:
+        Rearrange the error message macros. Make it possible to pass a ErrorMessage
+        variable or just the enum value name and get out an encoded error message
+        that optionally includes a free-form details string. The predefined error
+        name and the details string are joined together with a semicolon.
+
+        (WebKit::WebAutomationSession::getBrowsingContext):
+        (WebKit::WebAutomationSession::createBrowsingContext):
+        (WebKit::WebAutomationSession::closeBrowsingContext):
+        (WebKit::WebAutomationSession::switchToBrowsingContext):
+        (WebKit::WebAutomationSession::resizeWindowOfBrowsingContext):
+        (WebKit::WebAutomationSession::moveWindowOfBrowsingContext):
+        (WebKit::WebAutomationSession::navigateBrowsingContext):
+        (WebKit::WebAutomationSession::goBackInBrowsingContext):
+        (WebKit::WebAutomationSession::goForwardInBrowsingContext):
+        (WebKit::WebAutomationSession::reloadBrowsingContext):
+        (WebKit::WebAutomationSession::evaluateJavaScriptFunction):
+        (WebKit::WebAutomationSession::didEvaluateJavaScriptFunction):
+        (WebKit::WebAutomationSession::resolveChildFrameHandle):
+        (WebKit::WebAutomationSession::didResolveChildFrame):
+        (WebKit::WebAutomationSession::resolveParentFrameHandle):
+        (WebKit::WebAutomationSession::didResolveParentFrame):
+        (WebKit::WebAutomationSession::computeElementLayout):
+        (WebKit::WebAutomationSession::didComputeElementLayout):
+        (WebKit::WebAutomationSession::isShowingJavaScriptDialog):
+        (WebKit::WebAutomationSession::dismissCurrentJavaScriptDialog):
+        (WebKit::WebAutomationSession::acceptCurrentJavaScriptDialog):
+        (WebKit::WebAutomationSession::messageOfCurrentJavaScriptDialog):
+        (WebKit::WebAutomationSession::setUserInputForCurrentJavaScriptPrompt):
+        (WebKit::WebAutomationSession::getAllCookies):
+        (WebKit::WebAutomationSession::didGetCookiesForFrame):
+        (WebKit::WebAutomationSession::deleteSingleCookie):
+        (WebKit::WebAutomationSession::didDeleteCookie):
+        (WebKit::WebAutomationSession::addSingleCookie):
+        (WebKit::WebAutomationSession::deleteAllCookies):
+        (WebKit::WebAutomationSession::performMouseInteraction):
+        (WebKit::WebAutomationSession::performKeyboardInteractions):
+        (WebKit::WebAutomationSession::takeScreenshot):
+        (WebKit::WebAutomationSession::didTakeScreenshot):
+        Audit almost all early exits and provide a relevant error details message
+        if the error itself is ambiguous. Make sure to format asynchronous errors.
+
 2016-04-18  Brent Fulgham  <bfulg...@apple.com>
 
         CSP: Remove stubs for dynamically-added favicons (via link rel="icon")

Modified: trunk/Source/WebKit2/UIProcess/Automation/WebAutomationSession.cpp (199679 => 199680)


--- trunk/Source/WebKit2/UIProcess/Automation/WebAutomationSession.cpp	2016-04-18 17:30:36 UTC (rev 199679)
+++ trunk/Source/WebKit2/UIProcess/Automation/WebAutomationSession.cpp	2016-04-18 17:46:01 UTC (rev 199680)
@@ -38,16 +38,36 @@
 #include <WebCore/UUID.h>
 #include <algorithm>
 #include <wtf/HashMap.h>
+#include <wtf/text/StringConcatenate.h>
 
 using namespace Inspector;
 
-#define FAIL_WITH_PREDEFINED_ERROR_MESSAGE(messageName) \
+static const char* const errorNameAndDetailsSeparator = ";";
+
+// Make sure the predefined error name is valid, otherwise use InternalError.
+#define VALIDATED_ERROR_MESSAGE(errorString) Inspector::Protocol::AutomationHelpers::parseEnumValueFromString<Inspector::Protocol::Automation::ErrorMessage>(errorString).valueOr(Inspector::Protocol::Automation::ErrorMessage::InternalError)
+
+// If the error name is incorrect for these macros, it will be a compile-time error.
+#define STRING_FOR_PREDEFINED_ERROR_NAME(errorName) Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::errorName)
+#define STRING_FOR_PREDEFINED_ERROR_NAME_AND_DETAILS(errorName, detailsString) makeString(Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::errorName), errorNameAndDetailsSeparator, detailsString)
+
+// If the error message is not a predefined error, InternalError will be used instead.
+#define STRING_FOR_PREDEFINED_ERROR_MESSAGE(errorMessage) Inspector::Protocol::AutomationHelpers::getEnumConstantValue(VALIDATED_ERROR_MESSAGE(errorMessage))
+#define STRING_FOR_PREDEFINED_ERROR_MESSAGE_AND_DETAILS(errorMessage, detailsString) makeString(Inspector::Protocol::AutomationHelpers::getEnumConstantValue(VALIDATED_ERROR_MESSAGE(errorMessage)), errorNameAndDetailsSeparator, detailsString)
+
+// Convenience macros for filling in the error string of synchronous commands in bailout branches.
+#define FAIL_WITH_PREDEFINED_ERROR(errorName) \
 do { \
-    auto enumValue = Inspector::Protocol::Automation::ErrorMessage::messageName; \
-    errorString = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(enumValue); \
+    errorString = STRING_FOR_PREDEFINED_ERROR_NAME(errorName); \
     return; \
 } while (false)
 
+#define FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(errorName, detailsString) \
+do { \
+    errorString = STRING_FOR_PREDEFINED_ERROR_NAME_AND_DETAILS(errorName, detailsString); \
+    return; \
+} while (false)
+
 namespace WebKit {
 
 WebAutomationSession::WebAutomationSession()
@@ -237,7 +257,7 @@
 {
     WebPageProxy* page = webPageProxyForHandle(handle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
     context = buildBrowsingContextForPage(*page);
 }
@@ -246,11 +266,11 @@
 {
     ASSERT(m_client);
     if (!m_client)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(InternalError);
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InternalError, "The remote session could not request a new browsing context.");
 
     WebPageProxy* page = m_client->didRequestNewWindow(this);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(InternalError);
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InternalError, "The remote session failed to create a new browsing context.");
 
     m_activeBrowsingContextHandle = *handle = handleForWebPageProxy(*page);
 }
@@ -259,7 +279,7 @@
 {
     WebPageProxy* page = webPageProxyForHandle(handle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
     if (handle == m_activeBrowsingContextHandle)
         m_activeBrowsingContextHandle = emptyString();
@@ -271,11 +291,11 @@
 {
     WebPageProxy* page = webPageProxyForHandle(browsingContextHandle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
     WebFrameProxy* frame = webFrameProxyForHandle(optionalFrameHandle ? *optionalFrameHandle : emptyString(), *page);
     if (!frame)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(FrameNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(FrameNotFound);
 
     // FIXME: We don't need to track this in WK2. Remove in a follow up.
     m_activeBrowsingContextHandle = browsingContextHandle;
@@ -286,86 +306,86 @@
 
 void WebAutomationSession::resizeWindowOfBrowsingContext(Inspector::ErrorString& errorString, const String& handle, const Inspector::InspectorObject& sizeObject)
 {
-    // FIXME <rdar://problem/25094106>: Specify what parameter was missing or invalid and how.
-    // This requires some changes to the other end's error handling. Right now it looks for an
-    // exact error message match. We could stuff this into the 'data' field on error object.
     float width;
     if (!sizeObject.getDouble(WTF::ASCIILiteral("width"), width))
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(MissingParameter);
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(MissingParameter, "The 'width' parameter was not found or invalid.");
 
     float height;
     if (!sizeObject.getDouble(WTF::ASCIILiteral("height"), height))
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(MissingParameter);
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(MissingParameter, "The 'height' parameter was not found or invalid.");
 
-    if (width < 0 || height < 0)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(InvalidParameter);
+    if (width < 0)
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "The 'width' parameter had an invalid value.");
 
+    if (height < 0)
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "The 'height' parameter had an invalid value.");
+
     WebPageProxy* page = webPageProxyForHandle(handle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
     WebCore::FloatRect originalFrame;
     page->getWindowFrame(originalFrame);
-    
+
     WebCore::FloatRect newFrame = WebCore::FloatRect(originalFrame.location(), WebCore::FloatSize(width, height));
     if (newFrame == originalFrame)
         return;
 
     page->setWindowFrame(newFrame);
-    
+
     // If nothing changed at all, it's probably fair to report that something went wrong.
     // (We can't assume that the requested frame size will be honored exactly, however.)
     WebCore::FloatRect updatedFrame;
     page->getWindowFrame(updatedFrame);
     if (originalFrame == updatedFrame)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(InternalError);
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InternalError, "The window size was expected to have changed, but did not.");
 }
 
 void WebAutomationSession::moveWindowOfBrowsingContext(Inspector::ErrorString& errorString, const String& handle, const Inspector::InspectorObject& positionObject)
 {
-    // FIXME <rdar://problem/25094106>: Specify what parameter was missing or invalid and how.
-    // This requires some changes to the other end's error handling. Right now it looks for an
-    // exact error message match. We could stuff this into the 'data' field on error object.
     float x;
     if (!positionObject.getDouble(WTF::ASCIILiteral("x"), x))
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(MissingParameter);
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(MissingParameter, "The 'x' parameter was not found or invalid.");
 
     float y;
     if (!positionObject.getDouble(WTF::ASCIILiteral("y"), y))
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(MissingParameter);
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(MissingParameter, "The 'y' parameter was not found or invalid.");
 
-    if (x < 0 || y < 0)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(InvalidParameter);
+    if (x < 0)
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "The 'x' parameter had an invalid value.");
 
+    if (y < 0)
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "The 'y' parameter had an invalid value.");
+
     WebPageProxy* page = webPageProxyForHandle(handle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
     WebCore::FloatRect originalFrame;
     page->getWindowFrame(originalFrame);
-    
+
     WebCore::FloatRect newFrame = WebCore::FloatRect(WebCore::FloatPoint(x, y), originalFrame.size());
     if (newFrame == originalFrame)
         return;
 
     page->setWindowFrame(newFrame);
-    
+
     // If nothing changed at all, it's probably fair to report that something went wrong.
     // (We can't assume that the requested frame size will be honored exactly, however.)
     WebCore::FloatRect updatedFrame;
     page->getWindowFrame(updatedFrame);
     if (originalFrame == updatedFrame)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(InternalError);
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InternalError, "The window position was expected to have changed, but did not.");
 }
 
 void WebAutomationSession::navigateBrowsingContext(Inspector::ErrorString& errorString, const String& handle, const String& url, Ref<NavigateBrowsingContextCallback>&& callback)
 {
     WebPageProxy* page = webPageProxyForHandle(handle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
     if (auto callback = m_pendingNavigationInBrowsingContextCallbacksPerPage.take(page->pageID()))
-        callback->sendFailure(Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::Timeout));
+        callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_NAME(Timeout));
     m_pendingNavigationInBrowsingContextCallbacksPerPage.set(page->pageID(), WTFMove(callback));
 
     page->loadRequest(WebCore::URL(WebCore::URL(), url));
@@ -375,10 +395,10 @@
 {
     WebPageProxy* page = webPageProxyForHandle(handle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
     if (auto callback = m_pendingNavigationInBrowsingContextCallbacksPerPage.take(page->pageID()))
-        callback->sendFailure(Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::Timeout));
+        callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_NAME(Timeout));
     m_pendingNavigationInBrowsingContextCallbacksPerPage.set(page->pageID(), WTFMove(callback));
 
     page->goBack();
@@ -388,10 +408,10 @@
 {
     WebPageProxy* page = webPageProxyForHandle(handle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
     if (auto callback = m_pendingNavigationInBrowsingContextCallbacksPerPage.take(page->pageID()))
-        callback->sendFailure(Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::Timeout));
+        callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_NAME(Timeout));
     m_pendingNavigationInBrowsingContextCallbacksPerPage.set(page->pageID(), WTFMove(callback));
 
     page->goForward();
@@ -401,10 +421,10 @@
 {
     WebPageProxy* page = webPageProxyForHandle(handle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
     if (auto callback = m_pendingNavigationInBrowsingContextCallbacksPerPage.take(page->pageID()))
-        callback->sendFailure(Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::Timeout));
+        callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_NAME(Timeout));
     m_pendingNavigationInBrowsingContextCallbacksPerPage.set(page->pageID(), WTFMove(callback));
 
     const bool reloadFromOrigin = false;
@@ -422,11 +442,11 @@
 {
     WebPageProxy* page = webPageProxyForHandle(browsingContextHandle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
     WebFrameProxy* frame = webFrameProxyForHandle(optionalFrameHandle ? *optionalFrameHandle : emptyString(), *page);
     if (!frame)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(FrameNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(FrameNotFound);
 
     Vector<String> argumentsVector;
     argumentsVector.reserveCapacity(arguments.length());
@@ -452,25 +472,24 @@
     if (!callback)
         return;
 
-    if (!errorType.isEmpty()) {
-        // FIXME: We should send both the errorType and result, since result has the specific exception message.
-        callback->sendFailure(errorType);
-    } else
+    if (!errorType.isEmpty())
+        callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_MESSAGE_AND_DETAILS(errorType, result));
+    else
         callback->sendSuccess(result);
 }
 
 void WebAutomationSession::resolveChildFrameHandle(Inspector::ErrorString& errorString, const String& browsingContextHandle, const String* optionalFrameHandle, const int* optionalOrdinal, const String* optionalName, const String* optionalNodeHandle, Ref<ResolveChildFrameHandleCallback>&& callback)
 {
     if (!optionalOrdinal && !optionalName && !optionalNodeHandle)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(MissingParameter);
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(MissingParameter, "Command must specify a child frame by ordinal, name, or element handle.");
 
     WebPageProxy* page = webPageProxyForHandle(browsingContextHandle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
     WebFrameProxy* frame = webFrameProxyForHandle(optionalFrameHandle ? *optionalFrameHandle : emptyString(), *page);
     if (!frame)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(FrameNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(FrameNotFound);
 
     uint64_t callbackID = m_nextResolveFrameCallbackID++;
     m_resolveChildFrameHandleCallbacks.set(callbackID, WTFMove(callback));
@@ -500,7 +519,7 @@
         return;
 
     if (!errorType.isEmpty())
-        callback->sendFailure(errorType);
+        callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_MESSAGE(errorType));
     else
         callback->sendSuccess(handleForWebFrameID(frameID));
 }
@@ -509,11 +528,11 @@
 {
     WebPageProxy* page = webPageProxyForHandle(browsingContextHandle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
     WebFrameProxy* frame = webFrameProxyForHandle(frameHandle, *page);
     if (!frame)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(FrameNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(FrameNotFound);
 
     uint64_t callbackID = m_nextResolveParentFrameCallbackID++;
     m_resolveParentFrameHandleCallbacks.set(callbackID, WTFMove(callback));
@@ -528,7 +547,7 @@
         return;
 
     if (!errorType.isEmpty())
-        callback->sendFailure(errorType);
+        callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_MESSAGE(errorType));
     else
         callback->sendSuccess(handleForWebFrameID(frameID));
 }
@@ -537,11 +556,11 @@
 {
     WebPageProxy* page = webPageProxyForHandle(browsingContextHandle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
     WebFrameProxy* frame = webFrameProxyForHandle(frameHandle, *page);
     if (!frame)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(FrameNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(FrameNotFound);
 
     uint64_t callbackID = m_nextComputeElementLayoutCallbackID++;
     m_computeElementLayoutCallbacks.set(callbackID, WTFMove(callback));
@@ -559,7 +578,7 @@
         return;
 
     if (!errorType.isEmpty()) {
-        callback->sendFailure(errorType);
+        callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_MESSAGE(errorType));
         return;
     }
 
@@ -585,11 +604,11 @@
 {
     ASSERT(m_client);
     if (!m_client)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(InternalError);
+        FAIL_WITH_PREDEFINED_ERROR(InternalError);
 
     WebPageProxy* page = webPageProxyForHandle(browsingContextHandle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
     *result = m_client->isShowingJavaScriptDialogOnPage(this, page);
 }
@@ -598,14 +617,14 @@
 {
     ASSERT(m_client);
     if (!m_client)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(InternalError);
+        FAIL_WITH_PREDEFINED_ERROR(InternalError);
 
     WebPageProxy* page = webPageProxyForHandle(browsingContextHandle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
     if (!m_client->isShowingJavaScriptDialogOnPage(this, page))
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(NoJavaScriptDialog);
+        FAIL_WITH_PREDEFINED_ERROR(NoJavaScriptDialog);
 
     m_client->dismissCurrentJavaScriptDialogOnPage(this, page);
 }
@@ -614,14 +633,14 @@
 {
     ASSERT(m_client);
     if (!m_client)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(InternalError);
+        FAIL_WITH_PREDEFINED_ERROR(InternalError);
 
     WebPageProxy* page = webPageProxyForHandle(browsingContextHandle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
     if (!m_client->isShowingJavaScriptDialogOnPage(this, page))
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(NoJavaScriptDialog);
+        FAIL_WITH_PREDEFINED_ERROR(NoJavaScriptDialog);
 
     m_client->acceptCurrentJavaScriptDialogOnPage(this, page);
 }
@@ -630,14 +649,14 @@
 {
     ASSERT(m_client);
     if (!m_client)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(InternalError);
+        FAIL_WITH_PREDEFINED_ERROR(InternalError);
 
     WebPageProxy* page = webPageProxyForHandle(browsingContextHandle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
     if (!m_client->isShowingJavaScriptDialogOnPage(this, page))
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(NoJavaScriptDialog);
+        FAIL_WITH_PREDEFINED_ERROR(NoJavaScriptDialog);
 
     *text = m_client->messageOfCurrentJavaScriptDialogOnPage(this, page);
 }
@@ -646,14 +665,14 @@
 {
     ASSERT(m_client);
     if (!m_client)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(InternalError);
+        FAIL_WITH_PREDEFINED_ERROR(InternalError);
 
     WebPageProxy* page = webPageProxyForHandle(browsingContextHandle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
     if (!m_client->isShowingJavaScriptDialogOnPage(this, page))
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(NoJavaScriptDialog);
+        FAIL_WITH_PREDEFINED_ERROR(NoJavaScriptDialog);
 
     m_client->setUserInputForCurrentJavaScriptPromptOnPage(this, page, promptValue);
 }
@@ -662,12 +681,12 @@
 {
     WebPageProxy* page = webPageProxyForHandle(browsingContextHandle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
     WebFrameProxy* mainFrame = page->mainFrame();
     ASSERT(mainFrame);
     if (!mainFrame)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(FrameNotFound);
 
     uint64_t callbackID = m_nextGetCookiesCallbackID++;
     m_getCookieCallbacks.set(callbackID, WTFMove(callback));
@@ -707,7 +726,7 @@
         return;
 
     if (!errorType.isEmpty()) {
-        callback->sendFailure(errorType);
+        callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_MESSAGE(errorType));
         return;
     }
 
@@ -718,12 +737,12 @@
 {
     WebPageProxy* page = webPageProxyForHandle(browsingContextHandle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
     WebFrameProxy* mainFrame = page->mainFrame();
     ASSERT(mainFrame);
     if (!mainFrame)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(FrameNotFound);
 
     uint64_t callbackID = m_nextDeleteCookieCallbackID++;
     m_deleteCookieCallbacks.set(callbackID, WTFMove(callback));
@@ -738,7 +757,7 @@
         return;
 
     if (!errorType.isEmpty()) {
-        callback->sendFailure(errorType);
+        callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_MESSAGE(errorType));
         return;
     }
 
@@ -749,7 +768,7 @@
 {
     WebPageProxy* page = webPageProxyForHandle(browsingContextHandle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
     WebCore::URL activeURL = WebCore::URL(WebCore::URL(), page->pageLoadState().activeURL());
     ASSERT(activeURL.isValid());
@@ -757,14 +776,14 @@
     WebCore::Cookie cookie;
 
     if (!cookieObject.getString(WTF::ASCIILiteral("name"), cookie.name))
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(MissingParameter);
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(MissingParameter, "The parameter 'name' was not found.");
 
     if (!cookieObject.getString(WTF::ASCIILiteral("value"), cookie.value))
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(MissingParameter);
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(MissingParameter, "The parameter 'value' was not found.");
 
     String domain;
     if (!cookieObject.getString(WTF::ASCIILiteral("domain"), domain))
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(MissingParameter);
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(MissingParameter, "The parameter 'domain' was not found.");
 
     // Inherit the domain/host from the main frame's URL if it is not explicitly set.
     if (domain.isEmpty())
@@ -773,22 +792,22 @@
     cookie.domain = domain;
 
     if (!cookieObject.getString(WTF::ASCIILiteral("path"), cookie.path))
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(MissingParameter);
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(MissingParameter, "The parameter 'path' was not found.");
 
     double expires;
     if (!cookieObject.getDouble(WTF::ASCIILiteral("expires"), expires))
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(MissingParameter);
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(MissingParameter, "The parameter 'expires' was not found.");
 
     cookie.expires = expires * 1000.0;
 
     if (!cookieObject.getBoolean(WTF::ASCIILiteral("secure"), cookie.secure))
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(MissingParameter);
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(MissingParameter, "The parameter 'secure' was not found.");
 
     if (!cookieObject.getBoolean(WTF::ASCIILiteral("session"), cookie.session))
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(MissingParameter);
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(MissingParameter, "The parameter 'session' was not found.");
 
     if (!cookieObject.getBoolean(WTF::ASCIILiteral("httpOnly"), cookie.httpOnly))
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(MissingParameter);
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(MissingParameter, "The parameter 'httpOnly' was not found.");
 
     WebCookieManagerProxy* cookieManager = m_processPool->supplement<WebCookieManagerProxy>();
     cookieManager->addCookie(cookie, activeURL.host());
@@ -800,7 +819,7 @@
 {
     WebPageProxy* page = webPageProxyForHandle(browsingContextHandle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
     WebCore::URL activeURL = WebCore::URL(WebCore::URL(), page->pageLoadState().activeURL());
     ASSERT(activeURL.isValid());
@@ -830,22 +849,19 @@
 void WebAutomationSession::performMouseInteraction(Inspector::ErrorString& errorString, const String& handle, const Inspector::InspectorObject& requestedPositionObject, const String& mouseButtonString, const String& mouseInteractionString, const Inspector::InspectorArray& keyModifierStrings, RefPtr<Inspector::Protocol::Automation::Point>& updatedPositionObject)
 {
 #if !USE(APPKIT)
-    FAIL_WITH_PREDEFINED_ERROR_MESSAGE(NotImplemented);
+    FAIL_WITH_PREDEFINED_ERROR(NotImplemented);
 #else
     WebPageProxy* page = webPageProxyForHandle(handle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
-    // FIXME <rdar://problem/25094106>: Specify what parameter was missing or invalid and how.
-    // This requires some changes to the other end's error handling. Right now it looks for an
-    // exact error message match. We could stuff this into the 'data' field on error object.
     float x;
     if (!requestedPositionObject.getDouble(WTF::ASCIILiteral("x"), x))
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(MissingParameter);
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(MissingParameter, "The parameter 'x' was not found.");
 
     float y;
     if (!requestedPositionObject.getDouble(WTF::ASCIILiteral("y"), y))
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(MissingParameter);
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(MissingParameter, "The parameter 'y' was not found.");
 
     WebCore::FloatRect windowFrame;
     page->getWindowFrame(windowFrame);
@@ -857,21 +873,21 @@
 
     auto parsedInteraction = Inspector::Protocol::AutomationHelpers::parseEnumValueFromString<Inspector::Protocol::Automation::MouseInteraction>(mouseInteractionString);
     if (!parsedInteraction)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(InvalidParameter);
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "The parameter 'interaction' is invalid.");
 
     auto parsedButton = Inspector::Protocol::AutomationHelpers::parseEnumValueFromString<Inspector::Protocol::Automation::MouseButton>(mouseButtonString);
     if (!parsedButton)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(InvalidParameter);
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "The parameter 'button' is invalid.");
 
     WebEvent::Modifiers keyModifiers = (WebEvent::Modifiers)0;
     for (auto it = keyModifierStrings.begin(); it != keyModifierStrings.end(); ++it) {
         String modifierString;
         if (!it->get()->asString(modifierString))
-            FAIL_WITH_PREDEFINED_ERROR_MESSAGE(InvalidParameter);
+            FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "The parameter 'modifiers' is invalid.");
 
         auto parsedModifier = Inspector::Protocol::AutomationHelpers::parseEnumValueFromString<Inspector::Protocol::Automation::KeyModifier>(modifierString);
         if (!parsedModifier)
-            FAIL_WITH_PREDEFINED_ERROR_MESSAGE(InvalidParameter);
+            FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "A modifier in the 'modifiers' array is invalid.");
         WebEvent::Modifiers enumValue = protocolModifierToWebEventModifier(parsedModifier.value());
         keyModifiers = (WebEvent::Modifiers)(enumValue | keyModifiers);
     }
@@ -888,14 +904,14 @@
 void WebAutomationSession::performKeyboardInteractions(ErrorString& errorString, const String& handle, const Inspector::InspectorArray& interactions)
 {
 #if !USE(APPKIT)
-    FAIL_WITH_PREDEFINED_ERROR_MESSAGE(NotImplemented);
+    FAIL_WITH_PREDEFINED_ERROR(NotImplemented);
 #else
     WebPageProxy* page = webPageProxyForHandle(handle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
     if (!interactions.length())
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(InvalidParameter);
+        FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "The parameter 'interactions' was not found or empty.");
 
     // Validate all of the parameters before performing any interactions with the browsing context under test.
     Vector<std::function<void()>> actionsToPerform;
@@ -904,21 +920,21 @@
     for (auto interaction : interactions) {
         RefPtr<InspectorObject> interactionObject;
         if (!interaction->asObject(interactionObject))
-            FAIL_WITH_PREDEFINED_ERROR_MESSAGE(InvalidParameter);
+            FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "An interaction in the 'interactions' parameter was invalid.");
 
         String interactionTypeString;
         if (!interactionObject->getString(ASCIILiteral("type"), interactionTypeString))
-            FAIL_WITH_PREDEFINED_ERROR_MESSAGE(InvalidParameter);
+            FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "An interaction in the 'interactions' parameter is missing the 'type' key.");
         auto interactionType = Inspector::Protocol::AutomationHelpers::parseEnumValueFromString<Inspector::Protocol::Automation::KeyboardInteractionType>(interactionTypeString);
         if (!interactionType)
-            FAIL_WITH_PREDEFINED_ERROR_MESSAGE(InvalidParameter);
+            FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "An interaction in the 'interactions' parameter has an invalid 'type' key.");
 
         String virtualKeyString;
         bool foundVirtualKey = interactionObject->getString(ASCIILiteral("key"), virtualKeyString);
         if (foundVirtualKey) {
             auto virtualKey = Inspector::Protocol::AutomationHelpers::parseEnumValueFromString<Inspector::Protocol::Automation::VirtualKey>(virtualKeyString);
             if (!virtualKey)
-                FAIL_WITH_PREDEFINED_ERROR_MESSAGE(InvalidParameter);
+                FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "An interaction in the 'interactions' parameter has an invalid 'key' value.");
 
             actionsToPerform.uncheckedAppend([this, page, interactionType, virtualKey] {
                 platformSimulateKeyStroke(*page, interactionType.value(), virtualKey.value());
@@ -932,7 +948,7 @@
             case Inspector::Protocol::Automation::KeyboardInteractionType::KeyPress:
             case Inspector::Protocol::Automation::KeyboardInteractionType::KeyRelease:
                 // 'KeyPress' and 'KeyRelease' are meant for a virtual key and are not supported for a string (sequence of codepoints).
-                FAIL_WITH_PREDEFINED_ERROR_MESSAGE(InvalidParameter);
+                FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "An interaction in the 'interactions' parameter has an invalid 'key' value.");
 
             case Inspector::Protocol::Automation::KeyboardInteractionType::InsertByKey:
                 actionsToPerform.uncheckedAppend([this, page, keySequence] {
@@ -943,7 +959,7 @@
         }
 
         if (!foundVirtualKey && !foundKeySequence)
-            FAIL_WITH_PREDEFINED_ERROR_MESSAGE(MissingParameter);
+            FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(MissingParameter, "An interaction in the 'interactions' parameter is missing both 'key' and 'text'. One must be provided.");
     }
 
     ASSERT(actionsToPerform.size());
@@ -957,7 +973,7 @@
 {
     WebPageProxy* page = webPageProxyForHandle(handle);
     if (!page)
-        FAIL_WITH_PREDEFINED_ERROR_MESSAGE(WindowNotFound);
+        FAIL_WITH_PREDEFINED_ERROR(WindowNotFound);
 
     uint64_t callbackID = m_nextScreenshotCallbackID++;
     m_screenshotCallbacks.set(callbackID, WTFMove(callback));
@@ -972,13 +988,13 @@
         return;
 
     if (!errorType.isEmpty()) {
-        callback->sendFailure(errorType);
+        callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_MESSAGE(errorType));
         return;
     }
 
     String base64EncodedData = platformGetBase64EncodedPNGData(imageDataHandle);
     if (base64EncodedData.isEmpty()) {
-        callback->sendFailure(Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::InternalError));
+        callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_NAME(InternalError));
         return;
     }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to