Fix effective Origin URLs for IDB + Cache for file:/// pages

Fix effective Origin URLs for IDB + Cache for file:/// pages

This is a terrible, horrible, no good, very bad hack.

Previously, storage APIs (notable, Indexed DB, Cache Storage) would
pass the origins they operated on from Blink to Chromium's
content/browser as a serialization done through
SecurityOrigin->DatabaseIdentifier->String->IPC->String->GURL. That
was simplified to SecurityOrigin->String->GURL->IPC. A side effect of
this was that pages browsed as file:// URLs would end up as invalid
GURLs due to stricter checks in the SecurityOrigin->String step.
Special case that conversion to restore the previous behavior.

R=michaeln@chromium.org,mkwst@chromium.org
BUG=595840
Committed: https://crrev.com/5721760f19e829770503b2aa4e51ff76848f2270
Cr-Commit-Position: refs/heads/master@{#382587}

[jsbell, 2016-03-17 22:11:18]

I hate this a lot, but we gotta start somewhere.

Blink's DatabaseIdentifier doesn't know about the nuances of SecurityOrigin, so
it just concatenates protocol/host/port:

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

This effectively restores that behavior for a special case. I can think of
several slightly less terrible ways, e.g. exposing isLocal() in
WebSecurityOrigin, placing this near blink::WebStringToGURL (with big giant
warning signs and requiring a contract signed in blood), etc.

[michaeln, 2016-03-17 22:52:41]

lgtm, special casing for file urls is certainly not new :(

i'm not sure about some of the notions behind the nuances, it says "the less you
know about squirrely origins the less harm you can do", which may be true, but
it also constrains how much good you can do?

[jsbell, 2016-03-18 22:22:53]

Yay, this approach also solves the issue I was seeing over on
https://codereview.chromium.org/1779413002/ - which is what made me see if there
was a lurking bug in the first place.

[Mike West, 2016-03-21 08:28:34]

On 2016/03/17 at 22:11:18, jsbell wrote:
> This effectively restores that behavior for a special case. I can think of
several slightly less terrible ways, e.g. exposing isLocal() in
WebSecurityOrigin

This wouldn't be enough, as we don't special case all "local" protocols, just
`file:`. (See `SchemeRegistry::registerURLSchemeAsLocal`, which I think is only
used for ChromeOS/Android weirdnesses).

> placing this near blink::WebStringToGURL (with big giant warning signs and
requiring a contract signed in blood), etc.

Eh. I don't think that would help much. This seems like a reasonable approach to
a weird thing. LGTM.

[jsbell, 2016-03-21 16:23:55]

Thanks. 

Since I need this logic for WebSQL as well I'm going to try and find a common
place to stash this.

[jsbell, 2016-03-21 23:15:16]

jsbell@chromium.org changed reviewers:
+ jam@chromium.org

[jsbell, 2016-03-21 23:15:16]

jam@ - can you take a look at the addition of the new file and change to
content/renderer/renderer_blink_platform_impl ? (so basically the whole
thing...)

[jam, 2016-03-22 15:35:16]

lgtm

btw what's a good way to get from origin to GURL? i.e. see
GURL(origin_.Serialize()) in
https://codereview.chromium.org/1814003002/diff/60001/content/renderer/dom_st...

[jsbell, 2016-03-22 16:06:35]

The CQ bit was checked by jsbell@chromium.org

[jsbell, 2016-03-22 16:06:35]

The patchset sent to the CQ was uploaded after l-g-t-m from
michaeln@chromium.org, mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/1810193002/#ps20001
(title: "Factor out origin->GURL conversion helper")

[commit-bot: I haz the power, 2016-03-22 16:06:44]

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

[jsbell, 2016-03-22 16:07:52]

The CQ bit was unchecked by jsbell@chromium.org

[jsbell, 2016-03-22 16:08:55]

The CQ bit was checked by jsbell@chromium.org

[commit-bot: I haz the power, 2016-03-22 16:09:11]

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

[jsbell, 2016-03-22 16:09:59]

On 2016/03/22 15:35:16, jam wrote:
> btw what's a good way to get from origin to GURL?

I think we've intentionally left direct conversions out since ideally we
shouldn't be doing those conversions, but I'm not sure. mkwst@ ?

[commit-bot: I haz the power, 2016-03-22 16:42:28]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-03-22 16:44:34]

Description was changed from

==========
Fix effective Origin URLs for IDB + Cache for file:/// pages

This is a terrible, horrible, no good, very bad hack.

Previously, storage APIs (notable, Indexed DB, Cache Storage) would
pass the origins they operated on from Blink to Chromium's
content/browser as a serialization done through
SecurityOrigin->DatabaseIdentifier->String->IPC->String->GURL. That
was simplified to SecurityOrigin->String->GURL->IPC. A side effect of
this was that pages browsed as file:// URLs would end up as invalid
GURLs due to stricter checks in the SecurityOrigin->String step.
Special case that conversion to restore the previous behavior.

R=michaeln@chromium.org,mkwst@chromium.org
BUG=595840
==========

to

==========
Fix effective Origin URLs for IDB + Cache for file:/// pages

This is a terrible, horrible, no good, very bad hack.

Previously, storage APIs (notable, Indexed DB, Cache Storage) would
pass the origins they operated on from Blink to Chromium's
content/browser as a serialization done through
SecurityOrigin->DatabaseIdentifier->String->IPC->String->GURL. That
was simplified to SecurityOrigin->String->GURL->IPC. A side effect of
this was that pages browsed as file:// URLs would end up as invalid
GURLs due to stricter checks in the SecurityOrigin->String step.
Special case that conversion to restore the previous behavior.

R=michaeln@chromium.org,mkwst@chromium.org
BUG=595840

Committed: https://crrev.com/5721760f19e829770503b2aa4e51ff76848f2270
Cr-Commit-Position: refs/heads/master@{#382587}
==========

[commit-bot: I haz the power, 2016-03-22 16:44:35]

Patchset 2 (id:??) landed as
https://crrev.com/5721760f19e829770503b2aa4e51ff76848f2270
Cr-Commit-Position: refs/heads/master@{#382587}