Update comments in Request constructor to explain referrer handling more clearly

third_party/WebKit/Source/modules/fetch/Request.cpp

Index: third_party/WebKit/Source/modules/fetch/Request.cpp
diff --git a/third_party/WebKit/Source/modules/fetch/Request.cpp b/third_party/WebKit/Source/modules/fetch/Request.cpp
index 24239c4836f4347a712c41c4b901678217ac319a..12fb2a822f2c24e23ae0a9eba24d05494f45efda 100644
--- a/third_party/WebKit/Source/modules/fetch/Request.cpp
+++ b/third_party/WebKit/Source/modules/fetch/Request.cpp
@@ -120,63 +120,83 @@ Request* Request::createRequestWithRequestOrString(ScriptState* scriptState, Req
         // We don't use fallback values. We set these flags directly in below.
     }
 
-    // "14. If any of |init|'s members are present, run these substeps:"
+    // "If any of |init|'s members are present, run these substeps:"
     if (init.areAnyMembersSet) {
         // "1. If |request|'s |mode| is "navigate", throw a TypeError."
         if (request->mode() == WebURLRequest::FetchRequestModeNavigate) {
             exceptionState.throwTypeError("Cannot construct a Request with a Request whose mode is 'navigate' and a non-empty RequestInit.");
             return nullptr;
         }
-        // "2. Unset |request|'s omit-Origin-header flag."
-        // "3. Set |request|'s referrer to "client"."
-        // "4. Set |request|'s referrer policy to the empty string."
-        // => RequestInit::RequestInit.
+
+        // TODO(yhirano): Implement the following substep:
+        //   "Unset |request|'s omit-Origin-header flag."
+
+        // The step "Set |request|'s referrer to "client"." is performed by the
+        // code below as follows:
+        // - |init.referrer.referrer| gets initialized by the RequestInit
+        //   constructor to "about:client" when any of |options|'s member are

[yhirano, 2015/11/17 09:31:55]

members

[tyoshino (SeeGerritForStatus), 2015/11/18 13:42:51]

Done.

+        //   present.
+        // - The code below does the equivalent as the step specified in the
+        //   spec by processing the "about:client".
+
+        // TODO(yhirano): Implement the following substep:
+        //   "Set |request|'s referrer policy to the empty string."

[yhirano, 2015/11/17 09:31:54]

This is implemented :)

[tyoshino (SeeGerritForStatus), 2015/11/18 13:42:51]

Fixed

     }
 
-    //   15. If |init|'s referrer member is present, run these substeps:
-    // Note that JS null and undefined are encoded as an empty string and thus
+    // The following if-clause performs the following two steps:
+    // - "If |init|'s referrer member is present, run these substeps:"
+    // - "If |init|'s referrerPolicy member is present, set |request|'s

[yhirano, 2015/11/17 09:31:54]

This is not implemented :)

[tyoshino (SeeGerritForStatus), 2015/11/18 13:42:51]

Fixed

+    //   referrer policy to it."
+    //
+    // The condition "if any of |init|'s members are present"
+    // (areAnyMembersSet) is used for the if-clause instead of conditions
+    // indicating presence of each member as specified in the spec. This is to
+    // perform the substeps in the previous step together here.
+
+    // Note that JS null and undefined are encoded as an empty string, and thus

[yhirano, 2015/11/17 09:31:54]

redundant?

[tyoshino (SeeGerritForStatus), 2015/11/18 13:42:51]

Removed

     // a null string means referrer member is not set.
-    //   16. If |init|'s referrerPolicy member is present, set |request|'s
-    // referrer policy to it.
-    // areAnyMembersSet will be True, if any members in RequestInit are set and
-    // hence the referrer member
+
     if (init.areAnyMembersSet) {
-        // 1. Let |referrer| be |init|'s referrer member.
+        // Nothing to do for the step "Let |referrer| be |init|'s referrer
+        // member."
+
         if (init.referrer.referrer.isEmpty()) {
-            // 2. if |referrer| is the empty string, set |request|'s referrer to
-            // "no-referrer" and terminate these substeps.
+            // "If |referrer| is the empty string, set |request|'s referrer to
+            // "no-referrer" and terminate these substeps."
             request->setReferrerString(FetchRequestData::noReferrerString());
         } else {
-            // 3. Let |parsedReferrer| be the result of parsing |referrer| with
+            // "Let |parsedReferrer| be the result of parsing |referrer| with
             // |baseURL|.

[yhirano, 2015/11/17 09:31:55]

+"

[tyoshino (SeeGerritForStatus), 2015/11/18 13:42:51]

Done.

             KURL parsedReferrer = scriptState->executionContext()->completeURL(init.referrer.referrer);
             if (!parsedReferrer.isValid()) {
-                // 4. If |parsedReferrer| is failure, throw a TypeError.
+                // "If |parsedReferrer| is failure, throw a TypeError."
                 exceptionState.throwTypeError("Referrer '" + init.referrer.referrer + "' is not a valid URL.");
                 return nullptr;
             }
             if (parsedReferrer.protocolIsAbout() && parsedReferrer.host().isEmpty() && parsedReferrer.path() == "client") {
-                // 5. If |parsedReferrer|'s non-relative flag is set, scheme is
+                // "If |parsedReferrer|'s non-relative flag is set, scheme is
                 // "about", and path contains a single string "client", set
-                // request's referrer to "client" and terminate these substeps.
+                // request's referrer to "client" and terminate these
+                // substeps."
                 request->setReferrerString(FetchRequestData::clientReferrerString());
             } else if (!origin->isSameSchemeHostPortAndSuborigin(SecurityOrigin::create(parsedReferrer).get())) {
-                // 6. If |parsedReferrer|'s origin is not same origin with
-                // |origin|, throw a TypeError.
+                // "If |parsedReferrer|'s origin is not same origin with
+                // |origin|, throw a TypeError."
                 exceptionState.throwTypeError("The origin of '" + init.referrer.referrer + "' should be same as '" + origin->toString() + "'");
                 return nullptr;
             } else {
-                // 7. Set |request|'s referrer to |parsedReferrer|.
+                // "Set |request|'s referrer to |parsedReferrer|."
                 request->setReferrerString(AtomicString(parsedReferrer.string()));
             }
         }
         request->setReferrerPolicy(init.referrer.referrerPolicy);
     }
 
-    // "16. Let |mode| be |init|'s mode member if it is present, and
-    // |fallbackMode| otherwise."
-    // "17. If |mode| is "navigate", throw a TypeError.
-    // "18. If |mode| is non-null, set |request|'s mode to |mode|."
+    // The following code performs the following steps:
+    // - "Let |mode| be |init|'s mode member if it is present, and
+    //   |fallbackMode| otherwise."
+    // - "If |mode| is "navigate", throw a TypeError."
+    // - "If |mode| is non-null, set |request|'s mode to |mode|."
     if (init.mode == "navigate") {
         exceptionState.throwTypeError("Cannot construct a Request with a RequestInit whose mode member is set as 'navigate'.");
         return nullptr;
@@ -188,6 +208,8 @@ Request* Request::createRequestWithRequestOrString(ScriptState* scriptState, Req
     } else if (init.mode == "cors") {
         request->setMode(WebURLRequest::FetchRequestModeCORS);
     } else {
+        // |inputRequest| is directly checked here instead of setting and
+        // checking |fallbackMode| as specified in the spec.
         if (!inputRequest)
             request->setMode(WebURLRequest::FetchRequestModeCORS);
     }