This adds some GData private API to the file manager

This adds some GData private API to the file manager
for fetching the GData file properties and for pinning files.

BUG=chromium-os:27030, chromeium-os:27078
TEST=none


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

[Greg Spencer (Chromium), 2012-02-29 23:26:02]

I just realized that I'd really rather have a FindFilesByPath function on the
Filesystem (which doesn't exist yet) so I can just make one jump across threads
to get the entire list.  I'll see about adding that.  In the mean time, this
seems to do what we need.

[achuithb, 2012-02-29 23:33:06]

https://chromiumcodereview.appspot.com/9545006/diff/1/chrome/browser/chromeos...
File chrome/browser/chromeos/extensions/file_browser_private_api.cc (right):

https://chromiumcodereview.appspot.com/9545006/diff/1/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/file_browser_private_api.cc:1870:
base::ListValue* file_info = new base::ListValue;
Could you please add a DCHECK for CurrentlyOn(BrowserThread::UI)?

https://chromiumcodereview.appspot.com/9545006/diff/1/chrome/browser/chromeos...
File chrome/browser/chromeos/gdata/gdata_file_system.h (right):

https://chromiumcodereview.appspot.com/9545006/diff/1/chrome/browser/chromeos...
chrome/browser/chromeos/gdata/gdata_file_system.h:97: const int pinned_state()
const { return pinned_state_; }
const int?

[zel, 2012-02-29 23:46:37]

https://chromiumcodereview.appspot.com/9545006/diff/1/chrome/browser/chromeos...
File chrome/browser/chromeos/extensions/file_browser_private_api.cc (right):

https://chromiumcodereview.appspot.com/9545006/diff/1/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/file_browser_private_api.cc:410:
property_dict->Set("authors", authors_list);
remove authors

https://chromiumcodereview.appspot.com/9545006/diff/1/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/file_browser_private_api.cc:1856:
GetLocalPathsOnFileThreadAndRunCallbackOnUIThread(
You don't need to move things to file thread for gdata entities.

Create utility function that would give you virtual_path back, something like:

    FilePath virtual_path;
    fileapi::FileSystemType type;
    GURL file_origin_url;
    if (!CrackFileSystemURL(file_url, &file_origin_url, &type,
                            &virtual_path)) {
      continue;
    }
    if (type != fileapi::kFileSystemTypeExternal) {
      NOTREACHED();
      continue;
    }


replace the same part in FileBrowserFunction::GetLocalPathsOnFileThread() with
that function.

https://chromiumcodereview.appspot.com/9545006/diff/1/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/file_browser_private_api.cc:1879:
scoped_refptr<gdata::ReadOnlyFindFileDelegate> find_delegate(
yeah, you need a better one here, one that extracts what you need instead of
accessing unsafe find_delegate->file()

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

https://chromiumcodereview.appspot.com/9545006/diff/1/chrome/browser/chromeos...
chrome/browser/chromeos/gdata/gdata_file_system.cc:193: const
ScopedVector<Author>& authors = doc->authors();
kill authors, sorry for the confusion

https://chromiumcodereview.appspot.com/9545006/diff/1/chrome/browser/chromeos...
File chrome/browser/chromeos/gdata/gdata_file_system.h (right):

https://chromiumcodereview.appspot.com/9545006/diff/1/chrome/browser/chromeos...
chrome/browser/chromeos/gdata/gdata_file_system.h:110: AuthorList authors_;
let's just drop authors for there altogether... we can add it later if needed

[Greg Spencer (Chromium), 2012-03-01 02:26:32]

OK, this should be closer to what Zel wanted.  Still hasn't been tested.

https://chromiumcodereview.appspot.com/9545006/diff/1/chrome/browser/chromeos...
File chrome/browser/chromeos/extensions/file_browser_private_api.cc (right):

https://chromiumcodereview.appspot.com/9545006/diff/1/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/file_browser_private_api.cc:410:
property_dict->Set("authors", authors_list);
On 2012/02/29 23:46:38, zel wrote:
> remove authors

Done.

https://chromiumcodereview.appspot.com/9545006/diff/1/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/file_browser_private_api.cc:1856:
GetLocalPathsOnFileThreadAndRunCallbackOnUIThread(
On 2012/02/29 23:46:38, zel wrote:
> You don't need to move things to file thread for gdata entities.
> 
> Create utility function that would give you virtual_path back, something like:
> 
>     FilePath virtual_path;
>     fileapi::FileSystemType type;
>     GURL file_origin_url;
>     if (!CrackFileSystemURL(file_url, &file_origin_url, &type,
>                             &virtual_path)) {
>       continue;
>     }
>     if (type != fileapi::kFileSystemTypeExternal) {
>       NOTREACHED();
>       continue;
>     }
> 
> 
> replace the same part in FileBrowserFunction::GetLocalPathsOnFileThread() with
> that function.

Done.

https://chromiumcodereview.appspot.com/9545006/diff/1/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/file_browser_private_api.cc:1870:
base::ListValue* file_info = new base::ListValue;
On 2012/02/29 23:33:06, achuith.bhandarkar wrote:
> Could you please add a DCHECK for CurrentlyOn(BrowserThread::UI)?

Actually, removed this because Zel says I don't need to do anything on the file
thread in between.  I added a check to make sure I was on the UI thread in
RunImpl though.

https://chromiumcodereview.appspot.com/9545006/diff/1/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/file_browser_private_api.cc:1879:
scoped_refptr<gdata::ReadOnlyFindFileDelegate> find_delegate(
On 2012/02/29 23:46:38, zel wrote:
> yeah, you need a better one here, one that extracts what you need instead of
> accessing unsafe find_delegate->file()

Done.

https://chromiumcodereview.appspot.com/9545006/diff/1/chrome/browser/chromeos...
File chrome/browser/chromeos/gdata/gdata_file_system.h (right):

https://chromiumcodereview.appspot.com/9545006/diff/1/chrome/browser/chromeos...
chrome/browser/chromeos/gdata/gdata_file_system.h:97: const int pinned_state()
const { return pinned_state_; }
On 2012/02/29 23:33:06, achuith.bhandarkar wrote:
> const int?

Haha! Yes, they're so much more constant this way.  (Removed)

https://chromiumcodereview.appspot.com/9545006/diff/1/chrome/browser/chromeos...
chrome/browser/chromeos/gdata/gdata_file_system.h:110: AuthorList authors_;
On 2012/02/29 23:46:38, zel wrote:
> let's just drop authors for there altogether... we can add it later if needed

Done.

[Greg Spencer (Chromium), 2012-03-01 02:31:47]

https://chromiumcodereview.appspot.com/9545006/diff/1003/chrome/browser/chrom...
File chrome/browser/chromeos/gdata/gdata_file_system.h (right):

https://chromiumcodereview.appspot.com/9545006/diff/1003/chrome/browser/chrom...
chrome/browser/chromeos/gdata/gdata_file_system.h:349: // Returns the
GDataFileSystemFactory instance.
This is different (even if Rietveld doesn't show it) because I deleted some kind
of control character (0x0001, I think) from the comment.

[zel, 2012-03-01 02:37:38]

missing fileBrowserPrivate.json updates

https://chromiumcodereview.appspot.com/9545006/diff/1003/chrome/browser/chrom...
File chrome/browser/chromeos/extensions/file_browser_private_api.cc (right):

https://chromiumcodereview.appspot.com/9545006/diff/1003/chrome/browser/chrom...
chrome/browser/chromeos/extensions/file_browser_private_api.cc:1920:
path_list->GetString(i, &virtual_path);
s/virtual_path/file_url/

[Greg Spencer (Chromium), 2012-03-01 19:15:50]

Now I'm just waiting for Satoru to finish his refactor of the base class so I
can write my tests.

On 2012/03/01 02:37:38, zel wrote:
> missing fileBrowserPrivate.json updates

Done.

https://chromiumcodereview.appspot.com/9545006/diff/1003/chrome/browser/chrom...
> chrome/browser/chromeos/extensions/file_browser_private_api.cc:1920:
> path_list->GetString(i, &virtual_path);
> s/virtual_path/file_url/

Done.

[zel, 2012-03-01 22:14:18]

please get everything from chrome/browser/chromeos/gdata/* into another CL

http://codereview.chromium.org/9545006/diff/9001/chrome/browser/chromeos/gdat...
File chrome/browser/chromeos/gdata/gdata_file_system.h (right):

http://codereview.chromium.org/9545006/diff/9001/chrome/browser/chromeos/gdat...
chrome/browser/chromeos/gdata/gdata_file_system.h:87: const GURL&
thumbnail_url() const { return edit_url_; }
thumbnail_url_

http://codereview.chromium.org/9545006/diff/9001/chrome/browser/chromeos/gdat...
chrome/browser/chromeos/gdata/gdata_file_system.h:88: const GURL& edit_url()
const { return thumbnail_url_; }
edit_url_

[zel, 2012-03-01 22:16:12]

http://codereview.chromium.org/9545006/diff/9001/chrome/browser/chromeos/exte...
File chrome/browser/chromeos/extensions/file_browser_private_api.cc (right):

http://codereview.chromium.org/9545006/diff/9001/chrome/browser/chromeos/exte...
chrome/browser/chromeos/extensions/file_browser_private_api.cc:440:
property_dict->SetString("editUrl", edit_url_.spec());
let's check if we even have it - only hosted doc items will have one

if (!edit_url_.is_empty())
  property_dict->SetString("editUrl", edit_url_.spec());

[Greg Spencer (Chromium), 2012-03-02 22:24:12]

FYI, this is updated with the pinning API.  Still working on the testing code.

[zel, 2012-03-02 22:32:10]

http://codereview.chromium.org/9545006/diff/15001/chrome/browser/chromeos/ext...
File chrome/browser/chromeos/extensions/file_browser_private_api.cc (right):

http://codereview.chromium.org/9545006/diff/15001/chrome/browser/chromeos/ext...
chrome/browser/chromeos/extensions/file_browser_private_api.cc:444:
base::FundamentalValue* pinned =
no need for pinned declaration, just use DictionaryValue::Set<Type>(...) below
as 

property_dict->SetBoolean("isPinned",
   static_cast<bool>(cache_state_ & gdata::GDataFile::CACHE_STATE_PINNED));

http://codereview.chromium.org/9545006/diff/15001/chrome/browser/chromeos/ext...
chrome/browser/chromeos/extensions/file_browser_private_api.cc:449:
base::FundamentalValue* present =
same as above

http://codereview.chromium.org/9545006/diff/15001/chrome/browser/chromeos/ext...
chrome/browser/chromeos/extensions/file_browser_private_api.cc:454:
base::FundamentalValue* dirty =
same as above

[Greg Spencer (Chromium), 2012-03-02 22:48:36]

On 2012/03/02 22:32:10, zel wrote:
> no need for pinned declaration, just use DictionaryValue::Set<Type>
> (...) below

I swear I looked for SetBoolean and didn't find it.  I thought it was weird at
the time too.

Fixed.

[zel, 2012-03-02 22:50:54]

On 2012/03/02 22:48:36, Greg Spencer (Chromium) wrote:
> On 2012/03/02 22:32:10, zel wrote:
> > no need for pinned declaration, just use DictionaryValue::Set<Type>
> > (...) below
> 
> I swear I looked for SetBoolean and didn't find it.  I thought it was weird at
> the time too.
> 
> Fixed.

 I don't see the fix.

[zel, 2012-03-03 00:47:59]

lgtm

http://codereview.chromium.org/9545006/diff/17003/chrome/browser/chromeos/ext...
File chrome/browser/chromeos/extensions/file_browser_private_api.cc (right):

http://codereview.chromium.org/9545006/diff/17003/chrome/browser/chromeos/ext...
chrome/browser/chromeos/extensions/file_browser_private_api.cc:464:
base::FundamentalValue* dirty =
one more left

[asargent_no_longer_on_chrome, 2012-03-03 00:49:20]

fileBrowserPrivate.json LGTM

[commit-bot: I haz the power, 2012-03-03 01:19:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gspencer@chromium.org/9545006/20005

[commit-bot: I haz the power, 2012-03-03 08:33:56]

Change committed as 124837