gdata: Change permissions of cache persistent directory.

gdata: Change permissions of cache persistent directory.

The cache persistent directory needs to be accessible by the AVFS daemon
to allow mounting of archive files in the gdata cache.

BUG=chromium-os:28678
TEST=Verify the permissions of the cache persistent directory.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=132157

[kuan, 2012-04-13 02:12:13]

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

http://codereview.chromium.org/10072003/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_file_system.cc:219: PLOG(ERROR) << "Error
changing permissions of " << file_path.value();
if u want more informative log, u can append strerror(errno).

http://codereview.chromium.org/10072003/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_file_system.cc:3278:
gdata_cache_path_.Append(kGDataCachePersistentDir),
u can use cache_paths_[GDataRootDirectory::CACHE_TYPE_PERSISTENT].

[Ben Chan, 2012-04-13 03:05:21]

rebased to ToT and fixed one comment

https://chromiumcodereview.appspot.com/10072003/diff/1/chrome/browser/chromeo...
File chrome/browser/chromeos/gdata/gdata_file_system.cc (right):

https://chromiumcodereview.appspot.com/10072003/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/gdata/gdata_file_system.cc:219: PLOG(ERROR) << "Error
changing permissions of " << file_path.value();
On 2012/04/13 02:12:13, kuan wrote:
> if u want more informative log, u can append strerror(errno).

PLOG() actually does that but is preferred because it calls
safe_strerror(errno), which is multi-thread safe:

http://code.google.com/codesearch#OAMlx_jo-ck/src/base/logging.cc&type=cs&l=797

I notice that other code in gdata uses strerror(errno). we should consider
converting those to PLOG().

https://chromiumcodereview.appspot.com/10072003/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/gdata/gdata_file_system.cc:3278:
gdata_cache_path_.Append(kGDataCachePersistentDir),
On 2012/04/13 02:12:13, kuan wrote:
> u can use cache_paths_[GDataRootDirectory::CACHE_TYPE_PERSISTENT].

Done.

[kuan, 2012-04-13 03:30:59]

lgtm

https://chromiumcodereview.appspot.com/10072003/diff/1/chrome/browser/chromeo...
File chrome/browser/chromeos/gdata/gdata_file_system.cc (right):

https://chromiumcodereview.appspot.com/10072003/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/gdata/gdata_file_system.cc:219: PLOG(ERROR) << "Error
changing permissions of " << file_path.value();
On 2012/04/13 03:05:21, Ben Chan wrote:
> On 2012/04/13 02:12:13, kuan wrote:
> > if u want more informative log, u can append strerror(errno).
> 
> PLOG() actually does that but is preferred because it calls
> safe_strerror(errno), which is multi-thread safe:
> 
>
http://code.google.com/codesearch#OAMlx_jo-ck/src/base/logging.cc&type=cs&l=797
> 
> I notice that other code in gdata uses strerror(errno). we should consider
> converting those to PLOG().

i didn't know that, gd to know, thanks :)

[commit-bot: I haz the power, 2012-04-13 04:05:47]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[kuan, 2012-04-13 04:41:01]

LGTM.

[commit-bot: I haz the power, 2012-04-13 04:41:52]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[satorux1, 2012-04-13 06:09:23]

LGTM with a nit:

https://chromiumcodereview.appspot.com/10072003/diff/2002/chrome/browser/chro...
File chrome/browser/chromeos/gdata/gdata_file_system.cc (right):

https://chromiumcodereview.appspot.com/10072003/diff/2002/chrome/browser/chro...
chrome/browser/chromeos/gdata/gdata_file_system.cc:214: if
(chmod(file_path.value().c_str(), permissions) != 0) {
Please use HANDLE_EINTR when issuing a system call.

chmod can return EINTR:
http://pubs.opengroup.org/onlinepubs/009604499/functions/chmod.html

[Ben Chan, 2012-04-13 06:30:27]

https://chromiumcodereview.appspot.com/10072003/diff/2002/chrome/browser/chro...
File chrome/browser/chromeos/gdata/gdata_file_system.cc (right):

https://chromiumcodereview.appspot.com/10072003/diff/2002/chrome/browser/chro...
chrome/browser/chromeos/gdata/gdata_file_system.cc:214: if
(chmod(file_path.value().c_str(), permissions) != 0) {
On 2012/04/13 06:09:23, satorux1 wrote:
> Please use HANDLE_EINTR when issuing a system call.
> 
> chmod can return EINTR:
> http://pubs.opengroup.org/onlinepubs/009604499/functions/chmod.html

Done.

[commit-bot: I haz the power, 2012-04-13 06:30:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benchan@chromium.org/10072003/11002

[commit-bot: I haz the power, 2012-04-13 07:55:05]

Change committed as 132157