Custom handlers should throw SecurityError exception if the URL's origin differs from the document'…

Custom handlers should throw SecurityError exception if the URL's origin differs from the document's origin.

Secification: http://www.whatwg.org/specs/web-apps/current-work/#custom-handlers
"User agents must throw a SecurityError exception if the resulting absolute URL has an origin that differs from the origin specified by the entry settings object."

BUG=399184, 406236
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=182527

[pals, 2014-07-16 06:00:44]

PTAL.

[gyuyoung-inactive, 2014-07-16 06:20:17]

Yes, we should check whether URL's origin is same with document's one. Patch
looks fine to me except for trivial suggestion.

https://codereview.chromium.org/392993005/diff/1/LayoutTests/navigatorcontent...
File LayoutTests/navigatorcontentutils/register-protocol-handler.html (right):

https://codereview.chromium.org/392993005/diff/1/LayoutTests/navigatorcontent...
LayoutTests/navigatorcontentutils/register-protocol-handler.html:98: // Test
that the API throws SecurityError exception if the URL's origin differs from the
document's origin.
I think it would be nicer to move this test to invalid url tests zone. Probably
84 line ?

https://codereview.chromium.org/392993005/diff/1/Source/modules/navigatorcont...
File Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp (right):

https://codereview.chromium.org/392993005/diff/1/Source/modules/navigatorcont...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:97:
exceptionState.throwSecurityError("Can only register handler in the document's
origin.");
Isn't it better mention "custom handler" instead of just "handler" ?

[abarth-chromium, 2014-07-16 16:32:40]

https://codereview.chromium.org/392993005/diff/1/Source/modules/navigatorcont...
File Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp (right):

https://codereview.chromium.org/392993005/diff/1/Source/modules/navigatorcont...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:96: if
(!origin->isSameSchemeHostPort(document.securityOrigin())) {
You don't really ever want to call isSameSchemeHostPort.  How about:

document.securityOrigin()->canRequest(kurl)

?

https://codereview.chromium.org/392993005/diff/1/Source/modules/navigatorcont...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:163: if
(!verifyCustomHandlerURL(*document, baseURL, url, exceptionState))
Why don't we just pass in the complete URL to verifyCustomHandlerURL?

[gyuyoung-inactive, 2014-07-17 07:01:19]

https://codereview.chromium.org/392993005/diff/1/Source/modules/navigatorcont...
File Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp (right):

https://codereview.chromium.org/392993005/diff/1/Source/modules/navigatorcont...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:163: if
(!verifyCustomHandlerURL(*document, baseURL, url, exceptionState))
On 2014/07/16 16:32:39, abarth wrote:
> Why don't we just pass in the complete URL to verifyCustomHandlerURL?

We are passing baseURL and registered url to client side. So, if we complete to
merge two urls here, we can pass the complete url to client side. If so, it
would be good to handle this suggestion with new CL.

However, if we should pass two urls to client side continually, I don't know
what is benefit to merge the two urls here.

[pals, 2014-07-18 14:17:54]

Fixed review comments. Please have another look.

https://codereview.chromium.org/392993005/diff/1/LayoutTests/navigatorcontent...
File LayoutTests/navigatorcontentutils/register-protocol-handler.html (right):

https://codereview.chromium.org/392993005/diff/1/LayoutTests/navigatorcontent...
LayoutTests/navigatorcontentutils/register-protocol-handler.html:98: // Test
that the API throws SecurityError exception if the URL's origin differs from the
document's origin.
On 2014/07/16 06:20:17, gyuyoung wrote:
> I think it would be nicer to move this test to invalid url tests zone.
Probably
> 84 line ?

Done.

https://codereview.chromium.org/392993005/diff/1/Source/modules/navigatorcont...
File Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp (right):

https://codereview.chromium.org/392993005/diff/1/Source/modules/navigatorcont...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:96: if
(!origin->isSameSchemeHostPort(document.securityOrigin())) {
On 2014/07/16 16:32:39, abarth wrote:
> You don't really ever want to call isSameSchemeHostPort.  How about:
> 
> document.securityOrigin()->canRequest(kurl)
> 
> ?

Done.

https://codereview.chromium.org/392993005/diff/1/Source/modules/navigatorcont...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:97:
exceptionState.throwSecurityError("Can only register handler in the document's
origin.");
On 2014/07/16 06:20:17, gyuyoung wrote:
> Isn't it better mention "custom handler" instead of just "handler" ?

Done.

https://codereview.chromium.org/392993005/diff/1/Source/modules/navigatorcont...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:163: if
(!verifyCustomHandlerURL(*document, baseURL, url, exceptionState))
On 2014/07/16 16:32:39, abarth wrote:
> Why don't we just pass in the complete URL to verifyCustomHandlerURL?

Done.

[gyuyoung-inactive, 2014-07-21 00:30:21]

https://codereview.chromium.org/392993005/diff/60001/Source/modules/navigator...
File Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp (right):

https://codereview.chromium.org/392993005/diff/60001/Source/modules/navigator...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:168: KURL baseURL
= document->baseURL();
Should we use "baseURL" local variable ? This local variable is only used just
one time. I prefer not to use local variable if it is not necessary.

https://codereview.chromium.org/392993005/diff/60001/Source/modules/navigator...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:229: KURL baseURL
= document->baseURL();
ditto.

[pals, 2014-07-21 10:24:36]

On 2014/07/16 16:32:40, abarth wrote:
>
https://codereview.chromium.org/392993005/diff/1/Source/modules/navigatorcont...
> File Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp (right):
> 
>
https://codereview.chromium.org/392993005/diff/1/Source/modules/navigatorcont...
> Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:96: if
> (!origin->isSameSchemeHostPort(document.securityOrigin())) {
> You don't really ever want to call isSameSchemeHostPort.  How about:
> 
> document.securityOrigin()->canRequest(kurl)
> 
> ?

If I use document.securityOrigin()->canRequest(kurl),
As m_universalAccess is set to true for content_shell, canRequest() always
returns true.

I tried to set it to false from the test
[testRunner.setAllowUniversalAccessFromFileURLs(false);]
but by that time the document's origin already initialized.
Is there anything I am missing in the test.

[pals, 2014-07-22 09:18:06]

As I explained in my previous comments,
testRunner.setAllowUniversalAccessFromFileURLs(false) does not actually modify
m_universalAccess in SecurityOrigin. I have made changes to update document's
security origin when allowUniversalAccessFromFileURLs is changed settings.
Please review the changes.

[gyuyoung-inactive, 2014-07-23 00:07:13]

https://codereview.chromium.org/392993005/diff/100001/LayoutTests/navigatorco...
File LayoutTests/navigatorcontentutils/register-protocol-handler.html (right):

https://codereview.chromium.org/392993005/diff/100001/LayoutTests/navigatorco...
LayoutTests/navigatorcontentutils/register-protocol-handler.html:98: debug('PASS
Invalid url threw SecurityError exception: "' + errorMessage + '".');
Isn't this test for different origin ? It looks "Invalid url" needs to be
changed with "URL with different origin threw SecurityError..."

https://codereview.chromium.org/392993005/diff/100001/LayoutTests/navigatorco...
File LayoutTests/navigatorcontentutils/unregister-protocol-handler.html (right):

https://codereview.chromium.org/392993005/diff/100001/LayoutTests/navigatorco...
LayoutTests/navigatorcontentutils/unregister-protocol-handler.html:97:
debug('PASS Invalid url threw SecurityError exception: "' + errorMessage +
'".');
ditto.

[pals, 2014-07-23 05:39:29]

Fixed.

https://codereview.chromium.org/392993005/diff/100001/LayoutTests/navigatorco...
File LayoutTests/navigatorcontentutils/register-protocol-handler.html (right):

https://codereview.chromium.org/392993005/diff/100001/LayoutTests/navigatorco...
LayoutTests/navigatorcontentutils/register-protocol-handler.html:98: debug('PASS
Invalid url threw SecurityError exception: "' + errorMessage + '".');
On 2014/07/23 00:07:12, gyuyoung wrote:
> Isn't this test for different origin ? It looks "Invalid url" needs to be
> changed with "URL with different origin threw SecurityError..."

Good catch. Done.

https://codereview.chromium.org/392993005/diff/100001/LayoutTests/navigatorco...
File LayoutTests/navigatorcontentutils/unregister-protocol-handler.html (right):

https://codereview.chromium.org/392993005/diff/100001/LayoutTests/navigatorco...
LayoutTests/navigatorcontentutils/unregister-protocol-handler.html:97:
debug('PASS Invalid url threw SecurityError exception: "' + errorMessage +
'".');
On 2014/07/23 00:07:12, gyuyoung wrote:
> ditto.

Done.

[gyuyoung-inactive, 2014-07-23 06:51:24]

https://codereview.chromium.org/392993005/diff/120001/LayoutTests/navigatorco...
File
LayoutTests/navigatorcontentutils/is-protocol-handler-registered-expected.txt
(right):

https://codereview.chromium.org/392993005/diff/120001/LayoutTests/navigatorco...
LayoutTests/navigatorcontentutils/is-protocol-handler-registered-expected.txt:9:
PASS Invalid url threw SecurityError exception: "Failed to execute
'isProtocolHandlerRegistered' on 'Navigator': Can only register custom handler
in the document's origin.".
It looks all *-expected.txt are not updated yet.

[pals, 2014-07-23 06:55:41]

On 2014/07/23 06:51:24, gyuyoung wrote:
>
https://codereview.chromium.org/392993005/diff/120001/LayoutTests/navigatorco...
> File
> LayoutTests/navigatorcontentutils/is-protocol-handler-registered-expected.txt
> (right):
> 
>
https://codereview.chromium.org/392993005/diff/120001/LayoutTests/navigatorco...
>
LayoutTests/navigatorcontentutils/is-protocol-handler-registered-expected.txt:9:
> PASS Invalid url threw SecurityError exception: "Failed to execute
> 'isProtocolHandlerRegistered' on 'Navigator': Can only register custom handler
> in the document's origin.".
> It looks all *-expected.txt are not updated yet.

Rebased *-expected.txt.

[gyuyoung-inactive, 2014-07-23 06:59:52]

Looks good to me on navigator content utils side. However, I'm not sure whether
this cl can modify SecurityOrigin together.

[pals, 2014-07-31 06:51:21]

On 2014/07/23 06:59:52, gyuyoung wrote:
> Looks good to me on navigator content utils side. However, I'm not sure
whether
> this cl can modify SecurityOrigin together.

Last patch set also fixes
http://code.google.com/p/chromium/issues/detail?id=399184. Sould I create a
separate CL for the same.
Here is the content side CL https://codereview.chromium.org/428253006/.

[gyuyoung-inactive, 2014-07-31 06:56:51]

On 2014/07/31 06:51:21, sanjoy.pal wrote:
> On 2014/07/23 06:59:52, gyuyoung wrote:
> > Looks good to me on navigator content utils side. However, I'm not sure
> whether
> > this cl can modify SecurityOrigin together.
> 
> Last patch set also fixes
> http://code.google.com/p/chromium/issues/detail?id=399184. Sould I create a
> separate CL for the same.
> Here is the content side CL https://codereview.chromium.org/428253006/.

This CL changes functions of SecurityOrigin. So, I think the area should be
reviewed by the SecurityOrigin owner as well. If he want to separate this CL,
IMHO, you need to do.
If not, you don't need. :)

[pals, 2014-08-01 09:48:31]

On 2014/07/31 06:56:51, gyuyoung wrote:
> On 2014/07/31 06:51:21, sanjoy.pal wrote:
> > On 2014/07/23 06:59:52, gyuyoung wrote:
> > > Looks good to me on navigator content utils side. However, I'm not sure
> > whether
> > > this cl can modify SecurityOrigin together.
> > 
> > Last patch set also fixes
> > http://code.google.com/p/chromium/issues/detail?id=399184. Sould I create a
> > separate CL for the same.
> > Here is the content side CL https://codereview.chromium.org/428253006/.
> 
> This CL changes functions of SecurityOrigin. So, I think the area should be
> reviewed by the SecurityOrigin owner as well. If he want to separate this CL,
> IMHO, you need to do.
> If not, you don't need. :)

I shall submit a separate CL for the last patch-set to fix
http://code.google.com/p/chromium/issues/detail?id=399184.

[pals, 2014-08-06 13:20:32]

On a second thought, I think this CL mostly moves the security origin check from
content to blink side. So, it should land as a single CL.
Please review the change and the dependent CL
https://codereview.chromium.org/428253006/.

[abarth-chromium, 2014-08-06 18:34:56]

https://codereview.chromium.org/392993005/diff/160001/LayoutTests/navigatorco...
File LayoutTests/navigatorcontentutils/register-protocol-handler.html (right):

https://codereview.chromium.org/392993005/diff/160001/LayoutTests/navigatorco...
LayoutTests/navigatorcontentutils/register-protocol-handler.html:10:
testRunner.setAllowUniversalAccessFromFileURLs(false);
This is impossible.  Instead, you need to put this test in the http/tests
directory.  There it will run with an HTTP URL and hence not have universal
access.

https://codereview.chromium.org/392993005/diff/160001/Source/core/dom/Documen...
File Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/392993005/diff/160001/Source/core/dom/Documen...
Source/core/dom/Document.cpp:4806: securityOrigin()->setUniversalAccess(true);
This doesn't make sense.  There's no way to revoke universal access.  That's a
technical impossibility.

https://codereview.chromium.org/392993005/diff/160001/Source/core/frame/Setti...
File Source/core/frame/Settings.in (right):

https://codereview.chromium.org/392993005/diff/160001/Source/core/frame/Setti...
Source/core/frame/Settings.in:62: allowUniversalAccessFromFileURLs initial=true,
invalidate=UniversalAccessFromFileURLs
This setting cannot be changed dynamically, so there's no reason to have it
invalidate anything.  It's impossible to invalidate all the state that would
need to be invalidated for this to work correctly.

https://codereview.chromium.org/392993005/diff/160001/public/web/WebViewClient.h
File public/web/WebViewClient.h (right):

https://codereview.chromium.org/392993005/diff/160001/public/web/WebViewClien...
public/web/WebViewClient.h:274: }
Is Chromium prepared for this change or will this break these calls?

[pals, 2014-08-06 18:46:47]

https://codereview.chromium.org/392993005/diff/160001/LayoutTests/navigatorco...
File LayoutTests/navigatorcontentutils/register-protocol-handler.html (right):

https://codereview.chromium.org/392993005/diff/160001/LayoutTests/navigatorco...
LayoutTests/navigatorcontentutils/register-protocol-handler.html:10:
testRunner.setAllowUniversalAccessFromFileURLs(false);
On 2014/08/06 18:34:55, abarth wrote:
> This is impossible.  Instead, you need to put this test in the http/tests
> directory.  There it will run with an HTTP URL and hence not have universal
> access.

I shall move it to http/tests.

https://codereview.chromium.org/392993005/diff/160001/public/web/WebViewClient.h
File public/web/WebViewClient.h (right):

https://codereview.chromium.org/392993005/diff/160001/public/web/WebViewClien...
public/web/WebViewClient.h:274: }
On 2014/08/06 18:34:56, abarth wrote:
> Is Chromium prepared for this change or will this break these calls?
This is the chromium side CL
https://codereview.chromium.org/428253006/

[pals, 2014-08-07 14:06:06]

Please take a look.

https://codereview.chromium.org/392993005/diff/160001/LayoutTests/navigatorco...
File LayoutTests/navigatorcontentutils/register-protocol-handler.html (right):

https://codereview.chromium.org/392993005/diff/160001/LayoutTests/navigatorco...
LayoutTests/navigatorcontentutils/register-protocol-handler.html:10:
testRunner.setAllowUniversalAccessFromFileURLs(false);
On 2014/08/06 18:34:55, abarth wrote:
> This is impossible.  Instead, you need to put this test in the http/tests
> directory.  There it will run with an HTTP URL and hence not have universal
> access.

Done.

https://codereview.chromium.org/392993005/diff/160001/Source/core/dom/Documen...
File Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/392993005/diff/160001/Source/core/dom/Documen...
Source/core/dom/Document.cpp:4806: securityOrigin()->setUniversalAccess(true);
On 2014/08/06 18:34:55, abarth wrote:
> This doesn't make sense.  There's no way to revoke universal access.  That's a
> technical impossibility.
This change is not required anymore. Removed.

https://codereview.chromium.org/392993005/diff/160001/Source/core/frame/Setti...
File Source/core/frame/Settings.in (right):

https://codereview.chromium.org/392993005/diff/160001/Source/core/frame/Setti...
Source/core/frame/Settings.in:62: allowUniversalAccessFromFileURLs initial=true,
invalidate=UniversalAccessFromFileURLs
On 2014/08/06 18:34:55, abarth wrote:
> This setting cannot be changed dynamically, so there's no reason to have it
> invalidate anything.  It's impossible to invalidate all the state that would
> need to be invalidated for this to work correctly.
This change is not required anymore. Removed.

https://codereview.chromium.org/392993005/diff/160001/public/web/WebViewClient.h
File public/web/WebViewClient.h (right):

https://codereview.chromium.org/392993005/diff/160001/public/web/WebViewClien...
public/web/WebViewClient.h:274: }
On 2014/08/06 18:46:47, sanjoy.pal wrote:
> On 2014/08/06 18:34:56, abarth wrote:
> > Is Chromium prepared for this change or will this break these calls?
> This is the chromium side CL
> https://codereview.chromium.org/428253006/

Done.

[pals, 2014-08-20 13:41:16]

Friendly ping.

[abarth-chromium, 2014-08-20 18:15:10]

https://codereview.chromium.org/392993005/diff/200001/Source/modules/navigato...
File Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp (right):

https://codereview.chromium.org/392993005/diff/200001/Source/modules/navigato...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:88: KURL
kurl(baseURL, newURL);
document.completeURL(newURL)

https://codereview.chromium.org/392993005/diff/200001/Source/modules/navigato...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:170: KURL
absoluteURL(baseURL, url);
document->completeURL(...)

https://codereview.chromium.org/392993005/diff/200001/Source/modules/navigato...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:213: KURL
absoluteURL(baseURL, url);
ditto

https://codereview.chromium.org/392993005/diff/200001/Source/modules/navigato...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:233: KURL
absoluteURL(baseURL, url);
ditto

https://codereview.chromium.org/392993005/diff/200001/public/web/WebViewClient.h
File public/web/WebViewClient.h (right):

https://codereview.chromium.org/392993005/diff/200001/public/web/WebViewClien...
public/web/WebViewClient.h:274: }
Is Chromium prepared to handle this API change or will this break the feature?

[pals, 2014-08-21 14:02:56]

I am not sure what is the best way to land this change. I have created a CL to
prepare chromium to land this CL, https://codereview.chromium.org/485103005/. I
shall create a follow up CL to remove the overridden methods. This will need an
extra commit.

I have also created this CL https://codereview.chromium.org/428253006/ if we can
somehow land both the CLs bypassing the trybots.

https://codereview.chromium.org/392993005/diff/200001/Source/modules/navigato...
File Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp (right):

https://codereview.chromium.org/392993005/diff/200001/Source/modules/navigato...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:88: KURL
kurl(baseURL, newURL);
On 2014/08/20 18:15:10, abarth wrote:
> document.completeURL(newURL)

Done.

https://codereview.chromium.org/392993005/diff/200001/Source/modules/navigato...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:170: KURL
absoluteURL(baseURL, url);
On 2014/08/20 18:15:10, abarth wrote:
> document->completeURL(...)

Done.

https://codereview.chromium.org/392993005/diff/200001/Source/modules/navigato...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:213: KURL
absoluteURL(baseURL, url);
On 2014/08/20 18:15:10, abarth wrote:
> ditto

Done.

https://codereview.chromium.org/392993005/diff/200001/Source/modules/navigato...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:233: KURL
absoluteURL(baseURL, url);
On 2014/08/20 18:15:10, abarth wrote:
> ditto

Done.

https://codereview.chromium.org/392993005/diff/200001/public/web/WebViewClient.h
File public/web/WebViewClient.h (right):

https://codereview.chromium.org/392993005/diff/200001/public/web/WebViewClien...
public/web/WebViewClient.h:274: }
On 2014/08/20 18:15:10, abarth wrote:
> Is Chromium prepared to handle this API change or will this break the feature?
I am not sure what is the best way to land this change. I have created a CL to
prepare chromium to land this CL, https://codereview.chromium.org/485103005/. I
shall create a follow up CL to remove the overridden methods. This will need an
extra commit.

I have also created this CL https://codereview.chromium.org/428253006/ if we can
somehow land both the CLs bypassing the trybots.

[pals, 2014-08-26 10:33:36]

Trybots are green. PTAL.

[pals, 2014-09-01 12:58:32]

Chormium side changes landed here https://codereview.chromium.org/485103005/.
Please review.

[abarth-chromium, 2014-09-08 15:31:30]

abarth@chromium.org changed reviewers:
- abarth@chromium.org

[pals, 2014-09-16 13:13:09]

sanjoy.pal@samsung.com changed reviewers:
+ eseidel@chromium.org

[pals, 2014-09-16 13:18:46]

sanjoy.pal@samsung.com changed reviewers:
+ haraken@chromium.org
- eseidel@chromium.org

[pals, 2014-09-16 13:20:55]

On 2014/09/01 12:58:32, sanjoy_pal wrote:
> Chormium side changes landed here https://codereview.chromium.org/485103005/.
> Please review.

abarth@ was reviewing this CL. Last I checked with him, he is not having enough
bandwidth to review this CL now.

@haraken, could please help me to land this CL?

[gyuyoung-inactive, 2014-09-17 01:37:13]

On 2014/09/16 13:20:55, sanjoy_pal wrote:
> On 2014/09/01 12:58:32, sanjoy_pal wrote:
> > Chormium side changes landed here
https://codereview.chromium.org/485103005/.
> > Please review.
> 
> abarth@ was reviewing this CL. Last I checked with him, he is not having
enough
> bandwidth to review this CL now.
> 
> @haraken, could please help me to land this CL?

[gyuyoung-inactive, 2014-09-17 01:37:22]

On 2014/09/17 01:37:13, gyuyoung wrote:
> On 2014/09/16 13:20:55, sanjoy_pal wrote:
> > On 2014/09/01 12:58:32, sanjoy_pal wrote:
> > > Chormium side changes landed here
> https://codereview.chromium.org/485103005/.
> > > Please review.
> > 
> > abarth@ was reviewing this CL. Last I checked with him, he is not having
> enough
> > bandwidth to review this CL now.
> > 
> > @haraken, could please help me to land this CL?

[gyuyoung-inactive, 2014-09-17 01:38:18]

On 2014/09/16 13:20:55, sanjoy_pal wrote:
> On 2014/09/01 12:58:32, sanjoy_pal wrote:
> > Chormium side changes landed here
https://codereview.chromium.org/485103005/.
> > Please review.
> 
> abarth@ was reviewing this CL. Last I checked with him, he is not having
enough
> bandwidth to review this CL now.
> 
> @haraken, could please help me to land this CL?

Sorry for noise comments. Anyway, unofficial lgtm.

[haraken, 2014-09-22 05:44:52]

The change looks good.

Does the new behavior align with other browsers?

[haraken, 2014-09-22 05:51:10]

On 2014/09/22 05:44:52, haraken wrote:
> The change looks good.
> 
> Does the new behavior align with other browsers?

Chatted offline. The new behavior aligns with Firefox and the spec. IE doesn't
support it.

LGTM. (I'm not an OWNER of web/ and public/.)

[pals, 2014-09-22 05:53:34]

sanjoy.pal@samsung.com changed reviewers:
+ tkent@chromium.org

[pals, 2014-09-22 05:54:27]

On 2014/09/22 05:51:10, haraken wrote:
> On 2014/09/22 05:44:52, haraken wrote:
> > The change looks good.
> > 
> > Does the new behavior align with other browsers?
> 
> Chatted offline. The new behavior aligns with Firefox and the spec. IE doesn't
> support it.
> 
> LGTM. (I'm not an OWNER of web/ and public/.)

+tkent for web/ and public/.
Ptal.

[tkent, 2014-09-22 09:53:15]

https://codereview.chromium.org/392993005/diff/260001/Source/modules/navigato...
File Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp (right):

https://codereview.chromium.org/392993005/diff/260001/Source/modules/navigato...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:87: KURL baseURL
= document.baseURL();
This variable looks unnecessary.
The following |if| block uses |baseURL|, but we can replace it with
document.baseURL().

https://codereview.chromium.org/392993005/diff/260001/Source/modules/navigato...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:95: // The
specification says that the API throws SecurityError exception if the URL's
origin differs from the document's origin.
nit: I recommend to wrap code comments in 80 columns because of readability in
codereview.chromium.org.

https://codereview.chromium.org/392993005/diff/260001/Source/modules/navigato...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:169: KURL baseURL
= document->baseURL();
This variable is not used.

https://codereview.chromium.org/392993005/diff/260001/Source/modules/navigato...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:211: KURL baseURL
= document->baseURL();
This variable is not used.

https://codereview.chromium.org/392993005/diff/260001/Source/modules/navigato...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:230: KURL baseURL
= document->baseURL();
Ditto.

https://codereview.chromium.org/392993005/diff/260001/Source/modules/navigato...
File
Source/modules/navigatorcontentutils/testing/NavigatorContentUtilsClientMock.cpp
(right):

https://codereview.chromium.org/392993005/diff/260001/Source/modules/navigato...
Source/modules/navigatorcontentutils/testing/NavigatorContentUtilsClientMock.cpp:26:
const blink::KURL& url)
blink:: is not needed.

[pals, 2014-09-22 11:37:53]

Fixed the issues. PTAL.

https://codereview.chromium.org/392993005/diff/260001/Source/modules/navigato...
File Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp (right):

https://codereview.chromium.org/392993005/diff/260001/Source/modules/navigato...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:87: KURL baseURL
= document.baseURL();
On 2014/09/22 09:53:14, tkent (high review load) wrote:
> This variable looks unnecessary.
> The following |if| block uses |baseURL|, but we can replace it with
> document.baseURL().

Done.

https://codereview.chromium.org/392993005/diff/260001/Source/modules/navigato...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:95: // The
specification says that the API throws SecurityError exception if the URL's
origin differs from the document's origin.
On 2014/09/22 09:53:14, tkent (high review load) wrote:
> nit: I recommend to wrap code comments in 80 columns because of readability in
> http://codereview.chromium.org.

Done.

https://codereview.chromium.org/392993005/diff/260001/Source/modules/navigato...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:169: KURL baseURL
= document->baseURL();
On 2014/09/22 09:53:14, tkent (high review load) wrote:
> This variable is not used.

Done.

https://codereview.chromium.org/392993005/diff/260001/Source/modules/navigato...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:211: KURL baseURL
= document->baseURL();
On 2014/09/22 09:53:14, tkent (high review load) wrote:
> This variable is not used.

Done.

https://codereview.chromium.org/392993005/diff/260001/Source/modules/navigato...
Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp:230: KURL baseURL
= document->baseURL();
On 2014/09/22 09:53:14, tkent (high review load) wrote:
> Ditto.

Done.

https://codereview.chromium.org/392993005/diff/260001/Source/modules/navigato...
File
Source/modules/navigatorcontentutils/testing/NavigatorContentUtilsClientMock.cpp
(right):

https://codereview.chromium.org/392993005/diff/260001/Source/modules/navigato...
Source/modules/navigatorcontentutils/testing/NavigatorContentUtilsClientMock.cpp:26:
const blink::KURL& url)
On 2014/09/22 09:53:15, tkent (high review load) wrote:
> blink:: is not needed.

Done.

[tkent, 2014-09-23 22:45:08]

The CQ bit was checked by tkent@chromium.org

[tkent, 2014-09-23 22:45:09]

lgtm

[commit-bot: I haz the power, 2014-09-23 22:46:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/392993005/280001

[commit-bot: I haz the power, 2014-09-23 23:54:28]

Committed patchset #10 (id:280001) as 182527