IndexedDB: Pass url::Origin rather than GURL over IPC

IndexedDB: Pass url::Origin rather than GURL over IPC

Eliminate some WebSecurityOrigin -> string -> GURL conversions. The
IndexedDB back-end in content/browser/indexed_db still uses GURL
internally, so there's more cleanup to be done in follow-ups.

R=mkwst@chromium.org
BUG=591482
Committed: https://crrev.com/d77cea9f7f883a28cc9e5ca1e9091c0936a588fc
Cr-Commit-Position: refs/heads/master@{#387138}

[jsbell, 2016-03-23 22:46:24]

Description was changed from

==========
IndexedDB: Pass url::Origin rather than GURL over IPC

Eliminate some WebSecurityOrigin -> string -> GURL conversions. The
IndexedDB back-end in content/browser/indexed_db still uses GURL
internally, so there's more cleanup to be done in follow-ups.

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

to

==========
IndexedDB: Pass url::Origin rather than GURL over IPC

Eliminate some WebSecurityOrigin -> string -> GURL conversions. The
IndexedDB back-end in content/browser/indexed_db still uses GURL
internally, so there's more cleanup to be done in follow-ups.

R=mkwst@chromium.org
BUG=591482
==========

[jsbell, 2016-03-23 22:47:01]

jsbell@chromium.org changed reviewers:
- michaeln@chromium.org

[jsbell, 2016-03-23 22:47:01]

mkwst@ - can you please review?

(Not actually dependent on the other CL)

[Mike West, 2016-03-24 14:30:56]

LGTM. I'm not thrilled about all the `GURL(o.Serialize())` calls, and I think it
might be better to just take things all the way through (in lots of smaller CLs,
I suppose), but if you're happier with the code this way, I won't stand in your
way. :)

[jsbell, 2016-03-24 16:29:12]

On 2016/03/24 14:30:56, Mike West wrote:
> LGTM. I'm not thrilled about all the `GURL(o.Serialize())` calls, and I think
it
> might be better to just take things all the way through (in lots of smaller
CLs,
> I suppose), but if you're happier with the code this way, I won't stand in
your
> way. :)

Yeah, I'm envisioning that this is the first of the CLs, starting from IPC and
working back into the guts of the impl of each storage type.

[jsbell, 2016-03-30 23:26:25]

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

[jsbell, 2016-03-30 23:26:26]

michaeln@, cmumford@ - can you take a look at the IDB changes?

[michaeln, 2016-04-06 20:44:55]

lgtm

[jsbell, 2016-04-07 17:58:09]

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

[jsbell, 2016-04-07 17:58:10]

mpearson@ - histograms review?

(Note that I'll need to rebase this on top of the other review and adjust the
numbers. Just putting this in your queue.)

[Mark P, 2016-04-07 21:41:36]

histograms.xml lgtm approval once you properly rebase

[jsbell, 2016-04-13 19:32:43]

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

[jsbell, 2016-04-13 19:32:44]

creis@ - can you OWNERS-review the content/browser/bad_message.h?

[Charlie Reis, 2016-04-13 20:21:07]

bad_message.h LGTM if you're ok with conflating these separate conditions.

https://codereview.chromium.org/1832553002/diff/80001/content/browser/indexed...
File content/browser/indexed_db/indexed_db_dispatcher_host.cc (right):

https://codereview.chromium.org/1832553002/diff/80001/content/browser/indexed...
content/browser/indexed_db/indexed_db_dispatcher_host.cc:324:
bad_message::ReceivedBadMessage(this, bad_message::IDBDH_INVALID_ORIGIN);
You're using the same value in multiple places, which means you won't be able to
distinguish which IPC is to blame if this kill happens in practice.  Maybe
that's fine for now, and you can split them apart if it turns out that you see a
bunch in the wild?

(Don't forget to check the histogram after a few days on Dev Channel, or
subscribe to alerts.)

[jsbell, 2016-04-13 21:52:45]

https://codereview.chromium.org/1832553002/diff/80001/content/browser/indexed...
File content/browser/indexed_db/indexed_db_dispatcher_host.cc (right):

https://codereview.chromium.org/1832553002/diff/80001/content/browser/indexed...
content/browser/indexed_db/indexed_db_dispatcher_host.cc:324:
bad_message::ReceivedBadMessage(this, bad_message::IDBDH_INVALID_ORIGIN);
On 2016/04/13 20:21:07, Charlie Reis wrote:
> You're using the same value in multiple places, which means you won't be able
to
> distinguish which IPC is to blame if this kill happens in practice.  Maybe
> that's fine for now, and you can split them apart if it turns out that you see
a
> bunch in the wild?
> 
> (Don't forget to check the histogram after a few days on Dev Channel, or
> subscribe to alerts.)

Yeah, this would only happen if (1) we've got a bug and allowing an invalid
origin to start using Indexed DB (so any IPC is equally bad, and has a common
fix) or (2) the renderer is corrupted/compromised.

[jsbell, 2016-04-13 21:52:49]

The CQ bit was checked by jsbell@chromium.org

[jsbell, 2016-04-13 21:52:49]

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

[commit-bot: I haz the power, 2016-04-13 21:53:40]

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

[commit-bot: I haz the power, 2016-04-13 22:59:04]

Description was changed from

==========
IndexedDB: Pass url::Origin rather than GURL over IPC

Eliminate some WebSecurityOrigin -> string -> GURL conversions. The
IndexedDB back-end in content/browser/indexed_db still uses GURL
internally, so there's more cleanup to be done in follow-ups.

R=mkwst@chromium.org
BUG=591482
==========

to

==========
IndexedDB: Pass url::Origin rather than GURL over IPC

Eliminate some WebSecurityOrigin -> string -> GURL conversions. The
IndexedDB back-end in content/browser/indexed_db still uses GURL
internally, so there's more cleanup to be done in follow-ups.

R=mkwst@chromium.org
BUG=591482
==========

[commit-bot: I haz the power, 2016-04-13 22:59:05]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-04-13 23:00:26]

Description was changed from

==========
IndexedDB: Pass url::Origin rather than GURL over IPC

Eliminate some WebSecurityOrigin -> string -> GURL conversions. The
IndexedDB back-end in content/browser/indexed_db still uses GURL
internally, so there's more cleanup to be done in follow-ups.

R=mkwst@chromium.org
BUG=591482
==========

to

==========
IndexedDB: Pass url::Origin rather than GURL over IPC

Eliminate some WebSecurityOrigin -> string -> GURL conversions. The
IndexedDB back-end in content/browser/indexed_db still uses GURL
internally, so there's more cleanup to be done in follow-ups.

R=mkwst@chromium.org
BUG=591482

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

[commit-bot: I haz the power, 2016-04-13 23:00:27]

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