Credential manager: Convert strings to URLs early.

Credential manager: Convert strings to URLs early.

We don't want to pass strings into the browser process for conversion
into URL objects; we should validate input when creating data from
JavaScript.

This patch adjusts {Local,Federated}Credential objects to throw exceptions
in their constructors if URL validation fails, and changes both Web*Credential
and Platform*Credential objects to accept WebURL/KURL objects rather
than strings.

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

[Mike West, 2014-08-12 09:07:18]

Jochen, would you mind stamping the platformy bits?

[Mike West, 2014-08-12 09:09:38]

Philip, would you mind reviewing this patch? It's not terribly complex: it just
replaces some String/WebString parameters with KURL/WebURL and throws exceptions
upon validation errors.

-mike

[philipj_slow, 2014-08-12 09:42:40]

LGTM, but I am powerless outside of Source/core/

https://codereview.chromium.org/464783003/diff/1/LayoutTests/http/tests/crede...
File LayoutTests/http/tests/credentialmanager/federatedcredential-basics.html
(right):

https://codereview.chromium.org/464783003/diff/1/LayoutTests/http/tests/crede...
LayoutTests/http/tests/credentialmanager/federatedcredential-basics.html:28: var
credential = new FederatedCredential('id', 'name', '-',
'https://federation.net/');
It looks like assert_throws would simplify this to assert_throws(new
SyntaxError(), function() { new FederatedCredential('id', 'name', '-',
'https://federation.net/'); });

https://codereview.chromium.org/464783003/diff/1/LayoutTests/http/tests/crede...
LayoutTests/http/tests/credentialmanager/federatedcredential-basics.html:33: },
'Invalid avatar URL throws.');
In web-platform-tests I've been advised to not put the pass condition in the
test name, but rather just say what the test does. I think it's good advice, but
it's a matter of style so take your pick :)

https://codereview.chromium.org/464783003/diff/1/Source/modules/credentialman...
File Source/modules/credentialmanager/LocalCredential.h (right):

https://codereview.chromium.org/464783003/diff/1/Source/modules/credentialman...
Source/modules/credentialmanager/LocalCredential.h:24: LocalCredential(const
String& id, const String& name, const KURL& avatarURL, const String& password);
When is the URL suffix included on the variable name? It seems a bit redundant
when the type is KURL.

[Mike West, 2014-08-12 11:32:27]

On 2014/08/12 09:42:40, philipj wrote:
> LGTM, but I am powerless outside of Source/core/

Thanks! Just trying to make things easier for Jochen, who is (like all of us)
busy. :)

[jochen (gone - plz use gerrit), 2014-08-12 14:53:30]

lgtm

[Mike West, 2014-08-12 15:02:06]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2014-08-12 15:02:50]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mkwst@chromium.org/464783003/20001

[commit-bot: I haz the power, 2014-08-12 15:26:36]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/22296)

[commit-bot: I haz the power, 2014-08-12 15:54:38]

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

[commit-bot: I haz the power, 2014-08-12 15:54:38]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/22300)

[Mike West, 2014-08-12 16:03:00]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2014-08-12 16:03:59]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mkwst@chromium.org/464783003/20001

[commit-bot: I haz the power, 2014-08-12 17:41:28]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/22300)

[commit-bot: I haz the power, 2014-08-12 18:10:09]

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

[commit-bot: I haz the power, 2014-08-12 18:10:10]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/22322)

[Mike West, 2014-08-12 18:15:14]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2014-08-12 18:17:39]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mkwst@chromium.org/464783003/20001

[commit-bot: I haz the power, 2014-08-12 18:51:05]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/22334)

[commit-bot: I haz the power, 2014-08-12 19:23:11]

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

[commit-bot: I haz the power, 2014-08-12 19:23:13]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/22340)

[Mike West, 2014-08-12 22:57:20]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2014-08-12 22:58:27]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mkwst@chromium.org/464783003/20001

[commit-bot: I haz the power, 2014-08-12 23:31:24]

Change committed as 180120