IndexedDB: Use url::Origin rather than GURL for representing origins

IndexedDB: Use url::Origin rather than GURL for representing origins

This just updates the internals of Indexed DB. GURL is still used in
content/public/browser, storage/browser/quota, and
storage/browser/database so there are still conversions Origin<->GURL
conversions on those API boundaries.

R=michaeln@chromium.org
BUG=598424
Committed: https://crrev.com/bebef3258b61d65b2400fa7c339f56d558e69288
Cr-Commit-Position: refs/heads/master@{#387621}

[jsbell, 2016-03-30 23:28:35]

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

[jsbell, 2016-03-30 23:28:36]

michaeln@, cmumford@ - can you take a look?

Failure was due to a glitch in the dependent patchset. I'll reupload/resubmit
try jobs shortly but this CL itself should be fine. (Don't trust me,
obviously...)

[cmumford, 2016-03-31 20:54:16]

lgtm

https://codereview.chromium.org/1841553002/diff/1/content/browser/indexed_db/...
File content/browser/indexed_db/indexed_db_context_impl.cc (right):

https://codereview.chromium.org/1841553002/diff/1/content/browser/indexed_db/...
content/browser/indexed_db/indexed_db_context_impl.cc:94: auto file_path_iter =
file_paths.begin();
This is more of a style question/comment. I don't use iterator/iter/it for range
based for loops. I would instead use "file_path". Is there a consensus on this?

Also, "iter" was const before, and your use of auto has made it non-const I
believe.

https://codereview.chromium.org/1841553002/diff/1/content/browser/indexed_db/...
content/browser/indexed_db/indexed_db_context_impl.cc:317: void
IndexedDBContextImpl::DeleteForOrigin(const GURL& origin_url) {
This function will go away right? Want to add a TODO?

https://codereview.chromium.org/1841553002/diff/1/content/browser/indexed_db/...
File content/browser/indexed_db/indexed_db_database.cc (right):

https://codereview.chromium.org/1841553002/diff/1/content/browser/indexed_db/...
content/browser/indexed_db/indexed_db_database.cc:1916: // TODO(jsbell): Why is
this used here?
My guess is a cut-and-paste error from line 1843. Looks safe to delete "origin".

https://codereview.chromium.org/1841553002/diff/1/content/browser/indexed_db/...
File content/browser/indexed_db/indexed_db_database.h (right):

https://codereview.chromium.org/1841553002/diff/1/content/browser/indexed_db/...
content/browser/indexed_db/indexed_db_database.h:51: typedef
std::pair<url::Origin, base::string16> Identifier;
using Identifier = std::pair<url::Origin, base::string16>;

https://codereview.chromium.org/1841553002/diff/1/content/browser/indexed_db/...
File content/browser/indexed_db/indexed_db_factory_impl.cc (right):

https://codereview.chromium.org/1841553002/diff/1/content/browser/indexed_db/...
content/browser/indexed_db/indexed_db_factory_impl.cc:303: const url::Origin&
origin) {
What about adding

using url::Origin;

at the top of this file to simplify the *.cc files? Worth it?

[jsbell, 2016-04-01 20:31:35]

Thanks. Another look, cmumford@ ?

https://codereview.chromium.org/1841553002/diff/1/content/browser/indexed_db/...
File content/browser/indexed_db/indexed_db_context_impl.cc (right):

https://codereview.chromium.org/1841553002/diff/1/content/browser/indexed_db/...
content/browser/indexed_db/indexed_db_context_impl.cc:94: auto file_path_iter =
file_paths.begin();
On 2016/03/31 20:54:16, cmumford wrote:
> This is more of a style question/comment. I don't use iterator/iter/it for
range
> based for loops. I would instead use "file_path". Is there a consensus on
this?

Not a range-based for loop here, but point taken. I've reworked it to make it
more readable. (If only we had a zip(iter, iter) -> iter<pair>...)
 
> Also, "iter" was const before, and your use of auto has made it non-const I
> believe.

Thanks. I believe cbegin()/cend() are the right approach here w/ auto. How does
this look?

https://codereview.chromium.org/1841553002/diff/1/content/browser/indexed_db/...
content/browser/indexed_db/indexed_db_context_impl.cc:317: void
IndexedDBContextImpl::DeleteForOrigin(const GURL& origin_url) {
On 2016/03/31 20:54:16, cmumford wrote:
> This function will go away right? Want to add a TODO?

Done.

https://codereview.chromium.org/1841553002/diff/1/content/browser/indexed_db/...
File content/browser/indexed_db/indexed_db_database.cc (right):

https://codereview.chromium.org/1841553002/diff/1/content/browser/indexed_db/...
content/browser/indexed_db/indexed_db_database.cc:1916: // TODO(jsbell): Why is
this used here?
On 2016/03/31 20:54:16, cmumford wrote:
> My guess is a cut-and-paste error from line 1843. Looks safe to delete
"origin".

Removed.

https://codereview.chromium.org/1841553002/diff/1/content/browser/indexed_db/...
File content/browser/indexed_db/indexed_db_database.h (right):

https://codereview.chromium.org/1841553002/diff/1/content/browser/indexed_db/...
content/browser/indexed_db/indexed_db_database.h:51: typedef
std::pair<url::Origin, base::string16> Identifier;
On 2016/03/31 20:54:16, cmumford wrote:
> using Identifier = std::pair<url::Origin, base::string16>;

Done.

https://codereview.chromium.org/1841553002/diff/1/content/browser/indexed_db/...
File content/browser/indexed_db/indexed_db_factory_impl.cc (right):

https://codereview.chromium.org/1841553002/diff/1/content/browser/indexed_db/...
content/browser/indexed_db/indexed_db_factory_impl.cc:303: const url::Origin&
origin) {
On 2016/03/31 20:54:16, cmumford wrote:
> What about adding
> 
> using url::Origin;
> 
> at the top of this file to simplify the *.cc files? Worth it?

Done.

[cmumford, 2016-04-04 21:03:26]

slgtm

[michaeln, 2016-04-06 21:03:32]

r/s lgtm

[jsbell, 2016-04-15 16:41:01]

The CQ bit was checked by jsbell@chromium.org

[jsbell, 2016-04-15 16:41:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
cmumford@chromium.org, michaeln@chromium.org
Link to the patchset: https://codereview.chromium.org/1841553002/#ps40001
(title: "Rebased")

[commit-bot: I haz the power, 2016-04-15 16:41:19]

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

[commit-bot: I haz the power, 2016-04-15 16:46:35]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-15 16:48:04]

Description was changed from

==========
IndexedDB: Use url::Origin rather than GURL for representing origins

This just updates the internals of Indexed DB. GURL is still used in
content/public/browser, storage/browser/quota, and
storage/browser/database so there are still conversions Origin<->GURL
conversions on those API boundaries.

R=michaeln@chromium.org
BUG=598424
==========

to

==========
IndexedDB: Use url::Origin rather than GURL for representing origins

This just updates the internals of Indexed DB. GURL is still used in
content/public/browser, storage/browser/quota, and
storage/browser/database so there are still conversions Origin<->GURL
conversions on those API boundaries.

R=michaeln@chromium.org
BUG=598424

Committed: https://crrev.com/bebef3258b61d65b2400fa7c339f56d558e69288
Cr-Commit-Position: refs/heads/master@{#387621}
==========

[commit-bot: I haz the power, 2016-04-15 16:48:05]

Patchset 3 (id:??) landed as
https://crrev.com/bebef3258b61d65b2400fa7c339f56d558e69288
Cr-Commit-Position: refs/heads/master@{#387621}