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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 10198003: gdata: Get rid of GetGDataCacheTmpDirectory() and friends (Closed)

Created:
8 years, 8 months ago by satorux1
Modified:
8 years, 8 months ago
Reviewers:
achuithb
CC:
chromium-reviews, nkostylev+watch_chromium.org, achuith+watch_chromium.org, mihaip+watch_chromium.org, Aaron Boodman, rginda+watch_chromium.org, rdsmith+dwatch_chromium.org, stevenjb+watch_chromium.org, davemoore+watch_chromium.org
Visibility:
Public.

Description

gdata: Get rid of GetGDataCacheTmpDirectory() and friends Replace them with GetCacheDirectoryPath(), in favor of less public member functions from GDataFileSystem class, which is overly large. Wanted to make them just utility functions in gdata_util.cc but turned out we needed Profile* to compute these directory paths. BUG=chromium-os:29813 TEST=compiles Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=133639

Patch Set 1 #

Patch Set 2 : fix #

Total comments: 2

Patch Set 3 : fix infinite recursion #

Total comments: 2

Patch Set 4 : added a range check #

Patch Set 5 : rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+53 lines, -72 lines) Patch
M chrome/browser/chromeos/extensions/file_browser_private_api.cc View 1 chunk +4 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/gdata/gdata_file_system.h View 1 2 3 4 2 chunks +6 lines, -24 lines 0 comments Download
M chrome/browser/chromeos/gdata/gdata_file_system.cc View 1 2 3 4 10 chunks +21 lines, -33 lines 0 comments Download
M chrome/browser/chromeos/gdata/gdata_files.h View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/chromeos/gdata/gdata_files.cc View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/gdata/gdata_sync_client.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/chromeos/gdata/gdata_sync_client_unittest.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/chromeos/gdata/gdata_system_service.cc View 1 chunk +5 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/gdata/gdata_util.cc View 1 2 3 4 2 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/gdata/mock_gdata_file_system.h View 1 2 3 4 1 chunk +2 lines, -5 lines 0 comments Download
M chrome/browser/download/download_file_picker_chromeos.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/download/save_package_file_picker_chromeos.cc View 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 7 (0 generated)
satorux1
8 years, 8 months ago (2012-04-23 23:04:29 UTC) #1
achuithb
http://codereview.chromium.org/10198003/diff/2001/chrome/browser/chromeos/gdata/gdata_file_system.cc File chrome/browser/chromeos/gdata/gdata_file_system.cc (right): http://codereview.chromium.org/10198003/diff/2001/chrome/browser/chromeos/gdata/gdata_file_system.cc#newcode3189 chrome/browser/chromeos/gdata/gdata_file_system.cc:3189: return GetCacheDirectoryPath(sub_dir_type); I'm confused?
8 years, 8 months ago (2012-04-23 23:41:13 UTC) #2
satorux1
http://codereview.chromium.org/10198003/diff/2001/chrome/browser/chromeos/gdata/gdata_file_system.cc File chrome/browser/chromeos/gdata/gdata_file_system.cc (right): http://codereview.chromium.org/10198003/diff/2001/chrome/browser/chromeos/gdata/gdata_file_system.cc#newcode3189 chrome/browser/chromeos/gdata/gdata_file_system.cc:3189: return GetCacheDirectoryPath(sub_dir_type); On 2012/04/23 23:41:13, achuith.bhandarkar wrote: > I'm ...
8 years, 8 months ago (2012-04-23 23:43:00 UTC) #3
achuithb
lgtm with a question http://codereview.chromium.org/10198003/diff/10001/chrome/browser/chromeos/gdata/gdata_file_system.cc File chrome/browser/chromeos/gdata/gdata_file_system.cc (right): http://codereview.chromium.org/10198003/diff/10001/chrome/browser/chromeos/gdata/gdata_file_system.cc#newcode3189 chrome/browser/chromeos/gdata/gdata_file_system.cc:3189: return cache_paths_[sub_dir_type]; Are there any ...
8 years, 8 months ago (2012-04-23 23:48:01 UTC) #4
satorux1
http://codereview.chromium.org/10198003/diff/10001/chrome/browser/chromeos/gdata/gdata_file_system.cc File chrome/browser/chromeos/gdata/gdata_file_system.cc (right): http://codereview.chromium.org/10198003/diff/10001/chrome/browser/chromeos/gdata/gdata_file_system.cc#newcode3189 chrome/browser/chromeos/gdata/gdata_file_system.cc:3189: return cache_paths_[sub_dir_type]; On 2012/04/23 23:48:02, achuith.bhandarkar wrote: > Are ...
8 years, 8 months ago (2012-04-24 00:04:36 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/satorux@chromium.org/10198003/11001
8 years, 8 months ago (2012-04-24 00:12:10 UTC) #6
commit-bot: I haz the power
8 years, 8 months ago (2012-04-24 00:12:16 UTC) #7 
Can't apply patch for file chrome/browser/chromeos/gdata/gdata_file_system.h.
While running patch -p1 --forward --force;
patching file chrome/browser/chromeos/gdata/gdata_file_system.h
Hunk #1 succeeded at 367 with fuzz 2 (offset 10 lines).
Hunk #2 FAILED at 440.
1 out of 2 hunks FAILED -- saving rejects to file
chrome/browser/chromeos/gdata/gdata_file_system.h.rej

Powered by Google App Engine
This is Rietveld 408576698