Files.app: Show 'no space' message if there is not enough space to import photos.

Files.app: Show 'no space' message if there is not enough space to import photos.

The CL lets create scan for selection before import is executed. The scan is
needed for obtaining the correct entry number to be imported and their total
size.

The CL also adds ScanCache class. The new class does not cache multiple scan as
the previous cache did, but it can cache scan for selection.

BUG=420680
TEST=FileManagerJsTest.ImportControllerTest

[hirono, 2015-01-24 02:39:33]

hirono@chromium.org changed reviewers:
+ smckay@chromium.org

[hirono, 2015-01-24 02:39:33]

PTAL. Thanks!

[Steve McKay, 2015-01-26 18:08:44]

https://codereview.chromium.org/867233003/diff/20001/ui/file_manager/file_man...
File ui/file_manager/file_manager/foreground/js/import_controller.js (left):

https://codereview.chromium.org/867233003/diff/20001/ui/file_manager/file_man...
ui/file_manager/file_manager/foreground/js/import_controller.js:308:
importer.ControllerEnvironment.prototype.addVolumeUnmountListener;
We still need to invalidate scans when a volume is unmounted. I don't understand
why you removed this functionality.

https://codereview.chromium.org/867233003/diff/20001/ui/file_manager/file_man...
File ui/file_manager/file_manager/foreground/js/import_controller.js (right):

https://codereview.chromium.org/867233003/diff/20001/ui/file_manager/file_man...
ui/file_manager/file_manager/foreground/js/import_controller.js:13: NO_SPACE:
'no_spcae',
INSUFFICIENT_SPACE: 'insufficient_space'

https://codereview.chromium.org/867233003/diff/20001/ui/file_manager/file_man...
ui/file_manager/file_manager/foreground/js/import_controller.js:85: if
(this.scanCache_.currentCache.isInvalidated()) {
Let's keep this super simple for now. When *any* scan is invalidated, remove
*all* scans.

https://codereview.chromium.org/867233003/diff/20001/ui/file_manager/file_man...
ui/file_manager/file_manager/foreground/js/import_controller.js:91:
this.environment_.getLocalVolumeAvailableSize().then(function(size) {
Please wrap before anonymous functions. NOT wrapping makes it much harder for
the reader to pick out the separate semantic units.

Also, anon function should have JSDOC with both @param (and @this if bound).

https://codereview.chromium.org/867233003/diff/20001/ui/file_manager/file_man...
ui/file_manager/file_manager/foreground/js/import_controller.js:209:
importer.ImportController.prototype.getScanForImport_ = function() {
Move this logic into ScanCache and rename that class ScanManager. You'll get
better separation of concern.

https://codereview.chromium.org/867233003/diff/20001/ui/file_manager/file_man...
ui/file_manager/file_manager/foreground/js/import_controller.js:231: var scan =
this.scanCache_.getForEntries(targetEntries);
We can't throw away directory scans. A recursive scan of an MTP device is very
expensive and takes a long time. So we need a way to maintain multiple
concurrent scans...at least one for selection based scans and one for recursive
scans.

https://codereview.chromium.org/867233003/diff/20001/ui/file_manager/file_man...
File ui/file_manager/file_manager/foreground/js/import_controller_unittest.js
(right):

https://codereview.chromium.org/867233003/diff/20001/ui/file_manager/file_man...
ui/file_manager/file_manager/foreground/js/import_controller_unittest.js:353:
this.availableSpace_ = /* 1GB */ 1024 * 1024 * 1024;
1024 * 1024 * 1024;  // 1GB

[Steve McKay, 2015-01-28 15:27:50]

Hirono-san. We've made some changes to cloud-import that will allow us to push
updates. Basically we no longer use the command sub-system. This means we can
simplify this logic related to disk space calculation.

Here's what I think we should do. Let's not submit this CL. I'll use it as a
template for a subsequent change that finalizes the change to implement the
"push updates". At that point we can either land this CL, or land a different CL
based on this CL.

So don't bother making further changes on this CL, but please don't delete it so
we can use it as the basis for a future CL.

Sound good?

[chromium-reviews, 2015-01-29 00:18:16]

Yes! Thank you for the update!

On Thu Jan 29 2015 at 12:27:50 AM <smckay@chromium.org> wrote:

> Hirono-san. We've made some changes to cloud-import that will allow us to
> push
> updates. Basically we no longer use the command sub-system. This means we
> can
> simplify this logic related to disk space calculation.
>
> Here's what I think we should do. Let's not submit this CL. I'll use it as
> a
> template for a subsequent change that finalizes the change to implement the
> "push updates". At that point we can either land this CL, or land a
> different CL
> based on this CL.
>
> So don't bother making further changes on this CL, but please don't delete
> it so
> we can use it as the basis for a future CL.
>
> Sound good?
>
>
>
> https://codereview.chromium.org/867233003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[hirono, 2015-02-02 07:27:41]

On 2015/01/29 00:18:16, chromium-reviews wrote:
> Yes! Thank you for the update!
> 
> On Thu Jan 29 2015 at 12:27:50 AM <mailto:smckay@chromium.org> wrote:
> 
> > Hirono-san. We've made some changes to cloud-import that will allow us to
> > push
> > updates. Basically we no longer use the command sub-system. This means we
> > can
> > simplify this logic related to disk space calculation.
> >
> > Here's what I think we should do. Let's not submit this CL. I'll use it as
> > a
> > template for a subsequent change that finalizes the change to implement the
> > "push updates". At that point we can either land this CL, or land a
> > different CL
> > based on this CL.
> >
> > So don't bother making further changes on this CL, but please don't delete
> > it so
> > we can use it as the basis for a future CL.
> >
> > Sound good?
> >
> >
> >
> > https://codereview.chromium.org/867233003/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Steve's new patch was submitted. Let me close the CL.