[ServiceWorker] Make Response class better conformance with the spec.

Source/modules/serviceworkers/Response.cpp

Index: Source/modules/serviceworkers/Response.cpp
diff --git a/Source/modules/serviceworkers/Response.cpp b/Source/modules/serviceworkers/Response.cpp
index 51976779bc8c72151ad9b053d9e5101ed5598751..d1385bf3040a6608f5351a5a9a670134b503a5fb 100644
--- a/Source/modules/serviceworkers/Response.cpp
+++ b/Source/modules/serviceworkers/Response.cpp
@@ -8,48 +8,153 @@
 #include "bindings/core/v8/Dictionary.h"
 #include "core/fileapi/Blob.h"
 #include "modules/serviceworkers/ResponseInit.h"
-#include "platform/NotImplemented.h"
-#include "public/platform/WebServiceWorkerResponse.h"
 
 namespace WebCore {
 
-PassRefPtr<Response> Response::create(Blob* body, const Dictionary& responseInit)
+PassRefPtr<Response> Response::create(Blob* body, const Dictionary& responseInit, ExceptionState& exceptionState)
 {
-    return create(body, ResponseInit(responseInit));
+    return create(body, ResponseInit(responseInit), exceptionState);
 }
 
-PassRefPtr<Response> Response::create(Blob* body, const ResponseInit& responseInit)
+PassRefPtr<Response> Response::create(Blob* body, const ResponseInit& responseInit, ExceptionState& exceptionState)
 {
-    RefPtr<BlobDataHandle> blobDataHandle = body ? body->blobDataHandle() : nullptr;
+    // "1. If |init|'s status member is not in the range 200 to 599, throw a
+    // RangeError."
+    if (responseInit.status < 200 || 599 < responseInit.status) {
+        exceptionState.throwRangeError("Invalid status");
+        return nullptr;
+    }
 
-    // FIXME: Maybe append or override content-length and content-type headers using the blob. The spec will clarify what to do:
-    // https://github.com/slightlyoff/ServiceWorker/issues/192
-    return adoptRef(new Response(blobDataHandle.release(), responseInit));
+    // FIXME: "2. If |init|'s statusText member does not match the Reason-Phrase
+    //        token production, throw a TypeError."
+
+    // "3. Let |r| be a new Response object, associated with a new response,
+    // Headers object, and FetchBodyStream object."
+    RefPtr<Response> r = adoptRef(new Response());
+
+    // "4. Set |r|'s response's status to |init|'s status member."
+    r->m_response->setStatus(responseInit.status);
+
+    // "5. Set |r|'s response's status message to |init|'s statusText member."
+    r->m_response->setStatusMessage(AtomicString(responseInit.statusText));
+
+    // "6. If |init|'s headers member is present, run these substeps:"
+    if (responseInit.headers) {
+        // "1. Empty |r|'s response's header list."
+        r->m_response->headerList()->clearList();
+        // "2. Fill |r|'s Headers object with |init|'s headers member. Rethrow
+        // any exceptions."
+        r->m_headers->fillWith(responseInit.headers.get(), exceptionState);
+        if (exceptionState.hadException())
+            return nullptr;
+    } else if (!responseInit.headersDictionary.isUndefinedOrNull()) {
+        // "1. Empty |r|'s response's header list."
+        r->m_response->headerList()->clearList();
+        // "2. Fill |r|'s Headers object with |init|'s headers member. Rethrow
+        // any exceptions."
+        r->m_headers->fillWith(responseInit.headersDictionary, exceptionState);
+        if (exceptionState.hadException())
+            return nullptr;
+    }
+    // "7. If body is given, run these substeps:"
+    if (body) {
+        // "1. Let |stream| and |Content-Type| be the result of extracting body."
+        // "2. Set |r|'s response's body to |stream|."
+        // "3. If |r|'s response's header list contains no header named
+        // `Content-Type`, append `Content-Type`/|Content-Type| to |r|'s
+        // response's header list."
+        r->m_response->setBlobDataHandle(body->blobDataHandle());
+        if (!r->m_response->headerList()->has("Content-Type") && !body->type().isEmpty())

[falken, 2014/07/07 08:10:40]

The isEmpty() check doesn't seem to be in the quoted spec. Is there a
discrepancy here?

[horo, 2014/07/07 10:19:44]

Done.
Changed to !isNull()

[falken, 2014/07/08 03:56:24]

I'm still not sure this follows the spec. The spec seems to says you always
append "Content-Type" to |r|'s header list if it doesn't exist.

Scanning the spec's "extract body" algorithm, it looks like "Content-Type" is
set to the empty byte sequence by default. So it looks like we should be adding
"Content-Type": "" to |r| if body->type()->isNull(). WDYT?

[horo, 2014/07/08 05:04:43]

Humm..
I can't say which is correct.
I filed an issue.
https://github.com/slightlyoff/ServiceWorker/issues/363
And I changed this cl to add a empty string.

+            r->m_response->headerList()->append("Content-Type", body->type());
+    }
+
+    // FIXME: "8. Set |r|'s FetchBodyStream object's MIME type to the result of
+    //        extracting a MIME type from |r|'s response's header list."
+
+    // FIXME: "9. Set |r|'s FetchBodyStream object's codings to the result of
+    //        parsing `Content-Encoding` in |r|'s response's header list if that
+    //        result is not failure."
+
+    // "10. Return r."
+    return r.release();
 }
 
-PassRefPtr<HeaderMap> Response::headers() const
+PassRefPtr<Response> Response::create(PassRefPtr<FetchResponseData> response)
 {
-    // FIXME: Implement. Spec will eventually whitelist allowable headers.
+    return adoptRef(new Response(response));
+}
+
+String Response::type() const
+{
+    // "The type attribute's getter must return response's type."
+    switch (m_response->type()) {
+    case FetchResponseData::BasicType:
+        return "basic";
+    case FetchResponseData::CORSType:
+        return "cors";
+    case FetchResponseData::DefaultType:
+        return "default";
+    case FetchResponseData::ErrorType:
+        return "error";
+    case FetchResponseData::OpaqueType:
+        return "opaque";
+    }
+    ASSERT_NOT_REACHED();
+    return "";
+}
+
+String Response::url() const
+{
+    // "The url attribute's getter must return the empty string if response's
+    // url is null and response's url, serialized with the exclude fragment
+    // flag set, otherwise."
+    if (!m_response->url().hasFragmentIdentifier())
+        return m_response->url();
+    KURL url(m_response->url());
+    url.removeFragmentIdentifier();
+    return url;
+}
+
+
+unsigned short Response::status() const
+{
+    // "The status attribute's getter must return response's status."
+    return m_response->status();
+}
+
+

[falken, 2014/07/07 08:10:40]

extra newline

[horo, 2014/07/07 10:19:44]

Done.

+String Response::statusText() const
+{
+    // "The statusText attribute's getter must return response's status message."
+    return m_response->statusMessage();
+}
+
+

[falken, 2014/07/07 08:10:40]

extra newl

[horo, 2014/07/07 10:19:44]

Done.

+PassRefPtr<Headers> Response::headers() const
+{
+    // "The headers attribute's getter must return the associated Headers object."
     return m_headers;
 }
 
 void Response::populateWebServiceWorkerResponse(blink::WebServiceWorkerResponse& response)
 {
-    response.setStatus(status());
-    response.setStatusText(statusText());
-    response.setHeaders(m_headers->headerMap());
-    response.setBlobDataHandle(m_blobDataHandle);
+    m_response->populateWebServiceWorkerResponse(response);
+}
+
+Response::Response()
+    : m_response(FetchResponseData::create())
+    , m_headers(Headers::create(m_response->headerList()))
+{
+    m_headers->setGuard(Headers::ResponseGuard);
+    ScriptWrappable::init(this);
 }
 
-Response::Response(PassRefPtr<BlobDataHandle> blobDataHandle, const ResponseInit& responseInit)
-    : m_status(responseInit.status)
-    , m_statusText(responseInit.statusText)
-    , m_headers(responseInit.headers)
-    , m_blobDataHandle(blobDataHandle)
+Response::Response(PassRefPtr<FetchResponseData> response)
+    : m_response(response)
+    , m_headers(Headers::create(m_response->headerList()))
 {
+    m_headers->setGuard(Headers::ResponseGuard);
     ScriptWrappable::init(this);
-    if (!m_headers)
-        m_headers = HeaderMap::create();
 }
 
 } // namespace WebCore