Files.app: Implement media import cancellation.

Files.app: Implement media import cancellation.

Add a cancellation callback so that the active import task is cancelled when the user clicks on the cancel button in the progress notification.

BUG=420680
TEST=browser_test: FileManagerJsTest.MediaImportHandlerTest

Committed: https://crrev.com/3e53fe55d93d5557ffd6c99b2d1c3ea34dbbaad8
Cr-Commit-Position: refs/heads/master@{#311516}

[Ben Kwa, 2015-01-12 23:40:26]

kenobi@chromium.org changed reviewers:
+ mtomasz@chromium.org, smckay@chromium.org

[Steve McKay, 2015-01-13 00:13:01]

LGTM. And this makes a good guide CL for implementing cancel on unmount.

[mtomasz, 2015-01-13 00:27:42]

Please keep the CL title the same as the first line of the CL description, as
only the second one goes to git log.

https://codereview.chromium.org/846903004/diff/1/ui/file_manager/file_manager...
File ui/file_manager/file_manager/background/js/media_import_handler.js (right):

https://codereview.chromium.org/846903004/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/background/js/media_import_handler.js:234:
this.cancelCallback_();
nit: We usually reset the cancel callback to null before calling it, as the
callback is external and can do anything (including cancelling again). WDYT?

https://codereview.chromium.org/846903004/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/background/js/media_import_handler.js:272: if
(this.canceled_) {
Can we add some unit tests for the cancellation?

[Ben Kwa, 2015-01-13 16:59:30]

Patchset #2 (id:20001) has been deleted

[Ben Kwa, 2015-01-13 18:09:43]

Patchset #2 (id:40001) has been deleted

[Ben Kwa, 2015-01-13 18:17:52]

New patch contains a bit of noise in the diff vs patch 1, because of a rebase. 
Sorry about that.

https://codereview.chromium.org/846903004/diff/1/ui/file_manager/file_manager...
File ui/file_manager/file_manager/background/js/media_import_handler.js (right):

https://codereview.chromium.org/846903004/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/background/js/media_import_handler.js:234:
this.cancelCallback_();
On 2015/01/13 00:27:41, mtomasz wrote:
> nit: We usually reset the cancel callback to null before calling it, as the
> callback is external and can do anything (including cancelling again). WDYT?

Makes sense, thanks!

https://codereview.chromium.org/846903004/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/background/js/media_import_handler.js:272: if
(this.canceled_) {
On 2015/01/13 00:27:41, mtomasz wrote:
> Can we add some unit tests for the cancellation?

Done.

[Ben Kwa, 2015-01-13 18:18:53]

https://codereview.chromium.org/846903004/diff/1/ui/file_manager/file_manager...
File ui/file_manager/file_manager/background/js/media_import_handler.js (right):

https://codereview.chromium.org/846903004/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/background/js/media_import_handler.js:234:
this.cancelCallback_();
On 2015/01/13 18:17:52, Ben Kwa wrote:
> On 2015/01/13 00:27:41, mtomasz wrote:
> > nit: We usually reset the cancel callback to null before calling it, as the
> > callback is external and can do anything (including cancelling again). WDYT?
> 
> Makes sense, thanks!

PS fileOperationUtil.Task#requestCancel doesn't actually do what you're
describing above.  Might be worth fixing that.

[mtomasz, 2015-01-14 00:36:47]

lgtm

[Ben Kwa, 2015-01-14 14:51:05]

The CQ bit was checked by kenobi@chromium.org

[commit-bot: I haz the power, 2015-01-14 14:51:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/846903004/60001

[commit-bot: I haz the power, 2015-01-14 14:54:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-01-14 14:54:33]

Try jobs failed on following builders:
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_compile...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ng...)
  ios_rel_device_ninja_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[Ben Kwa, 2015-01-14 17:23:13]

The CQ bit was checked by kenobi@chromium.org

[commit-bot: I haz the power, 2015-01-14 17:23:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/846903004/80001

[commit-bot: I haz the power, 2015-01-14 18:23:13]

Committed patchset #3 (id:80001)

[commit-bot: I haz the power, 2015-01-14 18:24:12]

Patchset 3 (id:??) landed as
https://crrev.com/3e53fe55d93d5557ffd6c99b2d1c3ea34dbbaad8
Cr-Commit-Position: refs/heads/master@{#311516}