Get rid of the fake Drive/root entry.

Get rid of the fake Drive/root entry.

This patch removes the fake Drive/root entry, which was a hack. Along the way, the labels got moved away from the fake entries.

TEST=Tested manually. Partly browser tests.
BUG=334512
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=245441

[mtomasz, 2014-01-15 06:39:27]

@yoshiki: PTAL. Thanks.

[yoshiki, 2014-01-15 07:33:13]

https://codereview.chromium.org/135753003/diff/1/chrome/browser/resources/fil...
File chrome/browser/resources/file_manager/background/js/volume_manager.js
(right):

https://codereview.chromium.org/135753003/diff/1/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/background/js/volume_manager.js:71:
Object.seal(this);
Since you're changing this class into non-immutable, please make the const
properties private and add getters of them to prevent an unexpected change from
outside.

https://codereview.chromium.org/135753003/diff/1/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/background/js/volume_manager.js:80: return
this.root.toURL() +
I'm not sure but this.root seems null-able, so please add null-check before.

https://codereview.chromium.org/135753003/diff/1/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/background/js/volume_manager.js:92:
VolumeInfo.prototype.resolveDisplayRoot = function(onSuccess, onFailure) {
When on non-drive, can't we just use this.root?

https://codereview.chromium.org/135753003/diff/1/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/background/js/volume_manager.js:101:
console.log('found', directoryEntry);
nit: remove

https://codereview.chromium.org/135753003/diff/1/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/background/js/volume_manager.js:106:
console.log('not found', fileError);
ditto

https://codereview.chromium.org/135753003/diff/1/chrome/browser/resources/fil...
File chrome/browser/resources/file_manager/foreground/js/directory_tree.js
(right):

https://codereview.chromium.org/135753003/diff/1/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/foreground/js/directory_tree.js:553:
DirectoryTree.prototype.updateSubDirectories = function(
Could you change the method name? Or change the name of
DirectoryItem.updateSubDirectories? Before they were same name and had same
interface, but now not. Different name is better to prevent confusion.

[mtomasz, 2014-01-16 01:31:58]

Thanks. PTAL.

https://codereview.chromium.org/135753003/diff/1/chrome/browser/resources/fil...
File chrome/browser/resources/file_manager/background/js/volume_manager.js
(right):

https://codereview.chromium.org/135753003/diff/1/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/background/js/volume_manager.js:71:
Object.seal(this);
On 2014/01/15 07:33:13, yoshiki wrote:
> Since you're changing this class into non-immutable, please make the const
> properties private and add getters of them to prevent an unexpected change
from
> outside.

Done.

https://codereview.chromium.org/135753003/diff/1/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/background/js/volume_manager.js:80: return
this.root.toURL() +
On 2014/01/15 07:33:13, yoshiki wrote:
> I'm not sure but this.root seems null-able, so please add null-check before.

In general I think we should not have a volumeInfo in case of failure in
mounting. We wouldn't have a problem that root is null. But, since it is not
related to this patch, and this patch doesn't change behavior of
getDisplayRootURL, let me investigate and fix it separately.

https://codereview.chromium.org/135753003/diff/1/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/background/js/volume_manager.js:92:
VolumeInfo.prototype.resolveDisplayRoot = function(onSuccess, onFailure) {
On 2014/01/15 07:33:13, yoshiki wrote:
> When on non-drive, can't we just use this.root?

Done.

https://codereview.chromium.org/135753003/diff/1/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/background/js/volume_manager.js:101:
console.log('found', directoryEntry);
On 2014/01/15 07:33:13, yoshiki wrote:
> nit: remove

Done.

https://codereview.chromium.org/135753003/diff/1/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/background/js/volume_manager.js:106:
console.log('not found', fileError);
On 2014/01/15 07:33:13, yoshiki wrote:
> ditto

Done.

https://codereview.chromium.org/135753003/diff/1/chrome/browser/resources/fil...
File chrome/browser/resources/file_manager/foreground/js/directory_tree.js
(right):

https://codereview.chromium.org/135753003/diff/1/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/foreground/js/directory_tree.js:553:
DirectoryTree.prototype.updateSubDirectories = function(
On 2014/01/15 07:33:13, yoshiki wrote:
> Could you change the method name? Or change the name of
> DirectoryItem.updateSubDirectories? Before they were same name and had same
> interface, but now not. Different name is better to prevent confusion.

There is no defined interface @interface, not this method has @override, so I
think it is not so confusing. I think both methods do exactly the same, just
different way, so I think having the same method name is legit to me. WDYT?

[yoshiki, 2014-01-17 01:57:51]

lgtm

https://codereview.chromium.org/135753003/diff/1/chrome/browser/resources/fil...
File chrome/browser/resources/file_manager/background/js/volume_manager.js
(right):

https://codereview.chromium.org/135753003/diff/1/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/background/js/volume_manager.js:80: return
this.root.toURL() +
On 2014/01/16 01:31:58, mtomasz wrote:
> On 2014/01/15 07:33:13, yoshiki wrote:
> > I'm not sure but this.root seems null-able, so please add null-check before.
> 
> In general I think we should not have a volumeInfo in case of failure in
> mounting. We wouldn't have a problem that root is null. But, since it is not
> related to this patch, and this patch doesn't change behavior of
> getDisplayRootURL, let me investigate and fix it separately.

Ok. Got it.

https://codereview.chromium.org/135753003/diff/1/chrome/browser/resources/fil...
File chrome/browser/resources/file_manager/foreground/js/directory_tree.js
(right):

https://codereview.chromium.org/135753003/diff/1/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/foreground/js/directory_tree.js:553:
DirectoryTree.prototype.updateSubDirectories = function(
On 2014/01/16 01:31:58, mtomasz wrote:
> On 2014/01/15 07:33:13, yoshiki wrote:
> > Could you change the method name? Or change the name of
> > DirectoryItem.updateSubDirectories? Before they were same name and had same
> > interface, but now not. Different name is better to prevent confusion.
> 
> There is no defined interface @interface, not this method has @override, so I
> think it is not so confusing. I think both methods do exactly the same, just
> different way, so I think having the same method name is legit to me. WDYT?

OK. I agree with you.

[commit-bot: I haz the power, 2014-01-17 02:37:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mtomasz@chromium.org/135753003/60001

[commit-bot: I haz the power, 2014-01-17 06:35:38]

Change committed as 245441