From 4c29fe55b981fa36e875ca6f1040cf20371ce75d Mon Sep 17 00:00:00 2001
From: Carlos Garcia Campos <cgarcia@igalia.com>
Date: Mon, 16 Oct 2017 12:15:32 +0000
Subject: [PATCH] Merge r222255 - WebDriver: wrong response in case of errors
 https://bugs.webkit.org/show_bug.cgi?id=177127

Reviewed by Brian Burg.

I misunderstood the spec when I implemented this, so we either return a "value" key with the result in case of
success or the error object as the body in case of error. We should always add a "value" key to the body and set
it with either the result or the error object.

https://w3c.github.io/webdriver/webdriver-spec.html#dfn-send-an-error

* WebDriverService.cpp:
(WebDriver::WebDriverService::sendResponse const):
---
 Source/WebDriver/ChangeLog            | 16 +++++++++++++
 Source/WebDriver/WebDriverService.cpp | 34 ++++++++++++++++++---------
 2 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/Source/WebDriver/ChangeLog b/Source/WebDriver/ChangeLog
index b445ae15e362..ee13f4accb77 100644
--- a/Source/WebDriver/ChangeLog
+++ b/Source/WebDriver/ChangeLog
@@ -1,3 +1,19 @@
+2017-09-19  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        WebDriver: wrong response in case of errors
+        https://bugs.webkit.org/show_bug.cgi?id=177127
+
+        Reviewed by Brian Burg.
+
+        I misunderstood the spec when I implemented this, so we either return a "value" key with the result in case of
+        success or the error object as the body in case of error. We should always add a "value" key to the body and set
+        it with either the result or the error object.
+
+        https://w3c.github.io/webdriver/webdriver-spec.html#dfn-send-an-error
+
+        * WebDriverService.cpp:
+        (WebDriver::WebDriverService::sendResponse const):
+
 2017-09-18  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         WebDriver: wrong key name for capabilities in new session response
diff --git a/Source/WebDriver/WebDriverService.cpp b/Source/WebDriver/WebDriverService.cpp
index 7b97e3b9bd84..cb745c55e95e 100644
--- a/Source/WebDriver/WebDriverService.cpp
+++ b/Source/WebDriver/WebDriverService.cpp
@@ -248,19 +248,31 @@ void WebDriverService::handleRequest(HTTPRequestHandler::Request&& request, Func
 
 void WebDriverService::sendResponse(Function<void (HTTPRequestHandler::Response&&)>&& replyHandler, CommandResult&& result) const
 {
-    RefPtr<InspectorObject> responseObject;
+    // §6.3 Processing Model.
+    // https://w3c.github.io/webdriver/webdriver-spec.html#processing-model
+    RefPtr<InspectorValue> resultValue;
     if (result.isError()) {
-        responseObject = InspectorObject::create();
-        responseObject->setString(ASCIILiteral("error"), result.errorString());
-        responseObject->setString(ASCIILiteral("message"), result.errorMessage().value_or(emptyString()));
-        responseObject->setString(ASCIILiteral("stacktrace"), emptyString());
+        // When required to send an error.
+        // https://w3c.github.io/webdriver/webdriver-spec.html#dfn-send-an-error
+        // Let body be a new JSON Object initialised with the following properties: "error", "message", "stacktrace".
+        auto errorObject = InspectorObject::create();
+        errorObject->setString(ASCIILiteral("error"), result.errorString());
+        errorObject->setString(ASCIILiteral("message"), result.errorMessage().value_or(emptyString()));
+        errorObject->setString(ASCIILiteral("stacktrace"), emptyString());
+        // If the error data dictionary contains any entries, set the "data" field on body to a new JSON Object populated with the dictionary.
         if (auto& additionalData = result.additionalErrorData())
-            responseObject->setObject(ASCIILiteral("data"), RefPtr<InspectorObject> { additionalData });
-    } else {
-        responseObject = InspectorObject::create();
-        auto resultValue = result.result();
-        responseObject->setValue(ASCIILiteral("value"), resultValue ? WTFMove(resultValue) : InspectorValue::null());
-    }
+            errorObject->setObject(ASCIILiteral("data"), RefPtr<InspectorObject> { additionalData });
+        // Send a response with status and body as arguments.
+        resultValue = WTFMove(errorObject);
+    } else if (auto value = result.result())
+        resultValue = WTFMove(value);
+    else
+        resultValue = InspectorValue::null();
+
+    // When required to send a response.
+    // https://w3c.github.io/webdriver/webdriver-spec.html#dfn-send-a-response
+    RefPtr<InspectorObject> responseObject = InspectorObject::create();
+    responseObject->setValue(ASCIILiteral("value"), WTFMove(resultValue));
     replyHandler({ result.httpStatusCode(), responseObject->toJSONString().utf8(), ASCIILiteral("application/json; charset=utf-8") });
 }