CREDENTIAL: Extend FormData opacity to created Request objects

CREDENTIAL: Extend FormData opacity to created Request objects

Opaque FormData objects created from 'PasswordCredential::toFormData()'
need to persist their opacity through to Request objects, as defined in
https://w3c.github.io/webappsec/specs/credentialmanagement/#opaque-formdata-algorithms

This patch adjusts 'RequestInit', 'Body', and 'Request' such that opaque
'FormData' bodies lead to opaque 'Body' objects that reject read
attempts from JavaScript.

BUG=526995
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=201714

[Mike West, 2015-09-02 10:41:38]

mkwst@chromium.org changed reviewers:
+ philipj@opera.com

[Mike West, 2015-09-02 10:41:53]

One more patch in the series for you to skim, Philip.

[philipj_slow, 2015-09-02 12:12:48]

Lightly skimmed, LGTM with test nits :)

https://codereview.chromium.org/1305183009/diff/1/LayoutTests/http/tests/cred...
File LayoutTests/http/tests/credentialmanager/passwordcredential-basics.html
(right):

https://codereview.chromium.org/1305183009/diff/1/LayoutTests/http/tests/cred...
LayoutTests/http/tests/credentialmanager/passwordcredential-basics.html:80:
(function() {
I guess this is consistent with other tests around here, but a function scope
isn't needed if you use async_test(function() { ... }, "test name");

https://codereview.chromium.org/1305183009/diff/1/LayoutTests/http/tests/cred...
LayoutTests/http/tests/credentialmanager/passwordcredential-basics.html:95:
t.step_func(function (data) {
t.unreached_func("The arrayBuffer() method should reject.")

https://codereview.chromium.org/1305183009/diff/1/LayoutTests/http/tests/cred...
LayoutTests/http/tests/credentialmanager/passwordcredential-basics.html:98:
t.step_func(function (e) {
Can use step_func_done if you like, it doesn't make much difference here, but
it's nice when the function only calls done().

https://codereview.chromium.org/1305183009/diff/1/LayoutTests/http/tests/cred...
LayoutTests/http/tests/credentialmanager/passwordcredential-basics.html:105:
r.blob().then(
It looks like you could share all of the duplicated code with a function
test_something(methodName), to have one test per case, and just use
r[methodName]() around here.

https://codereview.chromium.org/1305183009/diff/1/Source/modules/fetch/Body.cpp
File Source/modules/fetch/Body.cpp (right):

https://codereview.chromium.org/1305183009/diff/1/Source/modules/fetch/Body.c...
Source/modules/fetch/Body.cpp:137: if (m_opaque)
The monkey patching says that this step should happen first, before everything
required by the Fetch spec. Not sure if it's possible to observe the difference,
though.

[Mike West, 2015-09-02 13:16:58]

The CQ bit was checked by mkwst@chromium.org

[Mike West, 2015-09-02 13:16:58]

The patchset sent to the CQ was uploaded after l-g-t-m from philipj@opera.com
Link to the patchset: https://codereview.chromium.org/1305183009/#ps20001
(title: "Testing nits.")

[commit-bot: I haz the power, 2015-09-02 13:17:08]

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

[Mike West, 2015-09-02 13:17:41]

Thanks! The testing nits were helpful. :)

https://codereview.chromium.org/1305183009/diff/1/Source/modules/fetch/Body.cpp
File Source/modules/fetch/Body.cpp (right):

https://codereview.chromium.org/1305183009/diff/1/Source/modules/fetch/Body.c...
Source/modules/fetch/Body.cpp:137: if (m_opaque)
On 2015/09/02 at 12:12:48, philipj wrote:
> The monkey patching says that this step should happen first, before everything
required by the Fetch spec. Not sure if it's possible to observe the difference,
though.

That's fair. We should do this before the `bodyUsed()` check. You'll get a
TypeError either way, but I guess you should get this message first. :)

[Mike West, 2015-09-02 13:17:42]

Thanks! The testing nits were helpful. :)

https://codereview.chromium.org/1305183009/diff/1/Source/modules/fetch/Body.cpp
File Source/modules/fetch/Body.cpp (right):

https://codereview.chromium.org/1305183009/diff/1/Source/modules/fetch/Body.c...
Source/modules/fetch/Body.cpp:137: if (m_opaque)
On 2015/09/02 at 12:12:48, philipj wrote:
> The monkey patching says that this step should happen first, before everything
required by the Fetch spec. Not sure if it's possible to observe the difference,
though.

That's fair. We should do this before the `bodyUsed()` check. You'll get a
TypeError either way, but I guess you should get this message first. :)

[philipj_slow, 2015-09-02 13:42:10]

https://codereview.chromium.org/1305183009/diff/20001/LayoutTests/http/tests/...
File LayoutTests/http/tests/credentialmanager/passwordcredential-basics.html
(right):

https://codereview.chromium.org/1305183009/diff/20001/LayoutTests/http/tests/...
LayoutTests/http/tests/credentialmanager/passwordcredential-basics.html:103: },
"Verify behavior of 'Body::" + methodName + "()' for opaque Requests.");
Very C++y that :: ;)

[commit-bot: I haz the power, 2015-09-02 14:14:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-09-02 14:14:26]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Mike West, 2015-09-03 08:17:12]

The CQ bit was checked by mkwst@chromium.org

[Mike West, 2015-09-03 08:17:12]

The patchset sent to the CQ was uploaded after l-g-t-m from philipj@opera.com
Link to the patchset: https://codereview.chromium.org/1305183009/#ps40001
(title: "Initializing variables is a good idea.")

[commit-bot: I haz the power, 2015-09-03 08:17:22]

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

[commit-bot: I haz the power, 2015-09-03 09:39:57]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=201714