In FileManager send 'on-completed' event only when a scan is completed. Send 'on-cancelled' event w…

In FileManager send 'on-completed' event only when a scan is completed. Send 'on-cancelled' event when a scan is cancelled. Make search & usual spinner work with these events.

BUG=141381
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=151263

[dgozman, 2012-08-13 12:58:08]

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

https://chromiumcodereview.appspot.com/10824277/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/directory_contents.js:253:
cr.dispatchSimpleEvent(this, 'scan-completed');
Forgot |!this.scanCacelled_|.

Maybe, create a method to check all 3 conditions and fire scan-completed?

https://chromiumcodereview.appspot.com/10824277/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/directory_contents.js:457:
this.lastChunkReceived();
May this call fire event for the second time?

[Oleg Eterevsky, 2012-08-13 13:32:21]

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

https://chromiumcodereview.appspot.com/10824277/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/directory_contents.js:253:
cr.dispatchSimpleEvent(this, 'scan-completed');
On 2012/08/13 12:58:08, dgozman wrote:
> Forgot |!this.scanCacelled_|.
> 
> Maybe, create a method to check all 3 conditions and fire scan-completed?

scanCancelled_ checked 3 lines above. I don't want to create a separate function
because the conditions still have to be checked here, to decide whether to call
readNextChunk().

https://chromiumcodereview.appspot.com/10824277/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/directory_contents.js:457:
this.lastChunkReceived();
On 2012/08/13 12:58:08, dgozman wrote:
> May this call fire event for the second time?

It shouldn't. readNextChunk() is called from onPrefetched() only if
allChunksFetched_ is not set yet.

[dgozman, 2012-08-13 13:36:47]

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

https://chromiumcodereview.appspot.com/10824277/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/directory_contents.js:255:
this.readNextChunk();
I believe, this is called if there is pending metadata request, but all chunks
are already fetched.

[Oleg Eterevsky, 2012-08-13 13:39:40]

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

https://chromiumcodereview.appspot.com/10824277/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/directory_contents.js:255:
this.readNextChunk();
On 2012/08/13 13:36:48, dgozman wrote:
> I believe, this is called if there is pending metadata request, but all chunks
> are already fetched.

You're right. Fixed.

[dgozman, 2012-08-13 13:47:12]

I'd say that you may place all checks and events firing to the top of
readNextChunk method.

But this code LGTM too.

[Oleg Eterevsky, 2012-08-13 13:52:07]

On 2012/08/13 13:47:12, dgozman wrote:
> I'd say that you may place all checks and events firing to the top of
> readNextChunk method.

readNextChunk is different for each implementation. Also it is not called from
lastChunkReceived().

> But this code LGTM too.

Thanks for the review.

[commit-bot: I haz the power, 2012-08-13 13:52:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oleg@chromium.org/10824277/5001

[commit-bot: I haz the power, 2012-08-13 15:04:08]

Change committed as 151263