New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 591482 link

Starred by 1 user
Status: Archived
Owner: ----
Closed: Jun 2017
Cc:
█ mkwst@chromium.org
jsb...@chromium.org
█ michaeln@chromium.org
█ pilgrim@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature

Blocking:
issue 598424



Sign in to add a comment

Use url::Origin rather than GURL for passing origins through IPC

Project Member Reported by jsb...@chromium.org, Mar 2 2016 
There's a common pattern in storage APIs (and probably others)

* Grab a blink::SecurityOrigin
* Convert to blink::WebSecurityOrigin
* Pass into a blink platform API
* In the Chromium code, blink::WebStringToGURL(security_origin.toString())
* Pass GURL over IPC
* Reason about security properties of GURL
* ...
* Profit!

Ideally, this would use url::Origin and not convert to GURL.

Comment 1 by jsb...@chromium.org, Mar 2 2016 

Note that this implies converting the storage backends to use url::Origin, and maybe teaching DatabaseIdentifier about url::Origin (for minting filenames). That can be done one system at a time, though. So this is not trivial, but some juicy refactoring to tackle some day.

Plus, mkwst will give you a cookie.

Comment 2 by jsb...@chromium.org, Mar 2 2016

Components: Blink>Storage

Comment 3 by jsb...@chromium.org, Mar 28 2016

Blocking: 598424
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 2 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9f1be5affd71497458f19e45b73d3a45b9f28e73

commit 9f1be5affd71497458f19e45b73d3a45b9f28e73
Author: jsbell <jsbell@chromium.org>
Date: Sat Apr 02 00:06:27 2016

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

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

BUG= 591482 

Review URL: https://codereview.chromium.org/1826103002

Cr-Commit-Position: refs/heads/master@{#384743}

[modify] https://crrev.com/9f1be5affd71497458f19e45b73d3a45b9f28e73/content/browser/cache_storage/cache_storage_dispatcher_host.cc
[modify] https://crrev.com/9f1be5affd71497458f19e45b73d3a45b9f28e73/content/browser/cache_storage/cache_storage_dispatcher_host.h
[modify] https://crrev.com/9f1be5affd71497458f19e45b73d3a45b9f28e73/content/common/cache_storage/cache_storage_messages.h
[modify] https://crrev.com/9f1be5affd71497458f19e45b73d3a45b9f28e73/content/renderer/cache_storage/cache_storage_dispatcher.cc
[modify] https://crrev.com/9f1be5affd71497458f19e45b73d3a45b9f28e73/content/renderer/cache_storage/cache_storage_dispatcher.h
[modify] https://crrev.com/9f1be5affd71497458f19e45b73d3a45b9f28e73/content/renderer/cache_storage/webserviceworkercachestorage_impl.cc
[modify] https://crrev.com/9f1be5affd71497458f19e45b73d3a45b9f28e73/content/renderer/cache_storage/webserviceworkercachestorage_impl.h
[modify] https://crrev.com/9f1be5affd71497458f19e45b73d3a45b9f28e73/content/renderer/renderer_blink_platform_impl.cc
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 13 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d77cea9f7f883a28cc9e5ca1e9091c0936a588fc

commit d77cea9f7f883a28cc9e5ca1e9091c0936a588fc
Author: jsbell <jsbell@chromium.org>
Date: Wed Apr 13 22:58:52 2016

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 

Review URL: https://codereview.chromium.org/1832553002

Cr-Commit-Position: refs/heads/master@{#387138}

[modify] https://crrev.com/d77cea9f7f883a28cc9e5ca1e9091c0936a588fc/content/browser/bad_message.h
[modify] https://crrev.com/d77cea9f7f883a28cc9e5ca1e9091c0936a588fc/content/browser/indexed_db/indexed_db_dispatcher_host.cc
[modify] https://crrev.com/d77cea9f7f883a28cc9e5ca1e9091c0936a588fc/content/browser/indexed_db/indexed_db_factory.h
[modify] https://crrev.com/d77cea9f7f883a28cc9e5ca1e9091c0936a588fc/content/browser/indexed_db/indexed_db_factory_impl.cc
[modify] https://crrev.com/d77cea9f7f883a28cc9e5ca1e9091c0936a588fc/content/browser/indexed_db/indexed_db_factory_impl.h
[modify] https://crrev.com/d77cea9f7f883a28cc9e5ca1e9091c0936a588fc/content/browser/indexed_db/indexed_db_factory_unittest.cc
[modify] https://crrev.com/d77cea9f7f883a28cc9e5ca1e9091c0936a588fc/content/browser/indexed_db/indexed_db_unittest.cc
[modify] https://crrev.com/d77cea9f7f883a28cc9e5ca1e9091c0936a588fc/content/browser/indexed_db/mock_indexed_db_factory.h
[modify] https://crrev.com/d77cea9f7f883a28cc9e5ca1e9091c0936a588fc/content/child/indexed_db/indexed_db_dispatcher.cc
[modify] https://crrev.com/d77cea9f7f883a28cc9e5ca1e9091c0936a588fc/content/child/indexed_db/indexed_db_dispatcher.h
[modify] https://crrev.com/d77cea9f7f883a28cc9e5ca1e9091c0936a588fc/content/child/indexed_db/webidbfactory_impl.cc
[modify] https://crrev.com/d77cea9f7f883a28cc9e5ca1e9091c0936a588fc/content/common/indexed_db/indexed_db_messages.h
[modify] https://crrev.com/d77cea9f7f883a28cc9e5ca1e9091c0936a588fc/tools/metrics/histograms/histograms.xml
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 21 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/984e21a28bb2e43ba6df6e28c8f0738ee6e23018

commit 984e21a28bb2e43ba6df6e28c8f0738ee6e23018
Author: jsbell <jsbell@chromium.org>
Date: Tue Jun 21 15:26:49 2016

WebSQL: Use url::Origin rather than database identifiers for IPC

Replace WebSecurityOrigin -> GURL -> string -> IPC -> string -> GURL with
simpler WebSecurityOrigin -> IPC -> Origin -> GURL. Consumers of the
messages still consume GURLs (Quota) or DatabaseIdentifiers (WebSQL) so
there's more tidying that can be done in follow-ups.

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

Review-Url: https://codereview.chromium.org/1832473002
Cr-Commit-Position: refs/heads/master@{#400996}

[modify] https://crrev.com/984e21a28bb2e43ba6df6e28c8f0738ee6e23018/content/browser/bad_message.h
[modify] https://crrev.com/984e21a28bb2e43ba6df6e28c8f0738ee6e23018/content/browser/renderer_host/OWNERS
[modify] https://crrev.com/984e21a28bb2e43ba6df6e28c8f0738ee6e23018/content/browser/renderer_host/database_message_filter.cc
[modify] https://crrev.com/984e21a28bb2e43ba6df6e28c8f0738ee6e23018/content/browser/renderer_host/database_message_filter.h
[modify] https://crrev.com/984e21a28bb2e43ba6df6e28c8f0738ee6e23018/content/child/OWNERS
[modify] https://crrev.com/984e21a28bb2e43ba6df6e28c8f0738ee6e23018/content/child/database_util.cc
[modify] https://crrev.com/984e21a28bb2e43ba6df6e28c8f0738ee6e23018/content/child/database_util.h
[modify] https://crrev.com/984e21a28bb2e43ba6df6e28c8f0738ee6e23018/content/child/db_message_filter.cc
[modify] https://crrev.com/984e21a28bb2e43ba6df6e28c8f0738ee6e23018/content/child/db_message_filter.h
[modify] https://crrev.com/984e21a28bb2e43ba6df6e28c8f0738ee6e23018/content/child/web_database_observer_impl.cc
[modify] https://crrev.com/984e21a28bb2e43ba6df6e28c8f0738ee6e23018/content/common/database_messages.h
[modify] https://crrev.com/984e21a28bb2e43ba6df6e28c8f0738ee6e23018/content/renderer/render_thread_impl.cc
[modify] https://crrev.com/984e21a28bb2e43ba6df6e28c8f0738ee6e23018/content/renderer/render_view_impl.cc
[modify] https://crrev.com/984e21a28bb2e43ba6df6e28c8f0738ee6e23018/content/renderer/renderer_blink_platform_impl.cc
[modify] https://crrev.com/984e21a28bb2e43ba6df6e28c8f0738ee6e23018/tools/metrics/histograms/histograms.xml

Comment 7 by jsb...@chromium.org, Jun 5 2017

Status: Archived (was: Available)
Remaining work here should happen as part of mojo-ifying
►

Sign in to add a comment