Expose gallery watching functionality to the public mediaGalleries API.

chrome/browser/extensions/api/media_galleries/media_galleries_watch_apitest.cc

Index: chrome/browser/extensions/api/media_galleries/media_galleries_watch_apitest.cc
diff --git a/chrome/browser/extensions/api/media_galleries_private/media_galleries_watch_apitest.cc b/chrome/browser/extensions/api/media_galleries/media_galleries_watch_apitest.cc
similarity index 71%
copy from chrome/browser/extensions/api/media_galleries_private/media_galleries_watch_apitest.cc
copy to chrome/browser/extensions/api/media_galleries/media_galleries_watch_apitest.cc
index a88cac6fb6c84835f6179a60c7ed7ec5b4578e54..204f223f6cc941780eb8f1181ba2836d89b8d12d 100644
--- a/chrome/browser/extensions/api/media_galleries_private/media_galleries_watch_apitest.cc
+++ b/chrome/browser/extensions/api/media_galleries/media_galleries_watch_apitest.cc
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
-// MediaGalleriesPrivate gallery watch API browser tests.
+// MediaGalleries gallery watch API browser tests.
 
 #include "base/file_util.h"
 #include "base/files/file_path.h"
@@ -30,12 +30,14 @@ namespace {
 // Id of test extension from
 // chrome/test/data/extensions/api_test/|kTestExtensionPath|
 const char kTestExtensionId[] = "gceegfkgibmgpfopknlcgleimclbknie";
-const char kTestExtensionPath[] = "media_galleries_private/gallerywatch";
+const char kTestExtensionPath[] = "media_galleries/gallerywatch";
 
 // JS commands.
 const char kGetAllWatchedGalleryIdsCmd[] = "getAllWatchedGalleryIds()";
 const char kGetMediaFileSystemsCmd[] = "getMediaFileSystems()";
 const char kSetupWatchOnValidGalleriesCmd[] = "setupWatchOnValidGalleries()";
+const char kSetupWatchOnUnlistenedValidGalleriesCmd[] =
+    "setupWatchOnUnlistenedValidGalleries()";
 const char kAddGalleryChangedListenerCmd[] = "addGalleryChangedListener()";
 const char kRemoveAllGalleryWatchCmd[] = "removeAllGalleryWatch()";
 const char kRemoveGalleryChangedListenerCmd[] =
@@ -58,32 +60,30 @@ const char kRemoveGalleryWatchOK[] = "remove_gallery_watch_ok";
 // Test reply messages.
 const char kNoGalleryWatchesInstalled[] = "gallery_watchers_does_not_exists";
 const char kAddGalleryWatchRequestFailed[] = "add_watch_request_failed";
+const char kAddGalleryWatchRequestRuntimeError[] =
+    "add_watch_request_runtime_error";
 const char kAddGalleryWatchRequestSucceeded[] = "add_watch_request_succeeded";
 const char kGalleryChangedEventReceived[] = "gallery_changed_event_received";
 const char kGalleryWatchesCheck[] = "gallery_watcher_checks";
 
 }  // namespace
 
-
 ///////////////////////////////////////////////////////////////////////////////
-//                 MediaGalleriesPrivateGalleryWatchApiTest                  //
+//                 MediaGalleriesGalleryWatchApiTest                         //
 ///////////////////////////////////////////////////////////////////////////////
 
-class MediaGalleriesPrivateGalleryWatchApiTest : public ExtensionApiTest {
+class MediaGalleriesGalleryWatchApiTest : public ExtensionApiTest {
  public:
-  MediaGalleriesPrivateGalleryWatchApiTest()
-      : extension_(NULL),
-        background_host_(NULL) {
-  }
-  virtual ~MediaGalleriesPrivateGalleryWatchApiTest() {}
+  MediaGalleriesGalleryWatchApiTest()
+      : extension_(NULL), background_host_(NULL) {}
+  virtual ~MediaGalleriesGalleryWatchApiTest() {}
 
  protected:
   // ExtensionApiTest overrides.
   virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
     ExtensionApiTest::SetUpCommandLine(command_line);
     command_line->AppendSwitchASCII(
-        extensions::switches::kWhitelistedExtensionID,
-        kTestExtensionId);
+        extensions::switches::kWhitelistedExtensionID, kTestExtensionId);
   }
   virtual void SetUpOnMainThread() OVERRIDE {
     ExtensionApiTest::SetUpOnMainThread();
@@ -116,17 +116,18 @@ class MediaGalleriesPrivateGalleryWatchApiTest : public ExtensionApiTest {
     base::FilePath gallery_file =
         test_gallery_.path().Append(FILE_PATH_LITERAL("test1.txt"));
     std::string content("new content");
-    int write_size = base::WriteFile(gallery_file, content.c_str(),
-                                     content.length());
+    int write_size =
+        base::WriteFile(gallery_file, content.c_str(), content.length());
     return (write_size == static_cast<int>(content.length()));
   }
 
   void SetupGalleryWatches() {
-    std::string expected_result = GalleryWatchesSupported() ?
-        kAddGalleryWatchRequestSucceeded : kAddGalleryWatchRequestFailed;
+    std::string expected_result = GalleryWatchesSupported()
+                                      ? kAddGalleryWatchRequestSucceeded
+                                      : kAddGalleryWatchRequestFailed;
 
     ExtensionTestMessageListener add_gallery_watch_finished(
-        expected_result, false  /* no reply */);
+        expected_result, false /* no reply */);
     ExecuteCmdAndCheckReply(kSetupWatchOnValidGalleriesCmd, kAddGalleryWatchOK);
     EXPECT_TRUE(add_gallery_watch_finished.WaitUntilSatisfied());
   }
@@ -136,9 +137,9 @@ class MediaGalleriesPrivateGalleryWatchApiTest : public ExtensionApiTest {
     ASSERT_TRUE(extension_);
     extensions::ExtensionSystem* extension_system =
         extensions::ExtensionSystem::Get(browser()->profile());
-    background_host_ =
-        extension_system->process_manager()->GetBackgroundHostForExtension(
-            extension_->id())->render_view_host();
+    background_host_ = extension_system->process_manager()
+                           ->GetBackgroundHostForExtension(extension_->id())
+                           ->render_view_host();
     ASSERT_TRUE(background_host_);
   }
 
@@ -152,25 +153,27 @@ class MediaGalleriesPrivateGalleryWatchApiTest : public ExtensionApiTest {
 
     ASSERT_TRUE(test_gallery_.CreateUniqueTempDir());
     MediaGalleryPrefInfo gallery_info;
-    ASSERT_FALSE(preferences->LookUpGalleryByPath(test_gallery_.path(),
-                                                  &gallery_info));
-    MediaGalleryPrefId id = preferences->AddGallery(
-        gallery_info.device_id,
-        gallery_info.path,
-        MediaGalleryPrefInfo::kAutoDetected,
-        gallery_info.volume_label,
-        gallery_info.vendor_name,
-        gallery_info.model_name,
-        gallery_info.total_size_in_bytes,
-        gallery_info.last_attach_time,
-        0, 0, 0);
+    ASSERT_FALSE(
+        preferences->LookUpGalleryByPath(test_gallery_.path(), &gallery_info));
+    MediaGalleryPrefId id =
+        preferences->AddGallery(gallery_info.device_id,
+                                gallery_info.path,
+                                MediaGalleryPrefInfo::kAutoDetected,
+                                gallery_info.volume_label,
+                                gallery_info.vendor_name,
+                                gallery_info.model_name,
+                                gallery_info.total_size_in_bytes,
+                                gallery_info.last_attach_time,
+                                0,
+                                0,
+                                0);
 
     preferences->SetGalleryPermissionForExtension(*extension_, id, true);
   }
 
   void FetchMediaGalleriesList() {
     ExtensionTestMessageListener get_media_systems_finished(
-        kGetMediaFileSystemsCallbackOK, false  /* no reply */);
+        kGetMediaFileSystemsCallbackOK, false /* no reply */);
     ExecuteCmdAndCheckReply(kGetMediaFileSystemsCmd, kGetMediaFileSystemsOK);
     EXPECT_TRUE(get_media_systems_finished.WaitUntilSatisfied());
   }
@@ -183,7 +186,7 @@ class MediaGalleriesPrivateGalleryWatchApiTest : public ExtensionApiTest {
 
   content::RenderViewHost* background_host_;
 
-  DISALLOW_COPY_AND_ASSIGN(MediaGalleriesPrivateGalleryWatchApiTest);
+  DISALLOW_COPY_AND_ASSIGN(MediaGalleriesGalleryWatchApiTest);
 };
 
 // Crashing on OSX.
@@ -192,17 +195,18 @@ class MediaGalleriesPrivateGalleryWatchApiTest : public ExtensionApiTest {
 #else
 #define MAYBE_BasicGalleryWatch BasicGalleryWatch
 #endif
-IN_PROC_BROWSER_TEST_F(MediaGalleriesPrivateGalleryWatchApiTest,
+IN_PROC_BROWSER_TEST_F(MediaGalleriesGalleryWatchApiTest,
                        MAYBE_BasicGalleryWatch) {
-  SetupGalleryWatches();
-
   // Add gallery watch listener.
   ExecuteCmdAndCheckReply(kAddGalleryChangedListenerCmd,
                           kAddGalleryChangedListenerOK);
 
+  SetupGalleryWatches();
+
   // Modify gallery contents.
   ExtensionTestMessageListener gallery_change_event_received(
-      kGalleryChangedEventReceived, false  /* no reply */);
+      kGalleryChangedEventReceived, false /* no reply */);
+
   ASSERT_TRUE(AddNewFileInTestGallery());
   if (GalleryWatchesSupported())
     EXPECT_TRUE(gallery_change_event_received.WaitUntilSatisfied());
@@ -217,19 +221,19 @@ IN_PROC_BROWSER_TEST_F(MediaGalleriesPrivateGalleryWatchApiTest,
 }
 
 // http://crbug.com/390979
-IN_PROC_BROWSER_TEST_F(MediaGalleriesPrivateGalleryWatchApiTest,
+IN_PROC_BROWSER_TEST_F(MediaGalleriesGalleryWatchApiTest,
                        DISABLED_RemoveListenerAndModifyGallery) {
   if (!GalleryWatchesSupported())
     return;
 
-  SetupGalleryWatches();
-
   // Add a gallery watch listener.
   ExecuteCmdAndCheckReply(kAddGalleryChangedListenerCmd,
                           kAddGalleryChangedListenerOK);
+  SetupGalleryWatches();
+
   // Modify gallery contents.
   ExtensionTestMessageListener gallery_change_event_received(
-      kGalleryChangedEventReceived, false  /* no reply */);
+      kGalleryChangedEventReceived, false /* no reply */);
   ASSERT_TRUE(AddNewFileInTestGallery());
   EXPECT_TRUE(gallery_change_event_received.WaitUntilSatisfied());
 
@@ -244,23 +248,20 @@ IN_PROC_BROWSER_TEST_F(MediaGalleriesPrivateGalleryWatchApiTest,
   ExecuteCmdAndCheckReply(kRemoveGalleryWatchCmd, kRemoveGalleryWatchOK);
 }
 
-IN_PROC_BROWSER_TEST_F(MediaGalleriesPrivateGalleryWatchApiTest,
+IN_PROC_BROWSER_TEST_F(MediaGalleriesGalleryWatchApiTest,
                        SetupGalleryWatchWithoutListeners) {
   if (!GalleryWatchesSupported())
     return;
 
-  SetupGalleryWatches();
-
-  // No listeners, modify gallery contents.
-  ExtensionTestMessageListener gallery_change_event_received(
-      kGalleryChangedEventReceived, false  /* no reply */);
-  ASSERT_TRUE(AddNewFileInTestGallery());
+  ExtensionTestMessageListener add_watch_runtime_error_received(

[tommycli, 2014/08/20 23:44:09]

I believe you don't need to make ExtensionTestMessageListeners here. Look at the
body of ExecuteCmdAndCheckReply.

Shouldn't you just pass kAddGalleryWatchRequestRuntimeError to that call?

Why is this test currently passing ?

[Oren Blasberg, 2014/08/21 02:45:21]

The test is passing because "setupWatchOnUnlistenedValidGalleries" does end
with:

  chrome.test.sendMessage('add_gallery_watch_ok');

so the ExecuteCmdAndCheckReply does indeed get that reply as expected.

But you're right, it isn't really adding any value to the test, so I just
ipmlemented your suggestion. Thanks!

+      kAddGalleryWatchRequestRuntimeError, false /* no reply */);
+  ExecuteCmdAndCheckReply(kSetupWatchOnUnlistenedValidGalleriesCmd,
+                          kAddGalleryWatchOK);
 
-  // Remove gallery watch.
-  ExecuteCmdAndCheckReply(kRemoveGalleryWatchCmd, kRemoveGalleryWatchOK);
+  EXPECT_TRUE(add_watch_runtime_error_received.WaitUntilSatisfied());
 }
 
-IN_PROC_BROWSER_TEST_F(MediaGalleriesPrivateGalleryWatchApiTest,
+IN_PROC_BROWSER_TEST_F(MediaGalleriesGalleryWatchApiTest,
                        SetupGalleryChangedListenerWithoutWatchers) {
   // Add gallery watch listener.
   ExecuteCmdAndCheckReply(kAddGalleryChangedListenerCmd,
@@ -269,7 +270,7 @@ IN_PROC_BROWSER_TEST_F(MediaGalleriesPrivateGalleryWatchApiTest,
   // Modify gallery contents. Listener should not get called because add watch
   // request was not called.
   ExtensionTestMessageListener gallery_change_event_received(
-      kGalleryChangedEventReceived, false  /* no reply */);
+      kGalleryChangedEventReceived, false /* no reply */);
   ASSERT_TRUE(AddNewFileInTestGallery());
 
   // Remove gallery watch listener.
@@ -277,22 +278,28 @@ IN_PROC_BROWSER_TEST_F(MediaGalleriesPrivateGalleryWatchApiTest,
                           kRemoveGalleryChangedListenerOK);
 }
 
-IN_PROC_BROWSER_TEST_F(MediaGalleriesPrivateGalleryWatchApiTest,
+IN_PROC_BROWSER_TEST_F(MediaGalleriesGalleryWatchApiTest,
                        SetupWatchOnInvalidGallery) {
+  // Add gallery watch listener.
+  ExecuteCmdAndCheckReply(kAddGalleryChangedListenerCmd,
+                          kAddGalleryChangedListenerOK);
   // Set up a invalid gallery watch.
   ExtensionTestMessageListener invalid_gallery_watch_request_finished(

[tommycli, 2014/08/20 23:44:09]

Same comment as above. Why is there a separate ExtensionTestMessageListener from
the one inth ebody of ExecuteCmdAndCheckReply?

Moreover, how is this test even passing if we currently aren't giving a result
to the callback?

I feel confused.

[Oren Blasberg, 2014/08/21 02:45:21]

Same reason as above - the setupWatchOnInvalidGallery method ends by sending the
'ok' message, prior to any callbacks executing which would send the
'kAddGalleryWatchRequestFailed' message.

You're right, we aren't giving a result to the callback. That's a bug which I am
fixing in the api file. But the test still passes because
onAddWatchRequestCallback treats the absence of 'details' as a failure; it goes
into the first IF.

So, I've removed the "!details" part of that IF, which makes the test fail
(timeout). Then I updated the API to fix the other bug. And finally I've made
the same change here as I did for the above comment, getting rid of the "OK"
message.

Thanks so much for noticing this stuff!

-      kAddGalleryWatchRequestFailed, false  /* no reply */);
+      kAddGalleryWatchRequestFailed, false /* no reply */);
   ExecuteCmdAndCheckReply(kSetupWatchOnInvalidGalleryCmd, kAddGalleryWatchOK);
   EXPECT_TRUE(invalid_gallery_watch_request_finished.WaitUntilSatisfied());
 }
 
-IN_PROC_BROWSER_TEST_F(MediaGalleriesPrivateGalleryWatchApiTest,
-                       GetAllGalleryWatch) {
+IN_PROC_BROWSER_TEST_F(MediaGalleriesGalleryWatchApiTest, GetAllGalleryWatch) {
+  // Add gallery watch listener.
+  ExecuteCmdAndCheckReply(kAddGalleryChangedListenerCmd,
+                          kAddGalleryChangedListenerOK);
+
   // Gallery watchers are not yet added.
-  // chrome.mediaGalleriesPrivate.getAllGalleryWatch should return an empty
+  // chrome.mediaGalleries.getAllGalleryWatch should return an empty
   // list.
   ExtensionTestMessageListener initial_get_all_check_finished(
-      kNoGalleryWatchesInstalled, false  /* no reply */);
+      kNoGalleryWatchesInstalled, false /* no reply */);
   ExecuteCmdAndCheckReply(kGetAllWatchedGalleryIdsCmd, kGetAllGalleryWatchOK);
   EXPECT_TRUE(initial_get_all_check_finished.WaitUntilSatisfied());
 
@@ -301,10 +308,10 @@ IN_PROC_BROWSER_TEST_F(MediaGalleriesPrivateGalleryWatchApiTest,
 
   SetupGalleryWatches();
 
-  // chrome.mediaGalleriesPrivate.getAllGalleryWatch should return the
+  // chrome.mediaGalleries.getAllGalleryWatch should return the
   // gallery identifiers.
   ExtensionTestMessageListener get_all_watched_galleries_finished(
-      kGalleryWatchesCheck, false  /* no reply */);
+      kGalleryWatchesCheck, false /* no reply */);
   ExecuteCmdAndCheckReply(kGetAllWatchedGalleryIdsCmd, kGetAllGalleryWatchOK);
   EXPECT_TRUE(get_all_watched_galleries_finished.WaitUntilSatisfied());
 
@@ -312,35 +319,39 @@ IN_PROC_BROWSER_TEST_F(MediaGalleriesPrivateGalleryWatchApiTest,
   ExecuteCmdAndCheckReply(kRemoveGalleryWatchCmd, kRemoveGalleryWatchOK);
 
   // Gallery watchers removed.
-  // chrome.mediaGalleriesPrivate.getAllGalleryWatch() should return an empty
+  // chrome.mediaGalleries.getAllGalleryWatch() should return an empty
   // list.
   ExtensionTestMessageListener final_get_all_check_finished(
-      kNoGalleryWatchesInstalled, false  /* no reply */);
+      kNoGalleryWatchesInstalled, false /* no reply */);
   ExecuteCmdAndCheckReply(kGetAllWatchedGalleryIdsCmd, kGetAllGalleryWatchOK);
   EXPECT_TRUE(final_get_all_check_finished.WaitUntilSatisfied());
 }
 
-IN_PROC_BROWSER_TEST_F(MediaGalleriesPrivateGalleryWatchApiTest,
+IN_PROC_BROWSER_TEST_F(MediaGalleriesGalleryWatchApiTest,
                        RemoveAllGalleryWatch) {
+  // Add gallery watch listener.
+  ExecuteCmdAndCheckReply(kAddGalleryChangedListenerCmd,
+                          kAddGalleryChangedListenerOK);
+
   if (!GalleryWatchesSupported())
     return;
 
   SetupGalleryWatches();
 
-  // chrome.mediaGalleriesPrivate.getAllGalleryWatch should return the watched
+  // chrome.mediaGalleries.getAllGalleryWatch should return the watched
   // gallery identifiers.
   ExtensionTestMessageListener get_all_watched_galleries_finished(
-      kGalleryWatchesCheck, false  /* no reply */);
+      kGalleryWatchesCheck, false /* no reply */);
   ExecuteCmdAndCheckReply(kGetAllWatchedGalleryIdsCmd, kGetAllGalleryWatchOK);
   EXPECT_TRUE(get_all_watched_galleries_finished.WaitUntilSatisfied());
 
   // Remove all gallery watchers.
   ExecuteCmdAndCheckReply(kRemoveAllGalleryWatchCmd, kRemoveAllGalleryWatchOK);
 
-  // Gallery watchers removed. chrome.mediaGalleriesPrivate.getAllGalleryWatch
+  // Gallery watchers removed. chrome.mediaGalleries.getAllGalleryWatch
   // should return an empty list.
   ExtensionTestMessageListener final_get_all_check_finished(
-      kNoGalleryWatchesInstalled, false  /* no reply */);
+      kNoGalleryWatchesInstalled, false /* no reply */);
   ExecuteCmdAndCheckReply(kGetAllWatchedGalleryIdsCmd, kGetAllGalleryWatchOK);
   EXPECT_TRUE(final_get_all_check_finished.WaitUntilSatisfied());
 }