Media Galleries: Refactor test to not need RunSecondPhase().

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

Index: chrome/browser/extensions/api/media_galleries/media_galleries_apitest.cc
===================================================================
--- chrome/browser/extensions/api/media_galleries/media_galleries_apitest.cc	(revision 224775)
+++ chrome/browser/extensions/api/media_galleries/media_galleries_apitest.cc	(working copy)
@@ -2,28 +2,23 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/bind.h"
-#include "base/bind_helpers.h"
-#include "base/command_line.h"
+#include "base/file_util.h"
 #include "base/files/scoped_temp_dir.h"
+#include "base/json/json_writer.h"
 #include "base/path_service.h"
-#include "base/strings/stringprintf.h"
+#include "base/safe_numerics.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/values.h"
 #include "chrome/browser/apps/app_browsertest_util.h"
 #include "chrome/browser/browser_process.h"
-#include "chrome/browser/extensions/extension_apitest.h"
-#include "chrome/browser/extensions/extension_process_manager.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/media_galleries/media_file_system_registry.h"
 #include "chrome/browser/media_galleries/media_galleries_preferences.h"
 #include "chrome/browser/media_galleries/media_galleries_test_util.h"
 #include "chrome/browser/storage_monitor/storage_info.h"
 #include "chrome/browser/storage_monitor/storage_monitor.h"
-#include "chrome/browser/ui/browser.h"
 #include "chrome/common/chrome_paths.h"
-#include "content/public/browser/render_view_host.h"
 #include "content/public/test/test_utils.h"
-#include "extensions/common/switches.h"
 
 using extensions::PlatformAppBrowserTest;
 
@@ -38,8 +33,6 @@
 base::FilePath::CharType kDevicePath[] = FILE_PATH_LITERAL("/qux");
 #endif
 
-const char kTestGalleries[] = "testGalleries(%d)";
-
 }  // namespace
 
 // This function is to ensure at least one (fake) media gallery exists for
@@ -68,6 +61,9 @@
 
 class MediaGalleriesPlatformAppBrowserTest : public PlatformAppBrowserTest {
  protected:
+  MediaGalleriesPlatformAppBrowserTest() : test_jpg_size_(0) {}
+  virtual ~MediaGalleriesPlatformAppBrowserTest() {}
+
   virtual void SetUpOnMainThread() OVERRIDE {
     PlatformAppBrowserTest::SetUpOnMainThread();
     ensure_media_directories_exist_.reset(new EnsureMediaDirectoriesExists);
@@ -79,19 +75,14 @@
     PlatformAppBrowserTest::TearDownOnMainThread();
   }
 
-  // Since ExtensionTestMessageListener does not work with RunPlatformAppTest(),
-  // This helper method can be used to run additional media gallery tests.
-  void RunSecondTestPhase(const std::string& command) {
-    const extensions::Extension* extension = GetSingleLoadedExtension();
-    extensions::ExtensionHost* host =
-        extensions::ExtensionSystem::Get(browser()->profile())->
-            process_manager()->GetBackgroundHostForExtension(extension->id());
-    ASSERT_TRUE(host);
+  bool RunMediaGalleriesTest(const std::string& extension_name) {
+    return RunMediaGalleriesTestWithArg(extension_name, NULL);
+  }
 
-    ResultCatcher catcher;
-    host->render_view_host()->ExecuteJavascriptInWebFrame(
-        base::string16(), base::UTF8ToUTF16(command));
-    EXPECT_TRUE(catcher.GetNextResult()) << message_;
+  bool RunMediaGalleriesTestWithArg(const std::string& extension_name,

[tommycli, 2013/09/24 00:34:08]

Since everywhere you use this, you are effectively converting from a
base::ListValue, this method could just take a const base::ListValue&. Save you
some SLOC

[Lei Zhang, 2013/09/24 01:37:46]

Done.

I made RunMediaGalleriesTest() and RunMediaGalleriesTestWithArgs() mirror
RunPlatformAppTest() and RunPlatformAppTestWithArg(), but I suppose we wouldn't
use it any other way in this file.

+                                    const char* custom_arg) {
+    const char kTestDir[] = "api_test/media_galleries/";
+    return RunPlatformAppTestWithArg(kTestDir + extension_name, custom_arg);
   }
 
   void AttachFakeDevice() {
@@ -128,40 +119,55 @@
     // Invalid file, should not show up as a FileEntry in JS at all.
     ASSERT_TRUE(base::CopyFile(test_data_path.AppendASCII("test.txt"),
                                write_path.AppendASCII("test.txt")));
+
+    int64 file_size;
+    ASSERT_TRUE(file_util::GetFileSize(test_data_path.AppendASCII("test.jpg"),
+                                       &file_size));
+    test_jpg_size_ = base::checked_numeric_cast<int>(file_size);
   }
 
   int num_galleries() const {
     return ensure_media_directories_exist_->num_galleries();
   }
 
+  int test_jpg_size() const { return test_jpg_size_; }
+
  private:
   std::string device_id_;
+  int test_jpg_size_;
   scoped_ptr<EnsureMediaDirectoriesExists> ensure_media_directories_exist_;
 };
 
 IN_PROC_BROWSER_TEST_F(MediaGalleriesPlatformAppBrowserTest,
                        MediaGalleriesNoAccess) {
-  ASSERT_TRUE(RunPlatformAppTest("api_test/media_galleries/no_access"))
+  base::ListValue custom_args;
+  custom_args.AppendInteger(num_galleries());
+
+  std::string json_string;
+  base::JSONWriter::Write(&custom_args, &json_string);
+  ASSERT_TRUE(RunMediaGalleriesTestWithArg("no_access", json_string.c_str()))
       << message_;
-  RunSecondTestPhase(base::StringPrintf(kTestGalleries, num_galleries()));
 }
 
 IN_PROC_BROWSER_TEST_F(MediaGalleriesPlatformAppBrowserTest, NoGalleriesRead) {
-  ASSERT_TRUE(RunPlatformAppTest("api_test/media_galleries/no_galleries"))
-      << message_;
+  ASSERT_TRUE(RunMediaGalleriesTest("no_galleries")) << message_;
 }
 
 IN_PROC_BROWSER_TEST_F(MediaGalleriesPlatformAppBrowserTest,
                        NoGalleriesCopyTo) {
-  ASSERT_TRUE(RunPlatformAppTest(
-      "api_test/media_galleries/no_galleries_copy_to")) << message_;
+  ASSERT_TRUE(RunMediaGalleriesTest("no_galleries_copy_to")) << message_;
 }
 
 IN_PROC_BROWSER_TEST_F(MediaGalleriesPlatformAppBrowserTest,
                        MediaGalleriesRead) {
-  ASSERT_TRUE(RunPlatformAppTest("api_test/media_galleries/read_access"))
+  base::ListValue custom_args;
+  custom_args.AppendInteger(num_galleries());
+  custom_args.AppendInteger(test_jpg_size());
+
+  std::string json_string;
+  base::JSONWriter::Write(&custom_args, &json_string);
+  ASSERT_TRUE(RunMediaGalleriesTestWithArg("read_access", json_string.c_str()))
       << message_;
-  RunSecondTestPhase(base::StringPrintf(kTestGalleries, num_galleries()));
 }
 
 IN_PROC_BROWSER_TEST_F(MediaGalleriesPlatformAppBrowserTest,
@@ -169,8 +175,7 @@
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
   MakeFakeMediaGalleryForTest(browser()->profile(), temp_dir.path());
-  ASSERT_TRUE(RunPlatformAppTest("api_test/media_galleries/copy_to_access"))
-      << message_;
+  ASSERT_TRUE(RunMediaGalleriesTest("copy_to_access")) << message_;
 }
 
 IN_PROC_BROWSER_TEST_F(MediaGalleriesPlatformAppBrowserTest,
@@ -178,26 +183,26 @@
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
   MakeFakeMediaGalleryForTest(browser()->profile(), temp_dir.path());
-  ASSERT_TRUE(RunPlatformAppTest(
-      "api_test/media_galleries/copy_to_access/no_access"))
-      << message_;
+  ASSERT_TRUE(RunMediaGalleriesTest("copy_to_access/no_access")) << message_;
 }
 
 IN_PROC_BROWSER_TEST_F(MediaGalleriesPlatformAppBrowserTest,
                        MediaGalleriesAccessAttached) {
   AttachFakeDevice();
 
-  ASSERT_TRUE(RunPlatformAppTest("api_test/media_galleries/access_attached"))
-      << message_;
+  base::ListValue custom_args;
+  custom_args.AppendInteger(num_galleries() + 1);
+  custom_args.AppendString(kDeviceName);
 
-  RunSecondTestPhase(base::StringPrintf(
-      "testGalleries(%d, \"%s\")", num_galleries() + 1, kDeviceName));
+  std::string json_string;
+  base::JSONWriter::Write(&custom_args, &json_string);
+  ASSERT_TRUE(RunMediaGalleriesTestWithArg(
+      "access_attached", json_string.c_str())) << message_;
 
   DetachFakeDevice();
 }
 
 IN_PROC_BROWSER_TEST_F(MediaGalleriesPlatformAppBrowserTest,
                        GetFilesystemMetadata) {
-  ASSERT_TRUE(RunPlatformAppTest("api_test/media_galleries/metadata"))
-      << message_;
+  ASSERT_TRUE(RunMediaGalleriesTest("metadata")) << message_;
 }