New issue
Advanced search Search tips

Issue 915773 link

Starred by 2 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

Clean up GURL usage with StorageUsageInfo

Project Member Reported by raul.tam...@gmail.com, Dec 17

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/71.0.3578.98 Safari/537.36

Steps to reproduce the problem:
N/A

What is the expected behavior?

What went wrong?
Conversion of StorageUsageInfo to url::Origin by https://chromium-review.googlesource.com/c/chromium/src/+/1378978 has resulted in lots of GURL/url::Origin interop code at API boundaries.
Clean this up where possible.

Did this work before? N/A 

Chrome version: 71.0.3578.98  Channel: stable
OS Version: 10.0
Flash Version:
 
Cc: jsb...@chromium.org
Components: Blink>Storage
Labels: -Type-Bug Type-Task
Status: Started (was: Unconfirmed)
Picking an arbitrary component; this is somewhere between Storage and Privacy
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 18

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

commit f2a59ca50494d7b5ab16585bfcc183120c80506c
Author: Raul Tambre <raul@tambre.ee>
Date: Tue Dec 18 16:59:46 2018

Convert StorageUsageInfo to use url::Origin

Conversion done at API boundaries is minimal.
There are no functional changes.

Bug: 915773

Change-Id: I99c8157cde329a24d2bcaeb868c5ddf1fbffddc1
Reviewed-on: https://chromium-review.googlesource.com/c/1378978
Reviewed-by: Joshua Bell <jsbell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617534}
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/chrome/browser/browsing_data/browsing_data_cache_storage_helper.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/chrome/browser/browsing_data/browsing_data_cache_storage_helper_browsertest.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/chrome/browser/browsing_data/browsing_data_indexed_db_helper.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/chrome/browser/browsing_data/browsing_data_indexed_db_helper_browsertest.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/chrome/browser/browsing_data/browsing_data_local_storage_helper.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/chrome/browser/browsing_data/browsing_data_service_worker_helper.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/chrome/browser/browsing_data/cookies_tree_model.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/chrome/browser/browsing_data/cookies_tree_model.h
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/chrome/browser/browsing_data/cookies_tree_model_unittest.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/chrome/browser/browsing_data/counters/site_data_counting_helper.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/chrome/browser/browsing_data/mock_browsing_data_cache_storage_helper.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/chrome/browser/browsing_data/mock_browsing_data_indexed_db_helper.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/chrome/browser/browsing_data/mock_browsing_data_service_worker_helper.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/chrome/browser/engagement/important_sites_usage_counter.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/chrome/browser/sessions/session_data_deleter.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/chrome/browser/ui/webui/cookies_tree_model_util.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/content/browser/browsing_data/clear_site_data_handler_browsertest.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/content/browser/cache_storage/cache_storage_manager.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/content/browser/cache_storage/cache_storage_manager_unittest.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/content/browser/dom_storage/dom_storage_browsertest.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/content/browser/dom_storage/dom_storage_context_wrapper.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/content/browser/dom_storage/local_storage_context_mojo.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/content/browser/dom_storage/local_storage_context_mojo_unittest.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/content/browser/indexed_db/indexed_db_context_impl.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/content/browser/service_worker/service_worker_context_wrapper.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/content/browser/service_worker/service_worker_quota_client.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/content/browser/storage_partition_impl.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/content/browser/storage_partition_impl_unittest.cc
[modify] https://crrev.com/f2a59ca50494d7b5ab16585bfcc183120c80506c/content/public/browser/storage_usage_info.h

I'll be working on the cleanup.
FYI, if you're in the mood for similar refactors:

* Any function where we call url::Origin::GetURL() where it would be reasonable to pass an origin, we could add an overload or convert all callers. Examples:

  * net::GetHostOrSpecFromURL
  * content::IsOriginSecure
  * IsCookieAccessAllowed
  * SpecialStoragePolicy::IsStoragedProtected (etc)

* Any unnecessary GURL usage in chrome/browser/browsing_data - there is a lot, so this may be daunting, and may merit a design doc. There is some other refactoring we may want to do first (e.g. adding common interfaces to many of the storage-type contexts so that the BrowsingDataXXXHelper classes can collapsed).



Owner: r...@tambre.ee
Labels: -OS-Windows -Pri-2 -Arch-x86_64 -Via-Wizard-Other Pri-3
I'll definitely be looking into converting other places, where origins ought to be used.
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 15

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

commit d51fbabd5ac66ca165c069130ca61c8662221c96
Author: Raul Tambre <raul@tambre.ee>
Date: Tue Jan 15 08:07:39 2019

Convert BrowsingDataFileSystemHelper to use url::Origin

Conversion done at API boundaries is minimal.
There are no functional changes.

Bug: 598424, 915773
Change-Id: I8d9ebc9dcce07724473d4c26e5acf3d93e7f9cc0
Reviewed-on: https://chromium-review.googlesource.com/c/1390039
Commit-Queue: Raul Tambre <raul@tambre.ee>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Reviewed-by: calamity <calamity@chromium.org>
Reviewed-by: Joshua Bell <jsbell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622779}
[modify] https://crrev.com/d51fbabd5ac66ca165c069130ca61c8662221c96/chrome/browser/browsing_data/browsing_data_file_system_helper.cc
[modify] https://crrev.com/d51fbabd5ac66ca165c069130ca61c8662221c96/chrome/browser/browsing_data/browsing_data_file_system_helper.h
[modify] https://crrev.com/d51fbabd5ac66ca165c069130ca61c8662221c96/chrome/browser/browsing_data/browsing_data_file_system_helper_unittest.cc
[modify] https://crrev.com/d51fbabd5ac66ca165c069130ca61c8662221c96/chrome/browser/browsing_data/cookies_tree_model.cc
[modify] https://crrev.com/d51fbabd5ac66ca165c069130ca61c8662221c96/chrome/browser/browsing_data/cookies_tree_model_unittest.cc
[modify] https://crrev.com/d51fbabd5ac66ca165c069130ca61c8662221c96/chrome/browser/browsing_data/mock_browsing_data_file_system_helper.cc
[modify] https://crrev.com/d51fbabd5ac66ca165c069130ca61c8662221c96/chrome/browser/browsing_data/mock_browsing_data_file_system_helper.h
[modify] https://crrev.com/d51fbabd5ac66ca165c069130ca61c8662221c96/chrome/browser/content_settings/local_shared_objects_container.cc
[modify] https://crrev.com/d51fbabd5ac66ca165c069130ca61c8662221c96/chrome/browser/content_settings/tab_specific_content_settings.cc
[modify] https://crrev.com/d51fbabd5ac66ca165c069130ca61c8662221c96/chrome/browser/ui/webui/cookies_tree_model_util.cc

Sign in to add a comment