Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(595)

Issue 11787028: New FileSystemURL cracking (Closed)

Created:
7 years, 11 months ago by tbarzic
Modified:
2 years, 5 months ago
Reviewers:
kinuko, benwells, brettw
CC:
chromium-reviews, jam, nkostylev+watch_chromium.org, feature-media-reviews_chromium.org, tzik+watch_chromium.org, achuith+watch_chromium.org, joi+watch-content_chromium.org, Aaron Boodman, rginda+watch_chromium.org, darin-cc_chromium.org, oshima+watch_chromium.org, chromium-apps-reviews_chromium.org, kinuko+watch, stevenjb+watch_chromium.org, davemoore+watch_chromium.org
Visibility:
Public.

Description

New FileSystemURL cracking follow up on https://codereview.chromium.org/11648027/ Instead of cracking FileSystemURL in its ctor (somewhat magically, using singleton IsolatedContext and SystemExternalMountPoints), require FileSystemURL to be cracked explicitly. The FileSystemURL can be cracked using specific MountPoints implementation or using specific FileSystemContext which will select available MountPoints implementation that should be used to crack the url. (CrackURL/CreateCrackedFileSystemURL methods). Modify FileSystemURL usages to use new cracking methods. BUG=158837 TEST=content_unittests: IsolatedContextTest.* ExternalMountPointsTest.* FileSystemContextTest.* Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=178664

Patch Set 1 #

Patch Set 2 : . #

Patch Set 3 : rebase #

Patch Set 4 : rebase #

Patch Set 5 : . #

Patch Set 6 : . #

Patch Set 7 : FileSystemContext tests #

Patch Set 8 : aaaaand had forgotten to 'git add' file I was uploading :) #

Patch Set 9 : some nits #

Patch Set 10 : fix some tests #

Patch Set 11 : . #

Total comments: 2

Patch Set 12 : fix some tests #

Patch Set 13 : made FileSystemURL ctors private #

Patch Set 14 : diff from the cls already in pipeline #

Patch Set 15 : . #

Patch Set 16 : unit_tests compile #

Patch Set 17 : browser_tests compile #

Total comments: 12

Patch Set 18 : Addressed review comments #

Patch Set 19 : removes FileSystemURL::is_cracked; Some cleanup in file_system_context_unittests #

Patch Set 20 : Couple of nits I noticed #

Total comments: 5

Patch Set 21 : addressed review comments #

Patch Set 22 : . #

Total comments: 1

Patch Set 23 : few nits #

Patch Set 24 : rebase #

Patch Set 25 : rebase #

Patch Set 26 : #

Patch Set 27 : fix test on Win #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1318 lines, -398 lines) Patch
M chrome/browser/browsing_data/browsing_data_file_system_helper_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/drive/drive_task_executor.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/drive/drive_task_executor.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 2 chunks +8 lines, -8 lines 0 comments Download
M chrome/browser/chromeos/extensions/file_browser_private_api.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 3 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/chromeos/extensions/file_browser_private_api.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 27 chunks +180 lines, -75 lines 0 comments Download
M chrome/browser/chromeos/extensions/file_handler_util.h View 1 2 3 4 5 6 7 8 9 10 11 12 6 chunks +16 lines, -9 lines 0 comments Download
M chrome/browser/chromeos/extensions/file_handler_util.cc View 1 2 3 4 5 6 7 8 9 10 11 12 21 chunks +37 lines, -51 lines 0 comments Download
M chrome/browser/chromeos/extensions/file_manager_util.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 4 chunks +11 lines, -8 lines 0 comments Download
M chrome/browser/extensions/api/downloads/downloads_api_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 2 chunks +3 lines, -4 lines 0 comments Download
M chrome/browser/extensions/api/sync_file_system/sync_file_system_api.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 3 chunks +22 lines, -8 lines 0 comments Download
M chrome/browser/extensions/api/sync_file_system/sync_file_system_apitest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +4 lines, -3 lines 0 comments Download
M content/browser/fileapi/fileapi_message_filter.cc View 1 2 3 4 5 17 chunks +19 lines, -19 lines 0 comments Download
M content/content_tests.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +1 line, -0 lines 0 comments Download
M webkit/blob/blob_url_request_job.cc View 1 chunk +1 line, -1 line 0 comments Download
M webkit/blob/blob_url_request_job_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +5 lines, -3 lines 0 comments Download
M webkit/chromeos/fileapi/cros_mount_point_provider.cc View 1 2 3 4 5 6 7 8 9 10 11 13 1 chunk +8 lines, -5 lines 0 comments Download
M webkit/chromeos/fileapi/cros_mount_point_provider_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +31 lines, -24 lines 0 comments Download
M webkit/fileapi/external_mount_points.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 3 chunks +7 lines, -0 lines 0 comments Download
M webkit/fileapi/external_mount_points.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 3 chunks +32 lines, -0 lines 0 comments Download
M webkit/fileapi/external_mount_points_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 3 chunks +235 lines, -0 lines 0 comments Download
M webkit/fileapi/file_system_context.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 8 chunks +29 lines, -4 lines 0 comments Download
M webkit/fileapi/file_system_context.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 4 chunks +46 lines, -3 lines 0 comments Download
A webkit/fileapi/file_system_context_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 1 chunk +219 lines, -0 lines 0 comments Download
M webkit/fileapi/file_system_dir_url_request_job.cc View 1 chunk +1 line, -1 line 0 comments Download
M webkit/fileapi/file_system_dir_url_request_job_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +4 lines, -3 lines 0 comments Download
M webkit/fileapi/file_system_file_stream_reader_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +4 lines, -3 lines 0 comments Download
M webkit/fileapi/file_system_mount_point_provider_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +3 lines, -2 lines 0 comments Download
M webkit/fileapi/file_system_quota_client_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +4 lines, -2 lines 0 comments Download
M webkit/fileapi/file_system_url.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 5 chunks +58 lines, -23 lines 0 comments Download
M webkit/fileapi/file_system_url.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 5 chunks +51 lines, -36 lines 0 comments Download
M webkit/fileapi/file_system_url_request_job.cc View 1 chunk +1 line, -1 line 0 comments Download
M webkit/fileapi/file_system_url_request_job_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +8 lines, -6 lines 0 comments Download
M webkit/fileapi/file_system_url_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 6 chunks +28 lines, -7 lines 0 comments Download
M webkit/fileapi/isolated_context.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 3 chunks +19 lines, -9 lines 0 comments Download
M webkit/fileapi/isolated_context.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 3 chunks +31 lines, -0 lines 0 comments Download
M webkit/fileapi/isolated_context_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 4 chunks +75 lines, -1 line 0 comments Download
M webkit/fileapi/isolated_file_util_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +4 lines, -3 lines 0 comments Download
M webkit/fileapi/local_file_system_test_helper.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 4 chunks +6 lines, -7 lines 0 comments Download
M webkit/fileapi/media/native_media_file_util_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 16 chunks +37 lines, -34 lines 0 comments Download
M webkit/fileapi/mount_points.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 2 chunks +21 lines, -0 lines 0 comments Download
M webkit/fileapi/sandbox_mount_point_provider.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +2 lines, -1 line 0 comments Download
M webkit/fileapi/syncable/canned_syncable_file_system.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +4 lines, -1 line 0 comments Download
M webkit/fileapi/syncable/local_file_sync_status_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -1 line 0 comments Download
M webkit/fileapi/syncable/syncable_file_system_util.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +10 lines, -7 lines 0 comments Download
M webkit/fileapi/syncable/syncable_file_system_util_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -1 line 0 comments Download
M webkit/fileapi/upload_file_system_file_element_reader.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +3 lines, -1 line 0 comments Download
M webkit/fileapi/upload_file_system_file_element_reader_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +5 lines, -3 lines 0 comments Download
M webkit/tools/test_shell/simple_file_system.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 11 chunks +13 lines, -13 lines 0 comments Download
M webkit/tools/test_shell/simple_file_writer.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -0 lines 0 comments Download
M webkit/tools/test_shell/simple_file_writer.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +3 lines, -2 lines 0 comments Download

Messages

Total messages: 16 (0 generated)
tbarzic
7 years, 11 months ago (2013-01-15 20:26:13 UTC) #1
kinuko
https://codereview.chromium.org/11787028/diff/22043/webkit/fileapi/file_system_url.h File webkit/fileapi/file_system_url.h (right): https://codereview.chromium.org/11787028/diff/22043/webkit/fileapi/file_system_url.h#newcode71 webkit/fileapi/file_system_url.h:71: explicit FileSystemURL(const GURL& filesystem_url); How hard it will be ...
7 years, 11 months ago (2013-01-16 04:05:18 UTC) #2
tbarzic
https://codereview.chromium.org/11787028/diff/22043/webkit/fileapi/file_system_url.h File webkit/fileapi/file_system_url.h (right): https://codereview.chromium.org/11787028/diff/22043/webkit/fileapi/file_system_url.h#newcode71 webkit/fileapi/file_system_url.h:71: explicit FileSystemURL(const GURL& filesystem_url); On 2013/01/16 04:05:18, kinuko wrote: ...
7 years, 11 months ago (2013-01-16 04:43:23 UTC) #3
tbarzic
On 2013/01/16 04:43:23, tbarzic wrote: > https://codereview.chromium.org/11787028/diff/22043/webkit/fileapi/file_system_url.h > File webkit/fileapi/file_system_url.h (right): > > https://codereview.chromium.org/11787028/diff/22043/webkit/fileapi/file_system_url.h#newcode71 > ...
7 years, 11 months ago (2013-01-17 21:02:41 UTC) #4
tbarzic
this also fixes http://crbug.com/160693 since it removed ChromeOS drive file system to FileSystemContext specific mount ...
7 years, 11 months ago (2013-01-17 22:57:18 UTC) #5
kinuko
Very cool, I think the change looks much nicer now! On 2013/01/17 22:57:18, tbarzic wrote: ...
7 years, 11 months ago (2013-01-18 07:30:36 UTC) #6
kinuko
Made some comments but nits only. I'll take another look but I think this looks ...
7 years, 11 months ago (2013-01-18 07:33:32 UTC) #7
tbarzic
Note that I've removed is_cracked property from FileSystemURL, since it now seems redundant. https://codereview.chromium.org/11787028/diff/50001/webkit/fileapi/file_system_context.cc File ...
7 years, 11 months ago (2013-01-18 21:33:14 UTC) #8
kinuko
https://codereview.chromium.org/11787028/diff/55003/chrome/browser/chromeos/drive/drive_task_executor.cc File chrome/browser/chromeos/drive/drive_task_executor.cc (right): https://codereview.chromium.org/11787028/diff/55003/chrome/browser/chromeos/drive/drive_task_executor.cc#newcode51 chrome/browser/chromeos/drive/drive_task_executor.cc:51: url != file_urls.end(); ++url) { nit: indent https://codereview.chromium.org/11787028/diff/55003/webkit/fileapi/external_mount_points.cc File ...
7 years, 11 months ago (2013-01-21 06:57:33 UTC) #9
tbarzic
https://chromiumcodereview.appspot.com/11787028/diff/55003/chrome/browser/chromeos/drive/drive_task_executor.cc File chrome/browser/chromeos/drive/drive_task_executor.cc (right): https://chromiumcodereview.appspot.com/11787028/diff/55003/chrome/browser/chromeos/drive/drive_task_executor.cc#newcode51 chrome/browser/chromeos/drive/drive_task_executor.cc:51: url != file_urls.end(); ++url) { On 2013/01/21 06:57:33, kinuko ...
7 years, 11 months ago (2013-01-22 21:30:56 UTC) #10
kinuko
There might still be some oversight but I think this lgtm. Thanks for working on ...
7 years, 11 months ago (2013-01-23 03:43:53 UTC) #11
tbarzic
+benwells for chrome/browser/extensions/ +brettw for webkit/tools/test_shell/ (and content/content_tests.gypi)
7 years, 11 months ago (2013-01-23 18:02:54 UTC) #12
benwells
c/b/e/ lgtm
7 years, 11 months ago (2013-01-24 01:03:46 UTC) #13
brettw
test shell lgtm rubberstamp
7 years, 11 months ago (2013-01-24 05:31:19 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tbarzic@chromium.org/11787028/74013
7 years, 11 months ago (2013-01-24 18:26:39 UTC) #15
commit-bot: I haz the power
7 years, 11 months ago (2013-01-24 22:01:53 UTC) #16
Message was sent while issue was closed.
Change committed as 178664

Powered by Google App Engine
This is Rietveld 408576698