CREDENTIAL: Teach Fetch to handle PasswordCredential objects.

third_party/WebKit/Source/modules/credentialmanager/PasswordCredential.cpp

Index: third_party/WebKit/Source/modules/credentialmanager/PasswordCredential.cpp
diff --git a/third_party/WebKit/Source/modules/credentialmanager/PasswordCredential.cpp b/third_party/WebKit/Source/modules/credentialmanager/PasswordCredential.cpp
index 4042182b3204aaa6c5996dcfdc7291a92bf65edf..41bf1f2fb7f1133711728cc39af00308dc2373a4 100644
--- a/third_party/WebKit/Source/modules/credentialmanager/PasswordCredential.cpp
+++ b/third_party/WebKit/Source/modules/credentialmanager/PasswordCredential.cpp
@@ -8,6 +8,7 @@
 #include "bindings/core/v8/Dictionary.h"
 #include "bindings/core/v8/ExceptionState.h"
 #include "core/dom/ExecutionContext.h"
+#include "core/dom/URLSearchParams.h"
 #include "core/html/FormData.h"
 #include "modules/credentialmanager/FormDataOptions.h"
 #include "modules/credentialmanager/PasswordCredentialData.h"
@@ -33,25 +34,47 @@ PasswordCredential* PasswordCredential::create(const PasswordCredentialData& dat
 
 PasswordCredential::PasswordCredential(WebPasswordCredential* webPasswordCredential)
     : Credential(webPasswordCredential->platformCredential())
+    , m_idName("username")
+    , m_passwordName("password")
 {
 }
 
 PasswordCredential::PasswordCredential(const String& id, const String& password, const String& name, const KURL& icon)
     : Credential(PlatformPasswordCredential::create(id, password, name, icon))
+    , m_idName("username")
+    , m_passwordName("password")
 {
 }
 
-FormData* PasswordCredential::toFormData(ScriptState* scriptState, const FormDataOptions& options)
+PassRefPtr<EncodedFormData> PasswordCredential::encodeFormData() const
 {
-    FormData* fd = FormData::create();
+    if (m_additionalData.isURLSearchParams()) {
+        // If |additionalData| is a 'URLSearchParams' object, build a urlencoded response.
+        URLSearchParams* params = URLSearchParams::create(URLSearchParamsInit());
+        URLSearchParams* additionalData = m_additionalData.getAsURLSearchParams();
+        for (const auto& param : additionalData->params())
+            params->append(param.first, param.second);
+        params->append(idName(), id());
+        params->append(passwordName(), password());
 
-    String errorMessage;
-    if (!scriptState->executionContext()->isSecureContext(errorMessage))
-        return fd;
+        return params->encodeFormData();

[philipj_slow, 2015/11/17 15:53:59]

Here and below I assume that idName() and passwordName() could be invalid
UTF-16, does the spec say what should happen, or should they be turned into
USVString in the IDL?

[Mike West, 2015/11/18 09:12:51]

They should be turned into USVString to match the interfaces for FormData and
URLSearchParams. Good eye!

+    }
 
-    fd->append(options.idName(), id());
-    fd->append(options.passwordName(), password());
-    return fd;
+    // Otherwise, we'll build a multipart response.
+    FormData* formData = FormData::create(nullptr);
+    if (m_additionalData.isFormData()) {
+        FormData* additionalData = m_additionalData.getAsFormData();
+        for (const FormData::Entry* entry : additionalData->entries()) {
+            if (entry->blob())
+                formData->append(formData->decode(entry->name()), entry->blob(), entry->filename());

[philipj_slow, 2015/11/17 15:53:59]

Should this algorithm match the FormData branch of
http://w3c.github.io/webappsec-credential-management/#monkey-patching-fetch-1 ?

I think this is right, but can you confirm that the 'with "utf-8" as the
explicit character encoding' bit in the spec is only for the actual encoding
step at the end of this function, and has nothing to do with how to intepret the
FormData object? Also, isn't there an existing implementation of
"multipart/form-data encoding algorithm" to use? It's nice when the structure of
spec and implementation is similar :)

[Mike West, 2015/11/18 09:12:51]

We would more accurately follow the spec if we moved all of this code into
`RequestInit`. I don't really want to do that, as I'd prefer to keep
`PasswordCredential` in charge of its own encoding. Perhaps that means the spec
changes could be restructured to call out to something that the Credential
Management spec defines. Now that I think about it, that sounds like a pretty
reasonable change, and pretty much in line with what we've done in other specs
to provide Fetch hooks.
Right. The `FormData` object itself just puts `Entries` (USVString pairs,
key/blob pairs) into a list. The encoding piece at the end is what needs to
worry about the text encoding of those values.
Chromium's basically implemented that algorithm as part of the FormData object
(see
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...),
which we call at #77 to produce the EncodedFormData. I don't think it's worth
trying to extract that code to match the spec language, as we only ever use it
in cases where we actually have a FormData object.

[philipj_slow, 2015/11/18 10:00:05]

OK, possible future spec refactoring it is!
Ack.
I see, it was close by all along. Makes sense the way it is.

+            else
+                formData->append(formData->decode(entry->name()), formData->decode(entry->value()));
+        }
+    }
+    formData->append(idName(), id());
+    formData->append(passwordName(), password());
+
+    return formData->encodeMultiPartFormData();
 }
 
 const String& PasswordCredential::password() const
@@ -62,6 +85,7 @@ const String& PasswordCredential::password() const
 DEFINE_TRACE(PasswordCredential)
 {
     Credential::trace(visitor);
+    visitor->trace(m_additionalData);
 }
 
 } // namespace blink