[fsp] Separate recursive and non-recursive watchers.

[fsp] Separate recursive and non-recursive watchers.

With this patch, if one extension has a recursive watcher, and another non-
recursive one, then the providing extension will know about both of them.

In an upcoming patch ObservedEntry will be renamed to Watcher, since now
we can have two observed entries for the same path, which doesn't sound the
best.

TEST=unit_tests, browser_tests: *FileSystemProvider*
BUG=248427
Committed: https://crrev.com/60c10816ed1c9cbabdd4ef1a1b640b8009912f81
Cr-Commit-Position: refs/heads/master@{#301532}

[mtomasz, 2014-10-24 01:40:54]

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

[mtomasz, 2014-10-24 01:41:04]

@hirono: PTAL. Thanks!

[hirono, 2014-10-24 04:34:54]

https://codereview.chromium.org/679573002/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/file_system_provider.idl (right):

https://codereview.chromium.org/679573002/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/file_system_provider.idl:343: DOMString entryPath;
recursive is needed?

https://codereview.chromium.org/679573002/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/file_system_provider.idl:362: DOMString
observedPath;
Now we cannot identify an observer by using observedPath.
How about introducing ID for observer?

[mtomasz, 2014-10-24 05:37:50]

https://codereview.chromium.org/679573002/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/file_system_provider.idl (right):

https://codereview.chromium.org/679573002/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/file_system_provider.idl:343: DOMString entryPath;
On 2014/10/24 04:34:54, hirono wrote:
> recursive is needed?

Good catch. Added. Unfortunately we don't have browser tests yet for that, but
they will come very soon.

https://codereview.chromium.org/679573002/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/file_system_provider.idl:362: DOMString
observedPath;
On 2014/10/24 04:34:54, hirono wrote:
> Now we cannot identify an observer by using observedPath.
> How about introducing ID for observer?

Right. I'd like to do it separately, after committing this one, and 642023003,
though.

[hirono, 2014-10-24 06:58:43]

https://codereview.chromium.org/679573002/diff/20001/chrome/browser/chromeos/...
File
chrome/browser/chromeos/extensions/file_system_provider/file_system_provider_api.cc
(right):

https://codereview.chromium.org/679573002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_system_provider/file_system_provider_api.cc:56:
const api::file_system_provider::Change& child_change) {
nit: child_change -> change

https://codereview.chromium.org/679573002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_system_provider/file_system_provider_api.cc:65:
const IDLChanges& child_changes) {
nit: child_changes -> changes

https://codereview.chromium.org/679573002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_system_provider/file_system_provider_api.cc:68:
for (IDLChanges::const_iterator it = child_changes.begin();
nit: 
You can use "for (auto it : changes)".
But up to you.

https://codereview.chromium.org/679573002/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/file_system_provider/observed_entry.h (right):

https://codereview.chromium.org/679573002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_system_provider/observed_entry.h:18: // Key for
storing an observed entry in the map.
Could you add a comment saying that we can have two observers for the same path?

https://codereview.chromium.org/679573002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_system_provider/observed_entry.h:41: bool
recursive;
Could you add comment recursive and last_tag?
They are not unobvious what these are.

https://codereview.chromium.org/679573002/diff/20001/chrome/browser/chromeos/...
File
chrome/browser/chromeos/file_system_provider/provided_file_system_unittest.cc
(right):

https://codereview.chromium.org/679573002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_system_provider/provided_file_system_unittest.cc:104:
const ProvidedFileSystemObserver::Changes& child_changes)
nit: child_changes -> changes
Same for followings.

[mtomasz, 2014-10-24 09:50:25]

https://codereview.chromium.org/679573002/diff/20001/chrome/browser/chromeos/...
File
chrome/browser/chromeos/extensions/file_system_provider/file_system_provider_api.cc
(right):

https://codereview.chromium.org/679573002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_system_provider/file_system_provider_api.cc:56:
const api::file_system_provider::Change& child_change) {
On 2014/10/24 06:58:43, hirono wrote:
> nit: child_change -> change

Done.

https://codereview.chromium.org/679573002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_system_provider/file_system_provider_api.cc:65:
const IDLChanges& child_changes) {
On 2014/10/24 06:58:42, hirono wrote:
> nit: child_changes -> changes

Done.

https://codereview.chromium.org/679573002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_system_provider/file_system_provider_api.cc:68:
for (IDLChanges::const_iterator it = child_changes.begin();
On 2014/10/24 06:58:43, hirono wrote:
> nit: 
> You can use "for (auto it : changes)".
> But up to you.

Right! Done.

https://codereview.chromium.org/679573002/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/file_system_provider/observed_entry.h (right):

https://codereview.chromium.org/679573002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_system_provider/observed_entry.h:18: // Key for
storing an observed entry in the map.
On 2014/10/24 06:58:43, hirono wrote:
> Could you add a comment saying that we can have two observers for the same
path?

Done.

https://codereview.chromium.org/679573002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_system_provider/observed_entry.h:41: bool
recursive;
On 2014/10/24 06:58:43, hirono wrote:
> Could you add comment recursive and last_tag?
> They are not unobvious what these are.
>  

These members are commented in 642023003.

https://codereview.chromium.org/679573002/diff/20001/chrome/browser/chromeos/...
File
chrome/browser/chromeos/file_system_provider/provided_file_system_unittest.cc
(right):

https://codereview.chromium.org/679573002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_system_provider/provided_file_system_unittest.cc:104:
const ProvidedFileSystemObserver::Changes& child_changes)
On 2014/10/24 06:58:43, hirono wrote:
> nit: child_changes -> changes
> Same for followings.

Done.

[hirono, 2014-10-27 02:16:17]

lgtm. Thanks!

[mtomasz, 2014-10-27 03:53:50]

The CQ bit was checked by mtomasz@chromium.org

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

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

[commit-bot: I haz the power, 2014-10-27 03:59:01]

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

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

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 04:01:00]

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

[mtomasz, 2014-10-27 04:01:14]

@benwells: PTAL at IDL. Thanks.

[benwells, 2014-10-27 21:05:59]

idl lgtm

[mtomasz, 2014-10-28 00:58:39]

The CQ bit was checked by mtomasz@chromium.org

[commit-bot: I haz the power, 2014-10-28 01:02:37]

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

[commit-bot: I haz the power, 2014-10-28 01:10:26]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-10-28 01:11:08]

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