Refactoring file manager: moving volume mounting related code to a separate class.

Refactoring file manager: moving volume mounting related code to a separate class.

Reason:
- Remove some complexity from file_manager.js
- Let FileTransferManager to mount gdata not having dependancy on FileManager.

BUG=127216
TEST=

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

[SeRya, 2012-05-15 08:23:44]

This is raw change not intended to commit at this stage. Please have a look
paying attention to code structure rather than details.

[Vladislav Kaznacheev, 2012-05-15 11:11:54]

The general direction makes sense to me. Some concerns in comments.

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

https://chromiumcodereview.appspot.com/10310163/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/css/file_manager.css:1176: [gdata='error']
#unmounted-panel > .gdata.error,
I introduced 'error' and 'retry' attribute for a reason, they are not always
present together.

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

https://chromiumcodereview.appspot.com/10310163/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/directory_model.js:67:
this.updateRoots(function() {
Calling updateRoots from the constructor somehow feels unnatural. Adding a
listener from a callback is strange too, at least it needs a comment.

https://chromiumcodereview.appspot.com/10310163/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/directory_model.js:806: * Creates an
object wich could cay wether directory has changed while it has
"cay", "canncelled" = typo?

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

https://chromiumcodereview.appspot.com/10310163/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/file_manager.js:1318: * @param
{boolean} invokeHandler Whether to invoke the default handler on
We do need the |invokeHandler| parameter. I cannot see how you kept the
functionality it provided.

[dgozman, 2012-05-15 12:57:31]

I like this, especially success/error callbacks for mount/unmount operations.

Will have a close look at finished CL.

[SeRya, 2012-05-15 18:16:20]

A lot of bugs fixed, works well. The only bug I found and not fixed so far is
flashing screen when gallery auto-opens on gdata.

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

https://chromiumcodereview.appspot.com/10310163/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/css/file_manager.css:1176: [gdata='error']
#unmounted-panel > .gdata.error,
Thank you for the point. I goning to control visibility of the retry button with
the hidden attribute.

On 2012/05/15 11:11:54, Vladislav Kaznacheev wrote:
> I introduced 'error' and 'retry' attribute for a reason, they are not always
> present together.

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

https://chromiumcodereview.appspot.com/10310163/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/file_manager.js:1318: * @param
{boolean} invokeHandler Whether to invoke the default handler on
On 2012/05/15 11:11:54, Vladislav Kaznacheev wrote:
> We do need the |invokeHandler| parameter. I cannot see how you kept the
> functionality it provided.

|invokeHandler| was inlined in place. I added invokeHandler var to help
understand how code changed.

[rginda, 2012-05-15 19:51:15]

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
File chrome/browser/resources/file_manager/js/volume_manager.js (right):

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/volume_manager.js:6: * FileManager
constructor.
Copypasta documentation here.  And the rest of the file has no documentation at
all.

[SeRya, 2012-05-16 13:45:43]

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
File chrome/browser/resources/file_manager/js/volume_manager.js (right):

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/volume_manager.js:6: * FileManager
constructor.
On 2012/05/15 19:51:15, rginda wrote:
> Copypasta documentation here.  And the rest of the file has no documentation
at
> all.

Robert, I uploaded this CL for preliminary code review. I din't bother with
comments yet. Gonna do it later.

[dgozman, 2012-05-16 14:31:45]

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
File chrome/browser/resources/file_manager/js/directory_model.js (right):

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/directory_model.js:805: * Creates an
object wich could cay wether directory has changed while it has
typo: cay -> say

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/directory_model.js:807: * if the used
change current directory.
typo: used -> user

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/directory_model.js:837: // This is
incorrect here.
I agree that this is incorrect. Fix?

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/directory_model.js:1089: * @param
{functioni?} opt_callback Called when all roots are resolved.
typo: functioni?

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/directory_model.js:1099: opt_callback
&& opt_callback();
I like the |if| construction much more.

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/directory_model.js:1370: break;
I think, you mean |return| instead of |break| here and below.

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/directory_model.js:1372: case
DirectoryModel.RootType.REMOVABLE:
What about RootType.ARCHIVE?

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/directory_model.js:1374:
this.root_.getDirectory(this.dm_.getCurrentDirPath(), {},
Why not use dm_.getCurrentDirectory ?

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
File chrome/browser/resources/file_manager/js/file_manager.js (right):

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/file_manager.js:252:
this.filesystemChanngeHandler_ =
typo here and below: chaNNge

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/file_manager.js:1779: listItem &&
listItem.setAttribute('disabled');
Don't like this syntax.

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/file_manager.js:2163:
props.filter(function(p) {return p && !p.availableOffline}).length == 0;
- long line
- this actually should be |!(p && p.availableOffline)|

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
File chrome/browser/resources/file_manager/js/volume_manager.js (right):

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/volume_manager.js:57: return
VolumeManager.instance_ = VolumeManager.instance_ || new VolumeManager();
long line

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/volume_manager.js:108: if
(event.mountPath) {
check event.status == 'success' here?

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/volume_manager.js:109: var requestKey =
this.makeRequestKey_('mount', event.mountType, event.sourcePath);
long line

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/volume_manager.js:126:
this.finishRequest_(this.makeRequestKey_('mount', '', mountPath), status);
'mount' -> 'unmount' ?

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/volume_manager.js:138: else if
(event.eventType == 'unmount' && event.status == 'success')
event.status == 'success' two lines above

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/volume_manager.js:161:
VolumeManager.prototype.makeRequestKey_ = function(requestType, mountType,
opt_mountPath) {
long line

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/volume_manager.js:188:
VolumeManager.prototype.getMountCondition = function(mountPath) {
I'd suggest to rename this to getMountStatus.

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/volume_manager.js:201: * Unmounts
device.
indentation

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/volume_manager.js:204:
VolumeManager.prototype.unmount = function(mountPath, successCallback,
errorCallback) {
long line

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/volume_manager.js:228:
VolumeManager.prototype.makeUrl_ = function(path) {
Move this method to util.js.

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/volume_manager.js:278:
callbacks[i].apply(self, args);
self -> null

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
File chrome/browser/resources/file_manager/main.html (right):

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/main.html:71: <script
src="js/volume_manager.js"></script>
You have to change main_scripts.js too.

[SeRya, 2012-05-18 11:31:48]

Now this CL is a commit candidate so please review it as carefully as usual.

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
File chrome/browser/resources/file_manager/js/directory_model.js (right):

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/directory_model.js:1370: break;
On 2012/05/16 14:31:46, dgozman wrote:
> I think, you mean |return| instead of |break| here and below.

Break is correct. The code below need to be executed.

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/directory_model.js:1372: case
DirectoryModel.RootType.REMOVABLE:
On 2012/05/16 14:31:46, dgozman wrote:
> What about RootType.ARCHIVE?

I assume they never change so we don't need to watch it.

https://chromiumcodereview.appspot.com/10310163/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/js/directory_model.js:1374:
this.root_.getDirectory(this.dm_.getCurrentDirPath(), {},
On 2012/05/16 14:31:46, dgozman wrote:
> Why not use dm_.getCurrentDirectory ?

getCurrentDirectory not always returns a valid Entry. Especially if GDATA just
mounted. Actually I believe we should get rid of getCurrentDirectory at all,
only let to ask path. Complexity related to fake entries should be encapsulated.

[dgozman, 2012-05-18 14:17:19]

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
File chrome/browser/resources/file_manager/js/directory_model.js (left):

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/directory_model.js:962: // Usually,
leaf does not exist, because it's just a suggested file name.
See this comment.

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
File chrome/browser/resources/file_manager/js/directory_model.js (right):

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/directory_model.js:968:
DirectoryModel.prototype.setupPath = function(path, opt_loadedCallback,
Two problems here:
1. (optional) This method still can open something above roots
(chrome://files/$/removable)
2. (mandatory) Many times, we have a suggested file name passed here. In this
case, we should open the parent directory, and put file name to the input. You
broke this.

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/directory_model.js:985:
resolveCallback(directoryEntry.fillPath, '', true);
typo: fillPath

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/directory_model.js:990: var INITIAL =
true;
Add todo to rename initial to something like put_in_history through the all
code.

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/directory_model.js:999:
resolveCallback(path, '', false);
false -> !EXISTS

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/directory_model.js:1003: function
netherFileNorEntry(error) {
typo: nether -> neither

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/directory_model.js:1008: function
getParentFiled(error) {
typo: getParentFiled -> getParentFailed

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
File chrome/browser/resources/file_manager/js/volume_manager.js (right):

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:67: * Time in
milliseconds whet we wait a respone for. If no response on
typo: whet -> that

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:125:
console.error('GData is not extpected initially mounted');
typo: extpected

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:128: function(volume)
{
strange indentation

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:176: if (!expectedly
&& mountPath in this.mountedVolumes_) {
Check for status=success here.

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:178: var e = new
cr.Event('unexpectedly-unmounted');
I'd suggest to rename this to 'externally-unmounted'

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:194: else if
(event.eventType == 'unmount' && event.status == 'success')
Already checked for success 3 lines above.

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:212:
function(metadata) {
strange indentation

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:287:
errorCallback(VolumeManager.Error.NOT_NOUNTED);
typo: NOT_NOUNTED

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:298: * @return
{VolumeManager.Error?} Retuns mount error code
typo: Retuns

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:303: return
this.getVolumeInfo_(mountPath).status;
status -> error

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:312: var status =
this.getMountError(mountPath);
status -> error

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:322:
this.validateMountPath_(mountPath);
You can remove most of these checks, and just leave one in getVolumeInfo_.

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:343:
VolumeManager.prototype.makeUrl_ = function(path) {
Move to util.js

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:391: * Called if not
response received in |TIMEOUT|.
typo: not -> no

[SeRya, 2012-05-21 08:06:36]

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
File chrome/browser/resources/file_manager/js/directory_model.js (right):

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/directory_model.js:968:
DirectoryModel.prototype.setupPath = function(path, opt_loadedCallback,
On 2012/05/18 14:17:19, dgozman wrote:
> Two problems here:
> 1. (optional) This method still can open something above roots
> (chrome://files/$/removable)
> 2. (mandatory) Many times, we have a suggested file name passed here. In this
> case, we should open the parent directory, and put file name to the input. You
> broke this.

Fixed. Tested open and save dialogs with existing and non-existing suggested
file names. Tested entering system directory to URL.

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/directory_model.js:990: var INITIAL =
true;
On 2012/05/18 14:17:19, dgozman wrote:
> Add todo to rename initial to something like put_in_history through the all
> code.

I don't think this is a good idea. For now we indeed mostly need to
differentiate directory change came from setupPath because they shouln't be put
to the history. But also I use it while long initialization to stop it if the
user changed directory manually.

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/directory_model.js:999:
resolveCallback(path, '', false);
On 2012/05/18 14:17:19, dgozman wrote:
> false -> !EXISTS

Done.

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
File chrome/browser/resources/file_manager/js/volume_manager.js (right):

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:67: * Time in
milliseconds whet we wait a respone for. If no response on
On 2012/05/18 14:17:19, dgozman wrote:
> typo: whet -> that

Done.

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:125:
console.error('GData is not extpected initially mounted');
On 2012/05/18 14:17:19, dgozman wrote:
> typo: extpected

Done.

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:128: function(volume)
{
On 2012/05/18 14:17:19, dgozman wrote:
> strange indentation

Done.

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:176: if (!expectedly
&& mountPath in this.mountedVolumes_) {
On 2012/05/18 14:17:19, dgozman wrote:
> Check for status=success here.

Done.

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:178: var e = new
cr.Event('unexpectedly-unmounted');
On 2012/05/18 14:17:19, dgozman wrote:
> I'd suggest to rename this to 'externally-unmounted'

Done.

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:194: else if
(event.eventType == 'unmount' && event.status == 'success')
On 2012/05/18 14:17:19, dgozman wrote:
> Already checked for success 3 lines above.

Done.

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:212:
function(metadata) {
On 2012/05/18 14:17:19, dgozman wrote:
> strange indentation

Done.

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:287:
errorCallback(VolumeManager.Error.NOT_NOUNTED);
On 2012/05/18 14:17:19, dgozman wrote:
> typo: NOT_NOUNTED

Done.

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:298: * @return
{VolumeManager.Error?} Retuns mount error code
On 2012/05/18 14:17:19, dgozman wrote:
> typo: Retuns

Done.

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:303: return
this.getVolumeInfo_(mountPath).status;
On 2012/05/18 14:17:19, dgozman wrote:
> status -> error

Done.

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:312: var status =
this.getMountError(mountPath);
On 2012/05/18 14:17:19, dgozman wrote:
> status -> error

Done.

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:322:
this.validateMountPath_(mountPath);
On 2012/05/18 14:17:19, dgozman wrote:
> You can remove most of these checks, and just leave one in getVolumeInfo_.

Done.

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:322:
this.validateMountPath_(mountPath);
On 2012/05/18 14:17:19, dgozman wrote:
> You can remove most of these checks, and just leave one in getVolumeInfo_.

Done.

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:343:
VolumeManager.prototype.makeUrl_ = function(path) {
On 2012/05/18 14:17:19, dgozman wrote:
> Move to util.js

Done.

https://chromiumcodereview.appspot.com/10310163/diff/17001/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:391: * Called if not
response received in |TIMEOUT|.
On 2012/05/18 14:17:19, dgozman wrote:
> typo: not -> no

Done.

[dgozman, 2012-05-21 13:32:16]

LGTM

https://chromiumcodereview.appspot.com/10310163/diff/22002/chrome/browser/res...
File chrome/browser/resources/file_manager/js/volume_manager.js (right):

https://chromiumcodereview.appspot.com/10310163/diff/22002/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:175: var expectedly =
requestKey in this.requests_;
expectedly -> externally

[SeRya, 2012-05-22 09:10:50]

Vlad, I've just merged with your CL. PLease, have a look.

https://chromiumcodereview.appspot.com/10310163/diff/22002/chrome/browser/res...
File chrome/browser/resources/file_manager/js/volume_manager.js (right):

https://chromiumcodereview.appspot.com/10310163/diff/22002/chrome/browser/res...
chrome/browser/resources/file_manager/js/volume_manager.js:175: var expectedly =
requestKey in this.requests_;
On 2012/05/21 13:32:16, dgozman wrote:
> expectedly -> externally

Changed to 'requested'.

[Vladislav Kaznacheev, 2012-05-22 09:48:29]

LGTM with a nit

https://chromiumcodereview.appspot.com/10310163/diff/18013/chrome/browser/res...
File chrome/browser/resources/file_manager/css/file_manager.css (right):

https://chromiumcodereview.appspot.com/10310163/diff/18013/chrome/browser/res...
chrome/browser/resources/file_manager/css/file_manager.css:26: /*display:
-webkit-box;*/
Commented property looks confusing

https://chromiumcodereview.appspot.com/10310163/diff/18013/chrome/browser/res...
File chrome/browser/resources/file_manager/js/file_manager.js (right):

https://chromiumcodereview.appspot.com/10310163/diff/18013/chrome/browser/res...
chrome/browser/resources/file_manager/js/file_manager.js:337: var url =
imageURL.substr(0, imageURL.lastIndexOf('/'));
Lets rename url to directoryUrl