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

Issue 591240 link

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



Sign in to add a comment

Reduce use of DatabaseIdentifier

Project Member Reported by jsb...@chromium.org, Mar 2 2016 
DatabaseIdentifier is a concept invented for our WebSQL implementation that is used for mapping an origin to a string which is safe to use as a filename. 

Unfortunately, it gets used in many storage API implementations (IndexedDB, FileSystem, DOMStorage, CacheStorage, ...) as a proxy for origin - passed as a string over IPC (why not GURL?), passed into APIs (also, why not GURL?), etc.

We also ended up with two separate impls (blink, chromium) that need to match. There are also many places that do conversions (GURL->string) and then immediately convert back.

Stop the crazy!

We should restrict its use to the guts of implementations, really just when filenames are needed, and even then wrapped with module-specific helper functions so that it is an implementation detail not a contract.
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 9 2016 

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

commit ab08537f91041bb00b4060e0ba6a770e9eab3d64
Author: jsbell <jsbell@chromium.org>
Date: Wed Mar 09 00:29:25 2016

IndexedDB: Pass origin to platform/IPC, rather than DatabaseIdentifier

Don't convert an origin to a DatabaseIdentifier just to immediately
convert it back to an origin. Just pass the origin as a GURL.

BUG= 591240 
R=mkwst@chromium.org

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

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

[modify] https://crrev.com/ab08537f91041bb00b4060e0ba6a770e9eab3d64/content/browser/indexed_db/indexed_db_dispatcher_host.cc
[modify] https://crrev.com/ab08537f91041bb00b4060e0ba6a770e9eab3d64/content/child/indexed_db/indexed_db_dispatcher.cc
[modify] https://crrev.com/ab08537f91041bb00b4060e0ba6a770e9eab3d64/content/child/indexed_db/indexed_db_dispatcher.h
[modify] https://crrev.com/ab08537f91041bb00b4060e0ba6a770e9eab3d64/content/child/indexed_db/webidbfactory_impl.cc
[modify] https://crrev.com/ab08537f91041bb00b4060e0ba6a770e9eab3d64/content/child/indexed_db/webidbfactory_impl.h
[modify] https://crrev.com/ab08537f91041bb00b4060e0ba6a770e9eab3d64/content/common/indexed_db/indexed_db_messages.h
[modify] https://crrev.com/ab08537f91041bb00b4060e0ba6a770e9eab3d64/third_party/WebKit/Source/modules/indexeddb/IDBFactory.cpp
[modify] https://crrev.com/ab08537f91041bb00b4060e0ba6a770e9eab3d64/third_party/WebKit/public/platform/modules/indexeddb/WebIDBFactory.h
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 10 2016 

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

commit b8f7a9160398c23a44807ff474e7e5288e223416
Author: jsbell <jsbell@chromium.org>
Date: Thu Mar 10 18:59:50 2016

Reduce use of DatabaseIdentifier in Indexed DB entry points.

A DatabaseIdentifier is a way to serialize an origin URL, but it is
only needed when creating filenames. Stop using it where an origin URL
is perfectly appropriate, and remove duplicate conversion functions
when it is used.

Also, remove a few unused includes of database identifier headers.

BUG= 591240 
R=michaeln@chromium.org

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

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

[modify] https://crrev.com/b8f7a9160398c23a44807ff474e7e5288e223416/chrome/browser/extensions/extension_service_unittest.cc
[modify] https://crrev.com/b8f7a9160398c23a44807ff474e7e5288e223416/content/browser/indexed_db/indexed_db_backing_store.cc
[modify] https://crrev.com/b8f7a9160398c23a44807ff474e7e5288e223416/content/browser/indexed_db/indexed_db_context_impl.cc
[modify] https://crrev.com/b8f7a9160398c23a44807ff474e7e5288e223416/content/browser/indexed_db/indexed_db_context_impl.h
[modify] https://crrev.com/b8f7a9160398c23a44807ff474e7e5288e223416/content/browser/indexed_db/indexed_db_factory_impl.cc
[modify] https://crrev.com/b8f7a9160398c23a44807ff474e7e5288e223416/content/browser/indexed_db/indexed_db_factory_unittest.cc
[modify] https://crrev.com/b8f7a9160398c23a44807ff474e7e5288e223416/content/browser/indexed_db/indexed_db_quota_client_unittest.cc
[modify] https://crrev.com/b8f7a9160398c23a44807ff474e7e5288e223416/content/browser/indexed_db/indexed_db_unittest.cc
[modify] https://crrev.com/b8f7a9160398c23a44807ff474e7e5288e223416/content/public/browser/indexed_db_context.h
[modify] https://crrev.com/b8f7a9160398c23a44807ff474e7e5288e223416/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScope.cpp
Project Member

Comment 7 by bugdroid1@chromium.org, Mar 22 2016 

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

commit 1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930
Author: jsbell <jsbell@chromium.org>
Date: Tue Mar 22 20:07:28 2016

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

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

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

[modify] https://crrev.com/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930/content/child/blink_platform_impl.cc
[modify] https://crrev.com/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930/content/child/blink_platform_impl.h
[modify] https://crrev.com/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930/content/child/db_message_filter.cc
[modify] https://crrev.com/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930/content/child/web_database_observer_impl.cc
[modify] https://crrev.com/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930/content/child/web_database_observer_impl.h
[modify] https://crrev.com/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930/content/renderer/renderer_blink_platform_impl.cc
[modify] https://crrev.com/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930/content/renderer/renderer_blink_platform_impl.h
[modify] https://crrev.com/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930/third_party/WebKit/Source/modules/cachestorage/InspectorCacheStorageAgent.cpp
[modify] https://crrev.com/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930/third_party/WebKit/Source/modules/filesystem/DOMFileSystem.cpp
[modify] https://crrev.com/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930/third_party/WebKit/Source/modules/webdatabase/Database.cpp
[modify] https://crrev.com/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930/third_party/WebKit/Source/modules/webdatabase/DatabaseTracker.cpp
[modify] https://crrev.com/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930/third_party/WebKit/Source/modules/webdatabase/DatabaseTracker.h
[modify] https://crrev.com/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930/third_party/WebKit/Source/modules/webdatabase/QuotaTracker.cpp
[modify] https://crrev.com/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930/third_party/WebKit/Source/modules/webdatabase/QuotaTracker.h
[modify] https://crrev.com/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930/third_party/WebKit/Source/modules/webdatabase/SQLTransactionClient.cpp
[modify] https://crrev.com/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930/third_party/WebKit/Source/platform/blink_platform.gypi
[modify] https://crrev.com/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930/third_party/WebKit/Source/platform/exported/WebSecurityOrigin.cpp
[delete] https://crrev.com/5f475e52d2c371c539e697852dd3a0162b2a1987/third_party/WebKit/Source/platform/weborigin/DatabaseIdentifier.cpp
[delete] https://crrev.com/5f475e52d2c371c539e697852dd3a0162b2a1987/third_party/WebKit/Source/platform/weborigin/DatabaseIdentifier.h
[delete] https://crrev.com/5f475e52d2c371c539e697852dd3a0162b2a1987/third_party/WebKit/Source/platform/weborigin/DatabaseIdentifierTest.cpp
[modify] https://crrev.com/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930/third_party/WebKit/Source/web/WebDatabase.cpp
[modify] https://crrev.com/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930/third_party/WebKit/public/platform/Platform.h
[modify] https://crrev.com/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930/third_party/WebKit/public/platform/WebDatabaseObserver.h
[modify] https://crrev.com/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930/third_party/WebKit/public/platform/WebSecurityOrigin.h
[modify] https://crrev.com/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930/third_party/WebKit/public/web/WebDatabase.h

Comment 8 by jsb...@chromium.org, Jul 20 2016

Status: Available (was: Started)

Comment 9 by jsb...@chromium.org, Oct 10 2016

Cc: jsb...@chromium.org
Owner: ----

Comment 10 by jsb...@chromium.org, Jun 2 2017

Status: Fixed (was: Available)
Closing this off - we'll find cases we want to revisit as needed.
►

Sign in to add a comment