Changed api to notify when watched directory is deleted.

chrome/browser/chromeos/extensions/file_manager/file_manager_private_apitest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/stl_util.h"
+#include "chrome/browser/chromeos/drive/file_change.h"
+#include "chrome/browser/chromeos/extensions/file_manager/event_router.h"
 #include "chrome/browser/chromeos/file_manager/drive_test_util.h"
+#include "chrome/browser/chromeos/file_manager/file_watcher.h"
 #include "chrome/browser/extensions/extension_apitest.h"
+#include "chrome/test/base/testing_profile.h"
 #include "chromeos/dbus/cros_disks_client.h"
 #include "chromeos/disks/mock_disk_mount_manager.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/install_warning.h"
 
 using ::testing::_;
 using ::testing::ReturnRef;
 
 using chromeos::disks::DiskMountManager;
 
@@ skipping 83 matching lines @@
     chromeos::DEVICE_TYPE_OPTICAL_DISC,
     0,
     true,
     false,
     false,
     true,
     false
   }
 };
 
+struct DispatchDirectoryChangeEvent {

[mtomasz, 2014/10/17 05:26:57]

nit: This can be a nested struct in DispatchDirectoryChangeEventImpl, since only
used there.

[yawano, 2014/10/20 00:49:48]

Acknowledged.

+  base::FilePath virtual_path;
+  std::vector<std::string> extension_ids;
+};
+
+class DispatchDirectoryChangeEventImpl {
+ public:
+  std::vector<DispatchDirectoryChangeEvent> events;
+
+  void Callback(const base::FilePath& virtual_path,
+                const drive::FileChange* list,
+                bool got_error,
+                const std::vector<std::string>& extension_ids) {
+    DispatchDirectoryChangeEvent dispatch_directory_change_event;
+    dispatch_directory_change_event.virtual_path = virtual_path;
+    dispatch_directory_change_event.extension_ids = extension_ids;
+    events.push_back(dispatch_directory_change_event);
+  }

[mtomasz, 2014/10/17 05:26:57]

Please use DISALLOW_COPY_AND_ASSIGN macro on classes which are not supposed to
be copied.

[yawano, 2014/10/20 00:49:48]

Done.

+};
+
+drive::FileChange* CreateDirectoryDeletionFileChange(
+    const base::FilePath& directory_path) {
+  drive::FileChange* file_change = new drive::FileChange;
+  file_change->Update(directory_path,
+                      drive::FileChange::FileType::FILE_TYPE_DIRECTORY,
+                      drive::FileChange::ChangeType::DELETE);
+
+  return file_change;
+}
+
+void AddFileWatchCallback(bool b) { }

[mtomasz, 2014/10/17 05:26:57]

Please avoid abbreviations (except common ones):
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#General_Namin...

[yawano, 2014/10/20 00:49:48]

Done.

+
 }  // namespace
 
 class FileManagerPrivateApiTest : public ExtensionApiTest {
  public:
   FileManagerPrivateApiTest()
       : disk_mount_manager_mock_(NULL) {
     InitMountPoints();
   }
 
   virtual ~FileManagerPrivateApiTest() {
@@ skipping 142 matching lines @@
 
 IN_PROC_BROWSER_TEST_F(FileManagerPrivateApiTest, Permissions) {
   EXPECT_TRUE(
       RunExtensionTestIgnoreManifestWarnings("file_browser/permissions"));
   const extensions::Extension* extension = GetSingleLoadedExtension();
   ASSERT_TRUE(extension);
   ASSERT_EQ(1u, extension->install_warnings().size());
   const extensions::InstallWarning& warning = extension->install_warnings()[0];
   EXPECT_EQ("fileManagerPrivate", warning.key);
 }
+
+IN_PROC_BROWSER_TEST_F(FileManagerPrivateApiTest, OnFileChanged) {
+  // In drive volume, deletion of a directory is notified via OnFileChanged.
+  // Local changes directly come to HandleFileWatchNotification from
+  // FileWatcher.
+
+  TestingProfile profile;
+  file_manager::EventRouter event_router(&profile);
+
+  DispatchDirectoryChangeEventImpl dispatch_directory_change_event_impl;
+  event_router.SetDispatchDirectoryChangeEventImplForTesting(base::Bind(
+      &DispatchDirectoryChangeEventImpl::Callback,
+      base::Unretained(&dispatch_directory_change_event_impl)));
+
+  // /a/b/c and /a/d/e are being watched.
+  // In this test case, we assume these pathes does not actually exist in the

[mtomasz, 2014/10/17 05:26:57]

nit: does -> do [for plural]

[mtomasz, 2014/10/17 05:26:57]

typo: paths

[yawano, 2014/10/20 00:49:48]

Sorry, these comments are for old patches. I deleted this.

[yawano, 2014/10/20 00:49:48]

Acknowledged.

+  // machine. (EventRouter does not notify event if the child directory still
+  // exists).
+  event_router.AddFileWatch(base::FilePath("no-existing-fs/root/a/b/c"),
+                            base::FilePath("no-existing-fs-virtual/root/a/b/c"),
+                            "extension_1",
+                            base::Bind(&AddFileWatchCallback));
+
+  event_router.AddFileWatch(base::FilePath("no-existing-fs/root/a/d/e"),

[mtomasz, 2014/10/17 05:26:57]

Please use FILE_PATH_LITERAL.

[yawano, 2014/10/20 00:49:48]

Done.

+                            base::FilePath("no-existing-fs-hash/root/a/d/e"),
+                            "extension_2",
+                            base::Bind(&AddFileWatchCallback));
+
+  // When /a is deleted (1 and 2 is notified).
+  event_router.OnFileChanged(*CreateDirectoryDeletionFileChange(
+      base::FilePath("no-existing-fs/root/a")));
+  ASSERT_EQ(2lu, dispatch_directory_change_event_impl.events.size());

[mtomasz, 2014/10/17 05:26:57]

Please either validate event members, or replace DispatchDirectoryChangeEvent
with a simple counter.

[yawano, 2014/10/20 00:49:48]

Done.

+
+  // When /a/b/c is deleted (1 is notified).
+  event_router.OnFileChanged(*CreateDirectoryDeletionFileChange(
+      base::FilePath("no-existing-fs/root/a/b/c")));
+  ASSERT_EQ(3lu, dispatch_directory_change_event_impl.events.size());

[mtomasz, 2014/10/17 05:26:57]

Isn't 3u enough?

[yawano, 2014/10/20 00:49:48]

Done.

+
+  // When /z/y is deleted (Not notified)
+  event_router.OnFileChanged(*CreateDirectoryDeletionFileChange(
+      base::FilePath("no-existing-fs/root/z/y")));
+  ASSERT_EQ(3lu, dispatch_directory_change_event_impl.events.size());
+
+  event_router.Shutdown();

[mtomasz, 2014/10/17 05:26:57]

If a test case fails on an assert, then EventRouter will be destroyed without
calling Shutdown. In such case we'll end up in a crash, right? If so, then we
may have to fix it, by eg. moving Shutdown to Teardown, and initialize event
router in SetUp(). WDYT?

[yawano, 2014/10/20 00:49:48]

Done.

Yes. I also think it makes safer to move initialization/shutdown of EventRouter
to Setup/Teardown.

+}