[File Manager] Add/remove Google Drive folder in the open File Manager when Drive enabled/disabled …

[File Manager] Add/remove Google Drive folder in the open File Manager when Drive enabled/disabled in settings


BUG=127626
TEST=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=137102

[Vladislav Kaznacheev, 2012-05-14 08:11:32]

Hi,

Antony, please review a small change to fileBrowserPrivate.json.

Sergey, please review the rest of the patch.

Thanks!

  Vlad

[Vladislav Kaznacheev, 2012-05-14 08:27:18]

Adding Dmitry

[dgozman, 2012-05-14 08:52:54]

https://chromiumcodereview.appspot.com/10391103/diff/1/chrome/browser/resourc...
File chrome/browser/resources/file_manager/js/directory_model.js (right):

https://chromiumcodereview.appspot.com/10391103/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/directory_model.js:1049: callback();
I hope, you've checked that callback was never null.

https://chromiumcodereview.appspot.com/10391103/diff/1/chrome/browser/resourc...
File chrome/browser/resources/file_manager/js/file_manager.js (right):

https://chromiumcodereview.appspot.com/10391103/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/file_manager.js:2617: { driveEnabled:
str('ENABLE_GDATA') == '1' };
Do we still need ENABLE_GDATA?

https://chromiumcodereview.appspot.com/10391103/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/file_manager.js:4473: style.href =
'css/gdrive_welcome.css';
This means, you want to remove gdrive_welcome.css from main.html?

[Vladislav Kaznacheev, 2012-05-14 09:45:42]

PTAL

https://chromiumcodereview.appspot.com/10391103/diff/1/chrome/browser/resourc...
File chrome/browser/resources/file_manager/js/directory_model.js (right):

https://chromiumcodereview.appspot.com/10391103/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/directory_model.js:1049: callback();
Yes, of course. There are only 3 call sites, all with meaningful callbacks.
On 2012/05/14 08:52:54, dgozman wrote:
> I hope, you've checked that callback was never null.

https://chromiumcodereview.appspot.com/10391103/diff/1/chrome/browser/resourc...
File chrome/browser/resources/file_manager/js/file_manager.js (right):

https://chromiumcodereview.appspot.com/10391103/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/file_manager.js:2617: { driveEnabled:
str('ENABLE_GDATA') == '1' };
We want to know if GData is enabled or not when we are loading. If we wait until
an async API call return we are going to have UI flicker.

On 2012/05/14 08:52:54, dgozman wrote:
> Do we still need ENABLE_GDATA?

https://chromiumcodereview.appspot.com/10391103/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/file_manager.js:4473: style.href =
'css/gdrive_welcome.css';
I am not adding this code, just moving it from line 4484. And gdrive_welcome.css
was never in main.html.

On 2012/05/14 08:52:54, dgozman wrote:
> This means, you want to remove gdrive_welcome.css from main.html?

[dgozman, 2012-05-14 09:56:09]

GData treatment becoming more and more hairy...

LGTM though

https://chromiumcodereview.appspot.com/10391103/diff/1/chrome/browser/resourc...
File chrome/browser/resources/file_manager/js/file_manager.js (right):

https://chromiumcodereview.appspot.com/10391103/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/file_manager.js:4473: style.href =
'css/gdrive_welcome.css';
On 2012/05/14 09:45:42, Vladislav Kaznacheev wrote:
> I am not adding this code, just moving it from line 4484. And
gdrive_welcome.css
> was never in main.html.
> 
> On 2012/05/14 08:52:54, dgozman wrote:
> > This means, you want to remove gdrive_welcome.css from main.html?
> 

Missed this.

[Vladislav Kaznacheev, 2012-05-14 10:09:51]

I agree with this completely. Once we are done with M20 merging we should
introduce an abstraction for that (something like FileAccessController)

Thanks for the review!

> GData treatment becoming more and more hairy...

[asargent_no_longer_on_chrome, 2012-05-14 16:56:41]

file_browser_private.json LGTM