Issue 10692154: gdata: Remove IsCache/SetCache/ClearCache* functions from GDataCache

Issue 10692154: gdata: Remove IsCache/SetCache/ClearCache* functions from GDataCache (Closed)

Created:
8 years, 5 months ago by satorux1

Modified:
8 years, 5 months ago

Reviewers:
hashimoto

CC:
chromium-reviews, achuith+watch_chromium.org, oshima+watch_chromium.org, stevenjb+watch_chromium.org, nkostylev+watch_chromium.org, davemoore+watch_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

gdata: Remove IsCache/SetCache/ClearCache* functions from GDataCache These should belong to CacheEntry as non-static member functions. BUG=136625 TEST=out/Release/unit_tests --gtest_filter=GData* Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=146183

Patch Set 1 #

Total comments: 11

Patch Set 2 : rebased #

Patch Set 3 : address comments #

Created: 8 years, 5 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+185 lines, -155 lines)			Patch
M	chrome/browser/chromeos/gdata/gdata_cache.h	View	1	2 chunks	+36 lines, -51 lines	0 comments	Download
M	chrome/browser/chromeos/gdata/gdata_cache.cc	View	1 2	12 chunks	+23 lines, -21 lines	0 comments	Download
M	chrome/browser/chromeos/gdata/gdata_cache_metadata.cc	View	1	6 chunks	+7 lines, -9 lines	0 comments	Download
M	chrome/browser/chromeos/gdata/gdata_cache_unittest.cc	View	1 2	12 chunks	+103 lines, -63 lines	0 comments	Download
M	chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc	View	1 2	5 chunks	+16 lines, -11 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

hashimoto

lgtm with nits http://codereview.chromium.org/10692154/diff/1/chrome/browser/chromeos/gdata/gdata_cache.cc File chrome/browser/chromeos/gdata/gdata_cache.cc (right): http://codereview.chromium.org/10692154/diff/1/chrome/browser/chromeos/gdata/gdata_cache.cc#newcode1523 chrome/browser/chromeos/gdata/gdata_cache.cc:1523: cache_entry.SetPersistent(true); nit: SetPersistent(sub_dir_type == CACHE_TYPE_PERSISTENT) ? ...

8 years, 5 months ago (2012-07-11 11:08:04 UTC) #2

satorux1

8 years, 5 months ago (2012-07-11 19:54:42 UTC) #3

http://codereview.chromium.org/10692154/diff/1/chrome/browser/chromeos/gdata/...
File chrome/browser/chromeos/gdata/gdata_cache.cc (right):

http://codereview.chromium.org/10692154/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_cache.cc:1523:
cache_entry.SetPersistent(true);
On 2012/07/11 11:08:04, hashimoto wrote:
> nit: SetPersistent(sub_dir_type == CACHE_TYPE_PERSISTENT) ?

Nice. Done.

http://codereview.chromium.org/10692154/diff/1/chrome/browser/chromeos/gdata/...
File chrome/browser/chromeos/gdata/gdata_cache_unittest.cc (right):

http://codereview.chromium.org/10692154/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_cache_unittest.cc:94: // Converts
|cache_state| to a GDataCAche::CacheEntry.
On 2012/07/11 11:08:04, hashimoto wrote:
> nit: s/CAche/Cache/

Done.

http://codereview.chromium.org/10692154/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_cache_unittest.cc:99: }  // anonymous
namespace
On 2012/07/11 11:08:04, hashimoto wrote:
> Could you fix this while you are at this file?
> s/anonymous namespace/namespace/

Done.

http://codereview.chromium.org/10692154/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_cache_unittest.cc:99: }  // anonymous
namespace
On 2012/07/11 11:08:04, hashimoto wrote:
> Could you fix this while you are at this file?
> s/anonymous namespace/namespace/

Done.

http://codereview.chromium.org/10692154/diff/1/chrome/browser/chromeos/gdata/...
File chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc (right):

http://codereview.chromium.org/10692154/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc:125: // Converts
|cache_state| to a GDataCAche::CacheEntry.
On 2012/07/11 11:08:04, hashimoto wrote:
> nit: s/CAche/Cache/

Done.

http://codereview.chromium.org/10692154/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc:130: }  // anonymous
namespace
On 2012/07/11 11:08:04, hashimoto wrote:
> Could you fix this while you are at this file?
> s/anonymous namespace/namespace/

Done.

Expand Messages | Collapse Messages