[ew] Simplify the entry watcher logic.

[ew] Simplify the entry watcher logic.

The responsibility of EntryWatcherService has been moved to WatcherManager
implementations.

Note, that the original code was unnecesarily complicated. After replacing
the observer interface with a simple callback, there is no need for an extra
service managing watchers anymore.

This was possible because with the new design entry watchers will no longer
survive reboots.

TEST=Compiles.
BUG=261491
Committed: https://crrev.com/b524db20a4704fb8e3047680d090c25d7a0a52c7
Cr-Commit-Position: refs/heads/master@{#301319}

[mtomasz, 2014-10-22 04:11:27]

mtomasz@chromium.org changed reviewers:
+ benwells@chromium.org

[mtomasz, 2014-10-22 04:11:51]

@benwells: PTAL. Thanks.

[benwells, 2014-10-22 23:09:30]

On 2014/10/22 04:11:51, mtomasz wrote:
> @benwells: PTAL. Thanks.

Apologies, the API review for this was some time ago and not fresh in my memory
:)

I remember we discussed the persistence but I can't remember the outcome. I just
had a look at the API proposal and it seemed to say observers would be retained.
Has there been some change / is the proposal up to date?

[mtomasz, 2014-10-23 01:09:56]

On 2014/10/22 23:09:30, benwells wrote:
> On 2014/10/22 04:11:51, mtomasz wrote:
> > @benwells: PTAL. Thanks.
> 
> Apologies, the API review for this was some time ago and not fresh in my
memory
> :)
> 
> I remember we discussed the persistence but I can't remember the outcome. I
just
> had a look at the API proposal and it seemed to say observers would be
retained.
> Has there been some change / is the proposal up to date?

First of all, we decided to repriotize tasks, and Entry Watcher API is postponed
for now. We still need watching logic in FSP though for Files app, but via our
existing private api (chrome.fileManagerPrivate). It's just much higher priority
for now.

Saying that, watchers will be still persistent between app shutdowns, but they
won't survive a system shutdown. The reason for this change is that it's almost
impossible to implement a reliable watching logic which would survive a
shutdown. Very few file systems will be able to provide such functionality and
it's optional in FSP API.

[benwells, 2014-10-23 01:51:10]

On 2014/10/23 01:09:56, mtomasz wrote:
> On 2014/10/22 23:09:30, benwells wrote:
> > On 2014/10/22 04:11:51, mtomasz wrote:
> > > @benwells: PTAL. Thanks.
> > 
> > Apologies, the API review for this was some time ago and not fresh in my
> memory
> > :)
> > 
> > I remember we discussed the persistence but I can't remember the outcome. I
> just
> > had a look at the API proposal and it seemed to say observers would be
> retained.
> > Has there been some change / is the proposal up to date?
> 
> First of all, we decided to repriotize tasks, and Entry Watcher API is
postponed
> for now. We still need watching logic in FSP though for Files app, but via our
> existing private api (chrome.fileManagerPrivate). It's just much higher
priority
> for now.
> 
> Saying that, watchers will be still persistent between app shutdowns, but they
> won't survive a system shutdown. The reason for this change is that it's
almost
> impossible to implement a reliable watching logic which would survive a
> shutdown. Very few file systems will be able to provide such functionality and
> it's optional in FSP API.

OK. Is the problem with reliability finding the things that changed between
reboots, is it something else?

[mtomasz, 2014-10-23 01:51:55]

On 2014/10/23 01:51:10, benwells wrote:
> On 2014/10/23 01:09:56, mtomasz wrote:
> > On 2014/10/22 23:09:30, benwells wrote:
> > > On 2014/10/22 04:11:51, mtomasz wrote:
> > > > @benwells: PTAL. Thanks.
> > > 
> > > Apologies, the API review for this was some time ago and not fresh in my
> > memory
> > > :)
> > > 
> > > I remember we discussed the persistence but I can't remember the outcome.
I
> > just
> > > had a look at the API proposal and it seemed to say observers would be
> > retained.
> > > Has there been some change / is the proposal up to date?
> > 
> > First of all, we decided to repriotize tasks, and Entry Watcher API is
> postponed
> > for now. We still need watching logic in FSP though for Files app, but via
our
> > existing private api (chrome.fileManagerPrivate). It's just much higher
> priority
> > for now.
> > 
> > Saying that, watchers will be still persistent between app shutdowns, but
they
> > won't survive a system shutdown. The reason for this change is that it's
> almost
> > impossible to implement a reliable watching logic which would survive a
> > shutdown. Very few file systems will be able to provide such functionality
and
> > it's optional in FSP API.
> 
> OK. Is the problem with reliability finding the things that changed between
> reboots, is it something else?

Yes, that's the problem.

[benwells, 2014-10-23 02:10:34]

Overall lg, just trying to figure out what the status of this all is.

https://codereview.chromium.org/642343004/diff/1/storage/browser/fileapi/watc...
File storage/browser/fileapi/watcher_manager.h (right):

https://codereview.chromium.org/642343004/diff/1/storage/browser/fileapi/watc...
storage/browser/fileapi/watcher_manager.h:43: virtual void WatchDirectory(
Is this implemented / called by anything?

[mtomasz, 2014-10-23 02:15:12]

https://codereview.chromium.org/642343004/diff/1/storage/browser/fileapi/watc...
File storage/browser/fileapi/watcher_manager.h (right):

https://codereview.chromium.org/642343004/diff/1/storage/browser/fileapi/watc...
storage/browser/fileapi/watcher_manager.h:43: virtual void WatchDirectory(
On 2014/10/23 02:10:34, benwells wrote:
> Is this implemented / called by anything?

Not now, but I'll need it for upcoming CLs. FSP code is going to implement this
interface. chrome.fileManagerPrivate will use fsp's implementation of it to set
watchers.

In the future watcher logic in chrome.fileManagerPrivate will be replaced with
Entry Watcher API.

[benwells, 2014-10-23 02:21:51]

lgtm

[mtomasz, 2014-10-23 02:22:09]

The CQ bit was checked by mtomasz@chromium.org

[commit-bot: I haz the power, 2014-10-23 02:23:24]

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

[commit-bot: I haz the power, 2014-10-23 02:28:24]

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

[commit-bot: I haz the power, 2014-10-23 02:28:25]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/72360)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/77280)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_asan_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel 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_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[mtomasz, 2014-10-23 09:20:30]

mtomasz@chromium.org changed reviewers:
+ sky@chromium.org

[mtomasz, 2014-10-23 09:20:52]

@sky: PTAL at content/public/test/test_file_system_backend.*. Thanks.

[sky, 2014-10-23 17:03:20]

LGTM

https://codereview.chromium.org/642343004/diff/20001/content/public/test/test...
File content/public/test/test_file_system_backend.cc (right):

https://codereview.chromium.org/642343004/diff/20001/content/public/test/test...
content/public/test/test_file_system_backend.cc:152: return NULL;
nullptr

[mtomasz, 2014-10-24 07:21:21]

The CQ bit was checked by mtomasz@chromium.org

[commit-bot: I haz the power, 2014-10-24 07:21:38]

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

[commit-bot: I haz the power, 2014-10-24 07:26:27]

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

[commit-bot: I haz the power, 2014-10-24 07:26:29]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[mtomasz, 2014-10-24 08:22:50]

The CQ bit was checked by mtomasz@chromium.org

[commit-bot: I haz the power, 2014-10-24 08:23:50]

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

[commit-bot: I haz the power, 2014-10-24 08:28:18]

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

[commit-bot: I haz the power, 2014-10-24 08:28:19]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[mtomasz, 2014-10-27 00:24:37]

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

[mtomasz, 2014-10-27 00:24:47]

@tzik: PTAL at storage/browser/fileapi/watcher_manager.h. Thanks.

[tzik, 2014-10-27 06:03:08]

lgtm

[mtomasz, 2014-10-27 06:06:28]

The CQ bit was checked by mtomasz@chromium.org

[commit-bot: I haz the power, 2014-10-27 06:07:06]

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

[commit-bot: I haz the power, 2014-10-27 06:42:20]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-10-27 06:42:59]

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