Remove Blink's DatabaseIdentifier implementation

Remove Blink's DatabaseIdentifier implementation

DatabaseIdentifier is used to map an origin (scheme/host/port) to a
string that is safe to use as a filename, and is used by multiple
storage APIs. Both Blink and Chromium ended up with implementations
that needed to be kept in sync, and it was also used in many places
unnecessarily.

This change removes the Blink implementation:

* When identifiers are still used for renderer->browser messaging in
  the WebSQL implementation, a WebSecurityOrigin is passed from Blink
  to Chromium and the identifiers are minted in Chromium code. (This
  should be replaced with url::Origin end-to-end, but we'll do that
  later - bug 591482.)

* The WebSQL implementation in Blink used database identifiers
  internally in maps; use stringified URLs instead, since they don't
  need to be filename-friendly.

* When WebSQL and FileSystem APIs do need to mint filenames in
  Blink, do so via Platform APIs implemented in Chromium.

BUG=591240
R=kinuko@chromium.org,michaeln@chromium.org,mkwst@chromium.org

Committed: https://crrev.com/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930
Cr-Commit-Position: refs/heads/master@{#382660}

[Mike West, 2016-03-11 07:29:02]

LGTM. Yay! Getting this out of Blink is a nice improvement. Thank you!

[jsbell, 2016-03-11 17:16:59]

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

[jsbell, 2016-03-11 17:16:59]

kinuko@, michaeln@ - can you please review?

Extra special attention to identifier replacements inside the webdatabase code
would be very appreciated; the use of identifiers rather than stringified URLs
may have some deeper purpose that wasn't apparent in the code.

[jsbell, 2016-03-11 17:17:15]

Description was changed from

==========
Remove Blink's DatabaseIdentifier implementation

DatabaseIdentifier is used to map an origin (scheme/host/port) to a
string that is safe to use as a filename, and is used by multiple
storage APIs. Both Blink and Chromium ended up with implementations
that needed to be kept in sync, and it was also used in many places
unnecessarily.

This change removes the Blink implementation:

* When identifiers are still used for renderer->browser messaging in
  the WebSQL implementation, a WebSecurityOrigin is passed from Blink
  to Chromium and the identifiers are minted in Chromium code. (This
  should be replaced with url::Origin end-to-end, but we'll do that
  later - bug 591482.)

* The WebSQL implementation in Blink used database identifiers
  internally in maps; use stringified URLs instead, since they don't
  need to be filename-friendly.

* When WebSQL and FileSystem APIs do need to mint filenames in
  Blink, do so via Platform APIs implemented in Chromium.

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

to

==========
Remove Blink's DatabaseIdentifier implementation

DatabaseIdentifier is used to map an origin (scheme/host/port) to a
string that is safe to use as a filename, and is used by multiple
storage APIs. Both Blink and Chromium ended up with implementations
that needed to be kept in sync, and it was also used in many places
unnecessarily.

This change removes the Blink implementation:

* When identifiers are still used for renderer->browser messaging in
  the WebSQL implementation, a WebSecurityOrigin is passed from Blink
  to Chromium and the identifiers are minted in Chromium code. (This
  should be replaced with url::Origin end-to-end, but we'll do that
  later - bug 591482.)

* The WebSQL implementation in Blink used database identifiers
  internally in maps; use stringified URLs instead, since they don't
  need to be filename-friendly.

* When WebSQL and FileSystem APIs do need to mint filenames in
  Blink, do so via Platform APIs implemented in Chromium.

BUG=591240
R=kinuko@chromium.org,michaeln@chromium.org,mkwst@chromium.org
==========

[jsbell, 2016-03-11 21:49:38]

org.chromium.android_webview.test.AwSettingsTest#testDatabaseEnabled failure
seems like it might be legit?

[michaeln, 2016-03-12 00:33:38]

lgtm 2

https://codereview.chromium.org/1779413002/diff/1/third_party/WebKit/public/p...
File third_party/WebKit/public/platform/Platform.h (right):

https://codereview.chromium.org/1779413002/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/Platform.h:214: virtual WebString
databaseCreateOriginIdentifier(const WebSecurityOrigin& origin) { return
WebString(); }
the android webview test harness probably has another Platform derivative that
needs to override the method

[kinuko, 2016-03-12 10:55:55]

lgtm

[jsbell, 2016-03-14 22:30:59]

https://codereview.chromium.org/1779413002/diff/1/third_party/WebKit/public/p...
File third_party/WebKit/public/platform/Platform.h (right):

https://codereview.chromium.org/1779413002/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/Platform.h:214: virtual WebString
databaseCreateOriginIdentifier(const WebSecurityOrigin& origin) { return
WebString(); }
On 2016/03/12 00:33:38, michaeln wrote:
> the android webview test harness probably has another Platform derivative that
> needs to override the method

Not quite that simple, alas. I'm not done debugging (it's a bit slow!) but
apparently in webview an invalid URL is being passed in, so Chromium's
DatabaseIdentifier::CreateFromOrigin() returns something different than what
Blink did.

[jsbell, 2016-03-16 16:40:08]

https://codereview.chromium.org/1779413002/diff/1/third_party/WebKit/public/p...
File third_party/WebKit/public/platform/Platform.h (right):

https://codereview.chromium.org/1779413002/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/Platform.h:214: virtual WebString
databaseCreateOriginIdentifier(const WebSecurityOrigin& origin) { return
WebString(); }
And continuing the investigation (didn't have much time yesterday) it a URL
where blink returns true for isLocal(). Hopefully it'll repro on desktop with
file: or something.

[jsbell, 2016-03-22 19:59:49]

The CQ bit was checked by jsbell@chromium.org

[jsbell, 2016-03-22 19:59:50]

The patchset sent to the CQ was uploaded after l-g-t-m from
michaeln@chromium.org, kinuko@chromium.org, mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/1779413002/#ps60001
(title: "Remove unneeded include")

[commit-bot: I haz the power, 2016-03-22 20:00:18]

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

[commit-bot: I haz the power, 2016-03-22 20:07:35]

Description was changed from

==========
Remove Blink's DatabaseIdentifier implementation

DatabaseIdentifier is used to map an origin (scheme/host/port) to a
string that is safe to use as a filename, and is used by multiple
storage APIs. Both Blink and Chromium ended up with implementations
that needed to be kept in sync, and it was also used in many places
unnecessarily.

This change removes the Blink implementation:

* When identifiers are still used for renderer->browser messaging in
  the WebSQL implementation, a WebSecurityOrigin is passed from Blink
  to Chromium and the identifiers are minted in Chromium code. (This
  should be replaced with url::Origin end-to-end, but we'll do that
  later - bug 591482.)

* The WebSQL implementation in Blink used database identifiers
  internally in maps; use stringified URLs instead, since they don't
  need to be filename-friendly.

* When WebSQL and FileSystem APIs do need to mint filenames in
  Blink, do so via Platform APIs implemented in Chromium.

BUG=591240
R=kinuko@chromium.org,michaeln@chromium.org,mkwst@chromium.org
==========

to

==========
Remove Blink's DatabaseIdentifier implementation

DatabaseIdentifier is used to map an origin (scheme/host/port) to a
string that is safe to use as a filename, and is used by multiple
storage APIs. Both Blink and Chromium ended up with implementations
that needed to be kept in sync, and it was also used in many places
unnecessarily.

This change removes the Blink implementation:

* When identifiers are still used for renderer->browser messaging in
  the WebSQL implementation, a WebSecurityOrigin is passed from Blink
  to Chromium and the identifiers are minted in Chromium code. (This
  should be replaced with url::Origin end-to-end, but we'll do that
  later - bug 591482.)

* The WebSQL implementation in Blink used database identifiers
  internally in maps; use stringified URLs instead, since they don't
  need to be filename-friendly.

* When WebSQL and FileSystem APIs do need to mint filenames in
  Blink, do so via Platform APIs implemented in Chromium.

BUG=591240
R=kinuko@chromium.org,michaeln@chromium.org,mkwst@chromium.org
==========

[commit-bot: I haz the power, 2016-03-22 20:07:37]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-03-22 20:09:36]

Description was changed from

==========
Remove Blink's DatabaseIdentifier implementation

DatabaseIdentifier is used to map an origin (scheme/host/port) to a
string that is safe to use as a filename, and is used by multiple
storage APIs. Both Blink and Chromium ended up with implementations
that needed to be kept in sync, and it was also used in many places
unnecessarily.

This change removes the Blink implementation:

* When identifiers are still used for renderer->browser messaging in
  the WebSQL implementation, a WebSecurityOrigin is passed from Blink
  to Chromium and the identifiers are minted in Chromium code. (This
  should be replaced with url::Origin end-to-end, but we'll do that
  later - bug 591482.)

* The WebSQL implementation in Blink used database identifiers
  internally in maps; use stringified URLs instead, since they don't
  need to be filename-friendly.

* When WebSQL and FileSystem APIs do need to mint filenames in
  Blink, do so via Platform APIs implemented in Chromium.

BUG=591240
R=kinuko@chromium.org,michaeln@chromium.org,mkwst@chromium.org
==========

to

==========
Remove Blink's DatabaseIdentifier implementation

DatabaseIdentifier is used to map an origin (scheme/host/port) to a
string that is safe to use as a filename, and is used by multiple
storage APIs. Both Blink and Chromium ended up with implementations
that needed to be kept in sync, and it was also used in many places
unnecessarily.

This change removes the Blink implementation:

* When identifiers are still used for renderer->browser messaging in
  the WebSQL implementation, a WebSecurityOrigin is passed from Blink
  to Chromium and the identifiers are minted in Chromium code. (This
  should be replaced with url::Origin end-to-end, but we'll do that
  later - bug 591482.)

* The WebSQL implementation in Blink used database identifiers
  internally in maps; use stringified URLs instead, since they don't
  need to be filename-friendly.

* When WebSQL and FileSystem APIs do need to mint filenames in
  Blink, do so via Platform APIs implemented in Chromium.

BUG=591240
R=kinuko@chromium.org,michaeln@chromium.org,mkwst@chromium.org

Committed: https://crrev.com/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930
Cr-Commit-Position: refs/heads/master@{#382660}
==========

[commit-bot: I haz the power, 2016-03-22 20:09:38]

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