Make mediaGalleriesPrivate.onDevice{Attached,Detached} work properly by always running the internal device tracking code.

BUG=155652
Review URL: https://codereview.chromium.org/11151010

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@162046 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 161 insertions, 20 deletions

diff --git a/chrome/browser/extensions/api/media_galleries_private/media_galleries_private_apitest.cc b/chrome/browser/extensions/api/media_galleries_private/media_galleries_private_apitest.cc
index a972c883..289f2b9 100644
--- a/chrome/browser/extensions/api/media_galleries_private/media_galleries_private_apitest.cc
+++ b/chrome/browser/extensions/api/media_galleries_private/media_galleries_private_apitest.cc
@@ -14,6 +14,7 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test_utils.h"
 #include "googleurl/src/gurl.h"
@@ -25,9 +26,25 @@ namespace {
 const char kTestExtensionId[] = "lkegdcleigedmkiikoijjgfchobofdbe";
 const char kTestExtensionPath[] = "media_galleries_private/attachdetach";
 
+// JS commands.
+const char kAddAttachListenerCmd[] = "addAttachListener()";
+const char kAddDetachListenerCmd[] = "addDetachListener()";
+const char kAddDummyDetachListenerCmd[] = "addDummyDetachListener()";
+const char kRemoveAttachListenerCmd[] = "removeAttachListener()";
+const char kRemoveDummyDetachListenerCmd[] = "removeDummyDetachListener()";
+
+// And JS reply messages.
+const char kAddAttachListenerOk[] = "add_attach_ok";
+const char kAddDetachListenerOk[] = "add_detach_ok";
+const char kAddDummyDetachListenerOk[] = "add_dummy_detach_ok";
+const char kRemoveAttachListenerOk[] = "remove_attach_ok";
+const char kRemoveDummyDetachListenerOk[] = "remove_dummy_detach_ok";
+
+// Test reply messages.
 const char kAttachTestOk[] = "attach_test_ok";
 const char kDetachTestOk[] = "detach_test_ok";
 
+// Dummy device properties.
 const char kDeviceId[] = "testDeviceId";
 const char kDeviceName[] = "foobar";
 FilePath::CharType kDevicePath[] = FILE_PATH_LITERAL("/qux");
@@ -35,6 +52,17 @@ FilePath::CharType kDevicePath[] = FILE_PATH_LITERAL("/qux");
 }  // namespace
 
 class MediaGalleriesPrivateApiTest : public ExtensionApiTest {
+ public:
+  MediaGalleriesPrivateApiTest() {}
+  virtual ~MediaGalleriesPrivateApiTest() {}
+
+  // ExtensionApiTest overrides.
+  virtual void SetUp() OVERRIDE {
+    device_id_ = chrome::MediaStorageUtil::MakeDeviceId(
+        chrome::MediaStorageUtil::REMOVABLE_MASS_STORAGE_WITH_DCIM, kDeviceId);
+    ExtensionApiTest::SetUp();
+  }
+
  protected:
   // ExtensionApiTest overrides.
   virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
@@ -42,6 +70,39 @@ class MediaGalleriesPrivateApiTest : public ExtensionApiTest {
     command_line->AppendSwitchASCII(switches::kWhitelistedExtensionID,
                                     kTestExtensionId);
   }
+
+  void ChangeListener(content::RenderViewHost* host,
+                      const std::string& js_command,
+                      const std::string& ok_message) {
+    ExtensionTestMessageListener listener(ok_message, false  /* no reply */);
+    host->ExecuteJavascriptInWebFrame(string16(), ASCIIToUTF16(js_command));
+    EXPECT_TRUE(listener.WaitUntilSatisfied());
+  }
+
+  void AttachDetach() {
+    Attach();
+    Detach();
+  }
+
+  void Attach() {
+    base::SystemMonitor::Get()->ProcessRemovableStorageAttached(
+        device_id_, ASCIIToUTF16(kDeviceName), kDevicePath);
+    WaitForDeviceEvents();
+  }
+
+  void Detach() {
+    base::SystemMonitor::Get()->ProcessRemovableStorageDetached(device_id_);
+    WaitForDeviceEvents();
+  }
+
+ private:
+  void WaitForDeviceEvents() {
+    content::RunAllPendingInMessageLoop();
+  }
+
+  std::string device_id_;
+
+  DISALLOW_COPY_AND_ASSIGN(MediaGalleriesPrivateApiTest);
 };
 
 IN_PROC_BROWSER_TEST_F(MediaGalleriesPrivateApiTest, DeviceAttachDetachEvents) {
@@ -50,24 +111,71 @@ IN_PROC_BROWSER_TEST_F(MediaGalleriesPrivateApiTest, DeviceAttachDetachEvents) {
       LoadExtension(test_data_dir_.AppendASCII(kTestExtensionPath));
   ASSERT_TRUE(extension);
 
-  base::SystemMonitor* system_monitor = base::SystemMonitor::Get();
-  ASSERT_TRUE(system_monitor);
+  content::RenderViewHost* host =
+      browser()->profile()->GetExtensionProcessManager()->
+      GetBackgroundHostForExtension(extension->id())->render_view_host();
+  ASSERT_TRUE(host);
 
-  const std::string device_id = chrome::MediaStorageUtil::MakeDeviceId(
-      chrome::MediaStorageUtil::REMOVABLE_MASS_STORAGE_WITH_DCIM, kDeviceId);
+  // No listeners, attach and detach a couple times.
+  AttachDetach();
+  AttachDetach();
 
-  // Attach event.
+  // Add attach listener.
+  ChangeListener(host, kAddAttachListenerCmd, kAddAttachListenerOk);
+
+  // Attach / detach
   const std::string expect_attach_msg =
       base::StringPrintf("%s,%s", kAttachTestOk, kDeviceName);
   ExtensionTestMessageListener attach_finished_listener(expect_attach_msg,
                                                         false  /* no reply */);
-  system_monitor->ProcessRemovableStorageAttached(
-      device_id, ASCIIToUTF16(kDeviceName), kDevicePath);
+  Attach();
+  EXPECT_TRUE(attach_finished_listener.WaitUntilSatisfied());
+  Detach();
+
+  // Attach / detach
+  Attach();
   EXPECT_TRUE(attach_finished_listener.WaitUntilSatisfied());
+  // Detach
+  Detach();
+
+  // Remove attach listener.
+  ChangeListener(host, kRemoveAttachListenerCmd, kRemoveAttachListenerOk);
+
+  // No listeners, attach and detach a couple times.
+  AttachDetach();
+  AttachDetach();
+
+  // Add detach listener.
+  ChangeListener(host, kAddDummyDetachListenerCmd, kAddDummyDetachListenerOk);
+
+  // Attach / detach
+  Attach();
 
-  // Detach event.
   ExtensionTestMessageListener detach_finished_listener(kDetachTestOk,
                                                         false  /* no reply */);
-  system_monitor->ProcessRemovableStorageDetached(device_id);
+  Detach();
+  EXPECT_TRUE(detach_finished_listener.WaitUntilSatisfied());
+
+  // Attach / detach
+  Attach();
+  Detach();
+  EXPECT_TRUE(detach_finished_listener.WaitUntilSatisfied());
+
+  // Switch ok dummy detach listener for the regular one.
+  ChangeListener(host, kRemoveDummyDetachListenerCmd,
+                 kRemoveDummyDetachListenerOk);
+  ChangeListener(host, kAddDetachListenerCmd, kAddDetachListenerOk);
+
+  // Add attach listener.
+  ChangeListener(host, kAddAttachListenerCmd, kAddAttachListenerOk);
+
+  Attach();
+  EXPECT_TRUE(attach_finished_listener.WaitUntilSatisfied());
+  Detach();
+  EXPECT_TRUE(detach_finished_listener.WaitUntilSatisfied());
+
+  Attach();
   EXPECT_TRUE(attach_finished_listener.WaitUntilSatisfied());
+  Detach();
+  EXPECT_TRUE(detach_finished_listener.WaitUntilSatisfied());
 }
diff --git a/chrome/browser/extensions/api/media_galleries_private/media_galleries_private_event_router.cc b/chrome/browser/extensions/api/media_galleries_private/media_galleries_private_event_router.cc
index 8df3f2e..06691e5 100644
--- a/chrome/browser/extensions/api/media_galleries_private/media_galleries_private_event_router.cc
+++ b/chrome/browser/extensions/api/media_galleries_private/media_galleries_private_event_router.cc
@@ -102,13 +102,14 @@ void MediaGalleriesPrivateEventRouter::OnRemovableStorageAttached(
     const std::string& id,
     const string16& name,
     const FilePath::StringType& location) {
+  TransientDeviceIds* device_ids = TransientDeviceIds::GetInstance();
+  if (!device_ids->DeviceAttached(id))
+    return;
+
   EventRouter* router = profile_->GetExtensionEventRouter();
   if (!router->HasEventListener(kOnAttachEventName))
     return;
 
-  TransientDeviceIds* device_ids = TransientDeviceIds::GetInstance();
-  if (!device_ids->DeviceAttached(id))
-    return;
   std::string transient_id = device_ids->GetTransientIdForUniqueId(id);
   CHECK(!transient_id.empty());
 
@@ -123,10 +124,6 @@ void MediaGalleriesPrivateEventRouter::OnRemovableStorageAttached(
 
 void MediaGalleriesPrivateEventRouter::OnRemovableStorageDetached(
     const std::string& id) {
-  EventRouter* router = profile_->GetExtensionEventRouter();
-  if (!router->HasEventListener(kOnDetachEventName))
-    return;
-
   TransientDeviceIds* device_ids = TransientDeviceIds::GetInstance();
   std::string transient_id = device_ids->GetTransientIdForUniqueId(id);
   if (transient_id.empty()) {
@@ -135,6 +132,10 @@ void MediaGalleriesPrivateEventRouter::OnRemovableStorageDetached(
   }
   CHECK(device_ids->DeviceDetached(id));
 
+  EventRouter* router = profile_->GetExtensionEventRouter();
+  if (!router->HasEventListener(kOnDetachEventName))
+    return;
+
   DeviceDetachmentDetails details;
   details.device_id = transient_id;
 
diff --git a/chrome/test/data/extensions/api_test/media_galleries_private/attachdetach/test.js b/chrome/test/data/extensions/api_test/media_galleries_private/attachdetach/test.js
index 8f1eb63..76d6a55 100644
--- a/chrome/test/data/extensions/api_test/media_galleries_private/attachdetach/test.js
+++ b/chrome/test/data/extensions/api_test/media_galleries_private/attachdetach/test.js
@@ -4,18 +4,50 @@
 
 var attachedDeviceId = '42';
 
-chrome.mediaGalleriesPrivate.onDeviceAttached.addListener(function(details) {
+var testAttach = function(details) {
   // Save the device id for the detach test.
   attachedDeviceId = details.deviceId;
   // The C++ test code will check if this is ok.
   chrome.test.sendMessage('attach_test_ok,' + details.deviceName);
-});
+};
 
-chrome.mediaGalleriesPrivate.onDeviceDetached.addListener(function(details) {
+// Makes sure the detach device id matches and then ACK the detach event.
+var testDetach = function(details) {
   // The C++ test does not know the device id, so check here.
   if (details.deviceId != attachedDeviceId) {
     chrome.test.fail('Bad device id in onDeviceDetached test.');
     return;
   }
   chrome.test.sendMessage('detach_test_ok');
-});
+};
+
+// Simply ACK the detach event.
+var testDummyDetach = function(details) {
+  chrome.test.sendMessage('detach_test_ok');
+};
+
+// These functions add / remove listeners.
+function addAttachListener() {
+  chrome.mediaGalleriesPrivate.onDeviceAttached.addListener(testAttach);
+  chrome.test.sendMessage('add_attach_ok');
+}
+
+function addDetachListener() {
+  chrome.mediaGalleriesPrivate.onDeviceDetached.addListener(testDetach);
+  chrome.test.sendMessage('add_detach_ok');
+}
+
+function addDummyDetachListener() {
+  chrome.mediaGalleriesPrivate.onDeviceDetached.addListener(testDummyDetach);
+  chrome.test.sendMessage('add_dummy_detach_ok');
+}
+
+function removeAttachListener() {
+  chrome.mediaGalleriesPrivate.onDeviceAttached.removeListener(testAttach);
+  chrome.test.sendMessage('remove_attach_ok');
+}
+
+function removeDummyDetachListener() {
+  chrome.mediaGalleriesPrivate.onDeviceAttached.removeListener(testDummyDetach);
+  chrome.test.sendMessage('remove_dummy_detach_ok');
+}