Fetch: 'password' credentials mode should include credentials.

Fetch: 'password' credentials mode should include credentials.

I am not going to admit how long it took me to find this bug. Because it
was a long time and involved me installing Fiddler to verify that
chrome://net-internals wasn't lying to me because OBVIOUSLY the data
wasn't being posted to the server even though net-internals said it was
and ugh.

'password' should act like 'include' (see #2 in [1]). We should include
cookies in either mode. *sigh*

[1]: https://w3c.github.io/webappsec-credential-management/#monkey-patching

BUG=599597, 601923
R=horo@chromium.org, estark@chromium.org

Committed: https://crrev.com/a40efde9a68ead4d71421854fda2409e1cc7b053
Cr-Commit-Position: refs/heads/master@{#386212}

[Mike West, 2016-04-08 18:46:37]

mkwst@chromium.org changed reviewers:
+ estark@chromium.org

[Mike West, 2016-04-08 18:46:37]

Emily! You win the "please review this trivial patch because it's trivial and
you expressed vague interest on Twitter" award! :)

-mike

horo@: FYI.

[Mike West, 2016-04-08 18:49:25]

https://codereview.chromium.org/1868253002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/credentialmanager/passwordcredential-fetch.html
(right):

https://codereview.chromium.org/1868253002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/credentialmanager/passwordcredential-fetch.html:68:
}, "Simple Fetch");
This is the only test that has anything to do with the issue.

https://codereview.chromium.org/1868253002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/credentialmanager/passwordcredential-fetch.html:212:
}, "'additionalData': FormData properties are properly injected after
assignment.");
This is from when I thought that adding the `FormData` object to the credential
before populating it might be the issue.

https://codereview.chromium.org/1868253002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/credentialmanager/passwordcredential-fetch.html:354:
}, 'fetch() after get()');
And this is when I managed to convince myself that `get()` was somehow tainting
the credential.

https://codereview.chromium.org/1868253002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/credentialmanager/passwordcredential-fetch.html:380:
}, 'fetch() after get() with additionalData');
Maybe it was `get()` with a `FormData`?

https://codereview.chromium.org/1868253002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/credentialmanager/passwordcredential-fetch.html:405:
}, 'fetch() after get() with additionalData from DOM');
Maybe pulling data from the DOM is somehow broken?

https://codereview.chromium.org/1868253002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/credentialmanager/passwordcredential-fetch.html:406:
</script>
There were lots more. All of them equally worthless.

https://codereview.chromium.org/1868253002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/fetch/FetchManager.cpp (right):

https://codereview.chromium.org/1868253002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/fetch/FetchManager.cpp:605: ||
m_request->credentials() == WebURLRequest::FetchCredentialsModePassword
*sobs*

https://codereview.chromium.org/1868253002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/fetch/FetchManager.cpp:610: ||
m_request->credentials() == WebURLRequest::FetchCredentialsModePassword) {
*more sobs*

[Mike West, 2016-04-08 18:50:51]

Description was changed from

==========
Fetch: 'password' credentials mode should include credentials.

I am not going to admit how long it took me to find this bug. Because it
was a long time and involved me installing Fiddler to verify that
chrome://net-internals wasn't lying to me because OBVIOUSLY the data
wasn't being posted to the server even though net-internals said it was
and ugh.

'password' should act like 'include'. We should include credentials in
either mode. *sigh*

BUG=599597
TBR=horo@chromium.org
==========

to

==========
Fetch: 'password' credentials mode should include credentials.

I am not going to admit how long it took me to find this bug. Because it
was a long time and involved me installing Fiddler to verify that
chrome://net-internals wasn't lying to me because OBVIOUSLY the data
wasn't being posted to the server even though net-internals said it was
and ugh.

'password' should act like 'include' (see #2 in [1]). We should include
credentials in either mode. *sigh*

[1]: https://w3c.github.io/webappsec-credential-management/#monkey-patching

BUG=599597
TBR=horo@chromium.org
==========

[Mike West, 2016-04-08 18:51:00]

Description was changed from

==========
Fetch: 'password' credentials mode should include credentials.

I am not going to admit how long it took me to find this bug. Because it
was a long time and involved me installing Fiddler to verify that
chrome://net-internals wasn't lying to me because OBVIOUSLY the data
wasn't being posted to the server even though net-internals said it was
and ugh.

'password' should act like 'include' (see #2 in [1]). We should include
credentials in either mode. *sigh*

[1]: https://w3c.github.io/webappsec-credential-management/#monkey-patching

BUG=599597
TBR=horo@chromium.org
==========

to

==========
Fetch: 'password' credentials mode should include credentials.

I am not going to admit how long it took me to find this bug. Because it
was a long time and involved me installing Fiddler to verify that
chrome://net-internals wasn't lying to me because OBVIOUSLY the data
wasn't being posted to the server even though net-internals said it was
and ugh.

'password' should act like 'include' (see #2 in [1]). We should include
cookies in either mode. *sigh*

[1]: https://w3c.github.io/webappsec-credential-management/#monkey-patching

BUG=599597
TBR=horo@chromium.org
==========

[Mike West, 2016-04-08 18:56:50]

Description was changed from

==========
Fetch: 'password' credentials mode should include credentials.

I am not going to admit how long it took me to find this bug. Because it
was a long time and involved me installing Fiddler to verify that
chrome://net-internals wasn't lying to me because OBVIOUSLY the data
wasn't being posted to the server even though net-internals said it was
and ugh.

'password' should act like 'include' (see #2 in [1]). We should include
cookies in either mode. *sigh*

[1]: https://w3c.github.io/webappsec-credential-management/#monkey-patching

BUG=599597
TBR=horo@chromium.org
==========

to

==========
Fetch: 'password' credentials mode should include credentials.

I am not going to admit how long it took me to find this bug. Because it
was a long time and involved me installing Fiddler to verify that
chrome://net-internals wasn't lying to me because OBVIOUSLY the data
wasn't being posted to the server even though net-internals said it was
and ugh.

'password' should act like 'include' (see #2 in [1]). We should include
cookies in either mode. *sigh*

[1]: https://w3c.github.io/webappsec-credential-management/#monkey-patching

BUG=599597
R=horo@chromium.org, estark@chromium.org
==========

[estark, 2016-04-08 20:03:36]

lgtm

https://codereview.chromium.org/1868253002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/credentialmanager/passwordcredential-fetch.html
(right):

https://codereview.chromium.org/1868253002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/credentialmanager/passwordcredential-fetch.html:406:
</script>
On 2016/04/08 18:49:25, Mike West wrote:
> There were lots more. All of them equally worthless.

Aw, but on the bright side, you improved test coverage! Yay!

[Mike West, 2016-04-08 20:51:33]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2016-04-08 20:51:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1868253002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1868253002/1

[Mike West, 2016-04-08 20:58:39]

Description was changed from

==========
Fetch: 'password' credentials mode should include credentials.

I am not going to admit how long it took me to find this bug. Because it
was a long time and involved me installing Fiddler to verify that
chrome://net-internals wasn't lying to me because OBVIOUSLY the data
wasn't being posted to the server even though net-internals said it was
and ugh.

'password' should act like 'include' (see #2 in [1]). We should include
cookies in either mode. *sigh*

[1]: https://w3c.github.io/webappsec-credential-management/#monkey-patching

BUG=599597
R=horo@chromium.org, estark@chromium.org
==========

to

==========
Fetch: 'password' credentials mode should include credentials.

I am not going to admit how long it took me to find this bug. Because it
was a long time and involved me installing Fiddler to verify that
chrome://net-internals wasn't lying to me because OBVIOUSLY the data
wasn't being posted to the server even though net-internals said it was
and ugh.

'password' should act like 'include' (see #2 in [1]). We should include
cookies in either mode. *sigh*

[1]: https://w3c.github.io/webappsec-credential-management/#monkey-patching

BUG=599597,601923
R=horo@chromium.org, estark@chromium.org
==========

[commit-bot: I haz the power, 2016-04-08 21:52:07]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-04-08 21:53:27]

Description was changed from

==========
Fetch: 'password' credentials mode should include credentials.

I am not going to admit how long it took me to find this bug. Because it
was a long time and involved me installing Fiddler to verify that
chrome://net-internals wasn't lying to me because OBVIOUSLY the data
wasn't being posted to the server even though net-internals said it was
and ugh.

'password' should act like 'include' (see #2 in [1]). We should include
cookies in either mode. *sigh*

[1]: https://w3c.github.io/webappsec-credential-management/#monkey-patching

BUG=599597,601923
R=horo@chromium.org, estark@chromium.org
==========

to

==========
Fetch: 'password' credentials mode should include credentials.

I am not going to admit how long it took me to find this bug. Because it
was a long time and involved me installing Fiddler to verify that
chrome://net-internals wasn't lying to me because OBVIOUSLY the data
wasn't being posted to the server even though net-internals said it was
and ugh.

'password' should act like 'include' (see #2 in [1]). We should include
cookies in either mode. *sigh*

[1]: https://w3c.github.io/webappsec-credential-management/#monkey-patching

BUG=599597,601923
R=horo@chromium.org, estark@chromium.org

Committed: https://crrev.com/a40efde9a68ead4d71421854fda2409e1cc7b053
Cr-Commit-Position: refs/heads/master@{#386212}
==========

[commit-bot: I haz the power, 2016-04-08 21:53:28]

Patchset 1 (id:??) landed as
https://crrev.com/a40efde9a68ead4d71421854fda2409e1cc7b053
Cr-Commit-Position: refs/heads/master@{#386212}