[Blink-FileAPI] Add the notification of update observer.

[Blink-FileAPI] Add the notification of update observer to
FileSystemOperationRunner::{CopyInForeignFile, CopyFileLocal, MoveFileLocal}

BUG=413545
TEST=content_unittests

Committed: https://crrev.com/8a59e3fcd7c0eb846cb089c94322d2b2f913e4f5
Cr-Commit-Position: refs/heads/master@{#295422}

[iseki, 2014-09-12 07:25:11]

iseki@chromium.org changed reviewers:
+ tzik@chromium.org

[iseki, 2014-09-12 07:25:11]

Please take look.
This CL is related to Issue 410198.
I found these classes and functions don't call notification.
I try to add the notification of OnModifyFile in another issue because it affect
many test case :(

[tzik, 2014-09-17 04:10:20]

# sorry for delayed review

https://codereview.chromium.org/565073002/diff/1/storage/browser/fileapi/copy...
File storage/browser/fileapi/copy_or_move_operation_delegate.cc (right):

https://codereview.chromium.org/565073002/diff/1/storage/browser/fileapi/copy...
storage/browser/fileapi/copy_or_move_operation_delegate.cc:67:
NotifyOnStartUpdate(dest_url_);
Since the notification is already sent by FileSystemOperationRunner in
PrepareForWrite, this NotifyOnStartUpdate seem not needed for the top level URL.
Could you revert this?

https://codereview.chromium.org/565073002/diff/1/storage/browser/fileapi/remo...
File storage/browser/fileapi/remove_operation_delegate.cc (right):

https://codereview.chromium.org/565073002/diff/1/storage/browser/fileapi/remo...
storage/browser/fileapi/remove_operation_delegate.cc:27:
NotifyOnStartUpdate(url_);
ditto

[tzik, 2014-09-17 05:03:06]

https://codereview.chromium.org/565073002/diff/1/storage/browser/fileapi/copy...
File storage/browser/fileapi/copy_or_move_operation_delegate.cc (right):

https://codereview.chromium.org/565073002/diff/1/storage/browser/fileapi/copy...
storage/browser/fileapi/copy_or_move_operation_delegate.cc:67:
NotifyOnStartUpdate(dest_url_);
On 2014/09/17 04:10:19, tzik wrote:
> Since the notification is already sent by FileSystemOperationRunner in
> PrepareForWrite, this NotifyOnStartUpdate seem not needed for the top level
URL.
> Could you revert this?

Sorry, I misunderstood the structure.
This notification is needed.

But, since the actual operation is done via FileSystemOperationRunner,
FileSystemOperationRunner::{Move,Copy}FileLocal are better place to put the
notification.

https://codereview.chromium.org/565073002/diff/1/storage/browser/fileapi/remo...
File storage/browser/fileapi/remove_operation_delegate.cc (right):

https://codereview.chromium.org/565073002/diff/1/storage/browser/fileapi/remo...
storage/browser/fileapi/remove_operation_delegate.cc:54:
NotifyOnStartUpdate(url);
ditto

https://codereview.chromium.org/565073002/diff/1/storage/browser/fileapi/remo...
storage/browser/fileapi/remove_operation_delegate.cc:61:
NotifyOnEndUpdate(url_);
ditto

https://codereview.chromium.org/565073002/diff/1/storage/browser/fileapi/remo...
storage/browser/fileapi/remove_operation_delegate.cc:74:
NotifyOnEndUpdate(url_);
ditto

[iseki, 2014-09-17 06:21:46]

Thank you for your review.

https://codereview.chromium.org/565073002/diff/1/storage/browser/fileapi/copy...
File storage/browser/fileapi/copy_or_move_operation_delegate.cc (right):

https://codereview.chromium.org/565073002/diff/1/storage/browser/fileapi/copy...
storage/browser/fileapi/copy_or_move_operation_delegate.cc:67:
NotifyOnStartUpdate(dest_url_);
On 2014/09/17 05:03:05, tzik wrote:
> On 2014/09/17 04:10:19, tzik wrote:
> > Since the notification is already sent by FileSystemOperationRunner in
> > PrepareForWrite, this NotifyOnStartUpdate seem not needed for the top level
> URL.
> > Could you revert this?
> 
> Sorry, I misunderstood the structure.
> This notification is needed.
> 
> But, since the actual operation is done via FileSystemOperationRunner,
> FileSystemOperationRunner::{Move,Copy}FileLocal are better place to put the
> notification.

Done.

https://codereview.chromium.org/565073002/diff/1/storage/browser/fileapi/copy...
storage/browser/fileapi/copy_or_move_operation_delegate.cc:67:
NotifyOnStartUpdate(dest_url_);
On 2014/09/17 04:10:19, tzik wrote:
> Since the notification is already sent by FileSystemOperationRunner in
> PrepareForWrite, this NotifyOnStartUpdate seem not needed for the top level
URL.
> Could you revert this?

Done.

https://codereview.chromium.org/565073002/diff/1/storage/browser/fileapi/remo...
File storage/browser/fileapi/remove_operation_delegate.cc (right):

https://codereview.chromium.org/565073002/diff/1/storage/browser/fileapi/remo...
storage/browser/fileapi/remove_operation_delegate.cc:27:
NotifyOnStartUpdate(url_);
On 2014/09/17 04:10:19, tzik wrote:
> ditto

Done.

https://codereview.chromium.org/565073002/diff/1/storage/browser/fileapi/remo...
storage/browser/fileapi/remove_operation_delegate.cc:54:
NotifyOnStartUpdate(url);
On 2014/09/17 05:03:06, tzik wrote:
> ditto

Done.

https://codereview.chromium.org/565073002/diff/1/storage/browser/fileapi/remo...
storage/browser/fileapi/remove_operation_delegate.cc:61:
NotifyOnEndUpdate(url_);
On 2014/09/17 05:03:06, tzik wrote:
> ditto

Done.

https://codereview.chromium.org/565073002/diff/1/storage/browser/fileapi/remo...
storage/browser/fileapi/remove_operation_delegate.cc:74:
NotifyOnEndUpdate(url_);
On 2014/09/17 05:03:06, tzik wrote:
> ditto

Done.

[tzik, 2014-09-18 03:37:29]

LGTM

Please update the description before land this CL.

[iseki, 2014-09-18 03:40:37]

On 2014/09/18 03:37:29, tzik wrote:
> LGTM
> 
> Please update the description before land this CL.

Thanks :)

[iseki, 2014-09-18 03:40:40]

The CQ bit was checked by iseki@chromium.org

[commit-bot: I haz the power, 2014-09-18 03:41:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/565073002/40001

[commit-bot: I haz the power, 2014-09-18 04:38:38]

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

[commit-bot: I haz the power, 2014-09-18 04:38:39]

Try jobs failed on following builders:
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[iseki, 2014-09-18 04:54:24]

The CQ bit was checked by iseki@chromium.org

[commit-bot: I haz the power, 2014-09-18 04:55:04]

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

[commit-bot: I haz the power, 2014-09-18 05:51:37]

Committed patchset #4 (id:60001) as 69941576293eaf5a53665317a44a106937bcffa3

[commit-bot: I haz the power, 2014-09-18 05:52:21]

Patchset 4 (id:??) landed as
https://crrev.com/8a59e3fcd7c0eb846cb089c94322d2b2f913e4f5
Cr-Commit-Position: refs/heads/master@{#295422}