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
   }
 };
 
+void DispatchDirectoryChangeEventImpl(
+    int* counter, const base::FilePath& virtual_path,
+    const drive::FileChange* list, bool got_error,
+    const std::vector<std::string>& extension_ids) {
+  ++(*counter);
+}
+
+scoped_ptr<drive::FileChange> CreateDirectoryDeletionFileChange(
+    const base::FilePath& directory_path) {
+  scoped_ptr<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.Pass();
+}
+
+void AddFileWatchCallback(bool success) {}
+
 }  // namespace
 
 class FileManagerPrivateApiTest : public ExtensionApiTest {
  public:
   FileManagerPrivateApiTest()
       : disk_mount_manager_mock_(NULL) {
     InitMountPoints();
   }
 
   virtual ~FileManagerPrivateApiTest() {
     DCHECK(!disk_mount_manager_mock_);
+    DCHECK(!testing_profile_);
+    DCHECK(!event_router_);
     STLDeleteValues(&volumes_);
   }
 
+  virtual void SetUpOnMainThread() override {
+    ExtensionApiTest::SetUpOnMainThread();
+
+    testing_profile_.reset(new TestingProfile());
+    event_router_.reset(new file_manager::EventRouter(testing_profile_.get()));
+  }
+
+  virtual void TearDownOnMainThread() override {
+    event_router_->Shutdown();
+
+    event_router_.reset();
+    testing_profile_.reset();
+
+    ExtensionApiTest::TearDownOnMainThread();
+  }
+
   // ExtensionApiTest override
   virtual void SetUpInProcessBrowserTestFixture() override {
     ExtensionApiTest::SetUpInProcessBrowserTestFixture();
-
     disk_mount_manager_mock_ = new chromeos::disks::MockDiskMountManager;
     chromeos::disks::DiskMountManager::InitializeForTesting(
         disk_mount_manager_mock_);
     disk_mount_manager_mock_->SetupDefaultReplies();
 
     // override mock functions.
     ON_CALL(*disk_mount_manager_mock_, FindDiskBySourcePath(_)).WillByDefault(
         Invoke(this, &FileManagerPrivateApiTest::FindVolumeBySourcePath));
     EXPECT_CALL(*disk_mount_manager_mock_, disks())
         .WillRepeatedly(ReturnRef(volumes_));
@@ skipping 95 matching lines @@
       const std::string& source_path) {
     DiskMountManager::DiskMap::const_iterator volume_it =
         volumes_.find(source_path);
     return (volume_it == volumes_.end()) ? NULL : volume_it->second;
   }
 
  protected:
   chromeos::disks::MockDiskMountManager* disk_mount_manager_mock_;
   DiskMountManager::DiskMap volumes_;
   DiskMountManager::MountPointMap mount_points_;
+  scoped_ptr<TestingProfile> testing_profile_;
+  scoped_ptr<file_manager::EventRouter> event_router_;
 };
 
 IN_PROC_BROWSER_TEST_F(FileManagerPrivateApiTest, Mount) {
   file_manager::test_util::WaitUntilDriveMountPointIsAdded(
       browser()->profile());
 
   // We will call fileManagerPrivate.unmountVolume once. To test that method, we
   // check that UnmountPath is really called with the same value.
   EXPECT_CALL(*disk_mount_manager_mock_, UnmountPath(_, _, _))
       .Times(0);
   EXPECT_CALL(*disk_mount_manager_mock_,
               UnmountPath(
                   chromeos::CrosDisksClient::GetArchiveMountPoint().AppendASCII(
                       "archive_mount_path").AsUTF8Unsafe(),
                   chromeos::UNMOUNT_OPTIONS_NONE, _)).Times(1);
 
   ASSERT_TRUE(RunComponentExtensionTest("file_browser/mount_test"))
       << message_;
 }
 
 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.
+  int counter = 0;
+  event_router_->SetDispatchDirectoryChangeEventImplForTesting(
+      base::Bind(&DispatchDirectoryChangeEventImpl, &counter));
+
+  // /a/b/c and /a/d/e are being watched.
+  event_router_->AddFileWatch(
+      base::FilePath(FILE_PATH_LITERAL("/no-existing-fs/root/a/b/c")),
+      base::FilePath(FILE_PATH_LITERAL("/no-existing-fs-virtual/root/a/b/c")),
+      "extension_1", base::Bind(&AddFileWatchCallback));
+
+  event_router_->AddFileWatch(
+      base::FilePath(FILE_PATH_LITERAL("/no-existing-fs/root/a/d/e")),
+      base::FilePath(FILE_PATH_LITERAL("/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(FILE_PATH_LITERAL("/no-existing-fs/root/a"))).get());
+  EXPECT_EQ(2, counter);
+
+  // When /a/b/c is deleted (1 is notified).
+  event_router_->OnFileChanged(*CreateDirectoryDeletionFileChange(
+                                    base::FilePath(FILE_PATH_LITERAL(

[mtomasz, 2014/10/20 08:56:53]

I'm not really sure if that is correct. scoped_ptr instances will be removed
once they get out of scope, and here we have only temporary objects, so we may
end up on "use after free". WDYT?

How about creating local variables holding the scoped pointers?

scoped_ptr<drive::FileChange> first_change = CreateDirectory...
event_router_->OnFileChanged(*file_change.get(), ...).

BTW. can't we just avoid pointers and have a normal object?

[yawano, 2014/10/20 10:42:37]

I checked the object is usable (not released) in EventRouter::OnFileChanged, and
I also checked these created objects are freed correctly by inserting a log to
destructor of FileChange.

However, as you commented, from readability point of view, I removed the helper
method and introduced temporary local variables. I think this makes the code
much simpler.

+                                        "/no-existing-fs/root/a/b/c"))).get());
+  EXPECT_EQ(3, counter);
+
+  // When /z/y is deleted (Not notified).
+  event_router_->OnFileChanged(*CreateDirectoryDeletionFileChange(
+                                    base::FilePath(FILE_PATH_LITERAL(
+                                        "/no-existing-fs/root/z/y"))).get());
+  EXPECT_EQ(3, counter);
+}