Add a method in DiskMountManager to refresh mount entries in addition to devices.

Add a DiskMountManager method to refresh mount entries in addition to devices.

Lack of the info caused Chrome OS Files.app to fail finding external storages
after browser-only restart (like that from about:flags or from browser crash.)

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

[kinaba, 2014-07-09 23:17:07]

Sorry for taking so long time but this is a continuation of
http://crrev.com/270958. PTAL.
@benchan: the whole patch.
@stevenjb: as a /chromeos OWNER.

[Ben Chan, 2014-07-10 00:07:12]

https://codereview.chromium.org/379743004/diff/1/chrome/browser/chromeos/file...
File chrome/browser/chromeos/file_manager/volume_manager.cc (right):

https://codereview.chromium.org/379743004/diff/1/chrome/browser/chromeos/file...
chrome/browser/chromeos/file_manager/volume_manager.cc:760: // Archives are
mounted after other type of volumes. See below.
"other type of volumes" -> "other types of volume"

https://codereview.chromium.org/379743004/diff/1/chrome/browser/chromeos/file...
chrome/browser/chromeos/file_manager/volume_manager.cc:776: if (!done[i]) {
this helps remove one indentation level

if (done[i])
  continue;

https://codereview.chromium.org/379743004/diff/1/chromeos/disks/disk_mount_ma...
File chromeos/disks/disk_mount_manager.cc (right):

https://codereview.chromium.org/379743004/diff/1/chromeos/disks/disk_mount_ma...
chromeos/disks/disk_mount_manager.cc:486: void RefreshEachDevice(const
std::vector<std::string>& devices, size_t i) {
i -> index

RefreshEachDevice seems like a misnomer. perhaps RefreshDeviceAtIndex?

https://codereview.chromium.org/379743004/diff/1/chromeos/disks/disk_mount_ma...
chromeos/disks/disk_mount_manager.cc:505: // Part of EnsureMountInfoRefrehsed().
typo: EnsureMountInfoRefrehsed -> EnsureMountInfoRefreshed

[stevenjb, 2014-07-10 00:53:21]

lgtm

https://codereview.chromium.org/379743004/diff/1/chrome/browser/chromeos/file...
File chrome/browser/chromeos/file_manager/volume_manager.cc (right):

https://codereview.chromium.org/379743004/diff/1/chrome/browser/chromeos/file...
chrome/browser/chromeos/file_manager/volume_manager.cc:773: // To check the
condition correctly in DoMountEvent, we care the order.
s/care/care about/

https://codereview.chromium.org/379743004/diff/1/chrome/browser/chromeos/file...
chrome/browser/chromeos/file_manager/volume_manager.cc:782: for (size_t parent =
0; parent < archives.size(); ++parent) {
Can't we start this with i+1, since done[0,i] will be true at this point?

https://codereview.chromium.org/379743004/diff/1/chrome/browser/chromeos/file...
chrome/browser/chromeos/file_manager/volume_manager.cc:787: parent = 0;  //
Search archives[parent]'s parent from the beginning.
parent = i + 1;

https://codereview.chromium.org/379743004/diff/1/chromeos/disks/disk_mount_ma...
File chromeos/disks/disk_mount_manager.h (right):

https://codereview.chromium.org/379743004/diff/1/chromeos/disks/disk_mount_ma...
chromeos/disks/disk_mount_manager.h:242: // Requests refreshing all the
information about mounted disks.
Update comment

[kinaba, 2014-07-10 01:36:32]

https://codereview.chromium.org/379743004/diff/1/chrome/browser/chromeos/file...
File chrome/browser/chromeos/file_manager/volume_manager.cc (right):

https://codereview.chromium.org/379743004/diff/1/chrome/browser/chromeos/file...
chrome/browser/chromeos/file_manager/volume_manager.cc:760: // Archives are
mounted after other type of volumes. See below.
On 2014/07/10 00:07:12, Ben Chan wrote:
> "other type of volumes" -> "other types of volume"

Done.

https://codereview.chromium.org/379743004/diff/1/chrome/browser/chromeos/file...
chrome/browser/chromeos/file_manager/volume_manager.cc:773: // To check the
condition correctly in DoMountEvent, we care the order.
On 2014/07/10 00:53:21, stevenjb wrote:
> s/care/care about/

Done.

https://codereview.chromium.org/379743004/diff/1/chrome/browser/chromeos/file...
chrome/browser/chromeos/file_manager/volume_manager.cc:776: if (!done[i]) {
On 2014/07/10 00:07:12, Ben Chan wrote:
> this helps remove one indentation level
> 
> if (done[i])
>   continue;
> 

Done.

https://codereview.chromium.org/379743004/diff/1/chrome/browser/chromeos/file...
chrome/browser/chromeos/file_manager/volume_manager.cc:782: for (size_t parent =
0; parent < archives.size(); ++parent) {
On 2014/07/10 00:53:21, stevenjb wrote:
> Can't we start this with i+1, since done[0,i] will be true at this point?

Good point. Thanks. Done.

https://codereview.chromium.org/379743004/diff/1/chrome/browser/chromeos/file...
chrome/browser/chromeos/file_manager/volume_manager.cc:787: parent = 0;  //
Search archives[parent]'s parent from the beginning.
On 2014/07/10 00:53:21, stevenjb wrote:
> parent = i + 1;

Done.

https://codereview.chromium.org/379743004/diff/1/chromeos/disks/disk_mount_ma...
File chromeos/disks/disk_mount_manager.cc (right):

https://codereview.chromium.org/379743004/diff/1/chromeos/disks/disk_mount_ma...
chromeos/disks/disk_mount_manager.cc:486: void RefreshEachDevice(const
std::vector<std::string>& devices, size_t i) {
On 2014/07/10 00:07:12, Ben Chan wrote:
> i -> index
> 
> RefreshEachDevice seems like a misnomer. perhaps RefreshDeviceAtIndex?

Done.

https://codereview.chromium.org/379743004/diff/1/chromeos/disks/disk_mount_ma...
chromeos/disks/disk_mount_manager.cc:505: // Part of EnsureMountInfoRefrehsed().
On 2014/07/10 00:07:12, Ben Chan wrote:
> typo: EnsureMountInfoRefrehsed -> EnsureMountInfoRefreshed

Done. Good eye :)

https://codereview.chromium.org/379743004/diff/1/chromeos/disks/disk_mount_ma...
File chromeos/disks/disk_mount_manager.h (right):

https://codereview.chromium.org/379743004/diff/1/chromeos/disks/disk_mount_ma...
chromeos/disks/disk_mount_manager.h:242: // Requests refreshing all the
information about mounted disks.
On 2014/07/10 00:53:21, stevenjb wrote:
> Update comment

Done.

[kinaba, 2014-07-11 00:13:10]

@benchan: do you have anything else?

[Ben Chan, 2014-07-11 01:16:24]

On 2014/07/11 00:13:10, kinaba wrote:
> @benchan: do you have anything else?

lgtm

[kinaba, 2014-07-11 03:26:17]

The CQ bit was checked by kinaba@chromium.org

[commit-bot: I haz the power, 2014-07-11 03:27:59]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/kinaba@chromium.org/379743004/20001

[commit-bot: I haz the power, 2014-07-11 05:16:39]

Change committed as 282558