test_runner: Add TestInfoExtractor to get the list of test-urls.

components/web_view/test_runner/test_runner_application_delegate.cc

Index: components/web_view/test_runner/test_runner_application_delegate.cc
diff --git a/components/web_view/test_runner/test_runner_application_delegate.cc b/components/web_view/test_runner/test_runner_application_delegate.cc
index ca8cb45241caf5e3946a356b3aae799584e75d8c..50ae0e201148f759f70bdcce07f973659ef5e61d 100644
--- a/components/web_view/test_runner/test_runner_application_delegate.cc
+++ b/components/web_view/test_runner/test_runner_application_delegate.cc
@@ -23,7 +23,6 @@
 #include "mojo/application/public/cpp/application_impl.h"
 #include "mojo/converters/geometry/geometry_type_converters.h"
 #include "mojo/services/network/public/interfaces/url_loader.mojom.h"
-#include "net/base/filename_util.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/size.h"
 #include "url/gurl.h"
@@ -35,104 +34,8 @@
 
 namespace web_view {
 
-namespace {
-
-GURL GetURLForLayoutTest(const std::string& test_name,
-                         base::FilePath* current_working_directory,
-                         bool* enable_pixel_dumping,
-                         std::string* expected_pixel_hash) {
-  // A test name is formated like file:///path/to/test'--pixel-test'pixelhash
-  std::string path_or_url = test_name;
-  std::string pixel_switch;
-  std::string pixel_hash;
-  std::string::size_type separator_position = path_or_url.find('\'');
-  if (separator_position != std::string::npos) {
-    pixel_switch = path_or_url.substr(separator_position + 1);
-    path_or_url.erase(separator_position);
-  }
-  separator_position = pixel_switch.find('\'');
-  if (separator_position != std::string::npos) {
-    pixel_hash = pixel_switch.substr(separator_position + 1);
-    pixel_switch.erase(separator_position);
-  }
-  if (enable_pixel_dumping) {
-    *enable_pixel_dumping =
-        (pixel_switch == "--pixel-test" || pixel_switch == "-p");
-  }
-  if (expected_pixel_hash)
-    *expected_pixel_hash = pixel_hash;
-
-  GURL test_url(path_or_url);
-  if (!(test_url.is_valid() && test_url.has_scheme())) {
-    // This is a test.
-    base::ThreadRestrictions::ScopedAllowIO allow_io;
-#if defined(OS_WIN)
-    base::FilePath::StringType wide_path_or_url =
-        base::SysNativeMBToWide(path_or_url);
-    base::FilePath local_file(wide_path_or_url);
-#else
-    base::FilePath local_file(path_or_url);
-#endif
-    if (!base::PathExists(local_file)) {
-      base::FilePath base_path;
-      PathService::Get(base::DIR_SOURCE_ROOT, &base_path);
-      local_file = base_path.Append(FILE_PATH_LITERAL("third_party"))
-                       .Append(FILE_PATH_LITERAL("WebKit"))
-                       .Append(FILE_PATH_LITERAL("LayoutTests"))
-                       .Append(local_file);
-    }
-    test_url = net::FilePathToFileURL(base::MakeAbsoluteFilePath(local_file));
-  }
-  base::FilePath local_path;
-  if (current_working_directory) {
-    // We're outside of the message loop here, and this is a test.
-    base::ThreadRestrictions::ScopedAllowIO allow_io;
-    if (net::FileURLToFilePath(test_url, &local_path))
-      *current_working_directory = local_path.DirName();
-    else
-      base::GetCurrentDirectory(current_working_directory);
-  }
-  return test_url;
-}
-
-bool GetNextTestURL(const base::CommandLine::StringVector& args,
-                    size_t* position,
-                    GURL* test_url) {
-  std::string test_string;
-  if (*position >= args.size())
-    return false;
-  if (args[*position] == FILE_PATH_LITERAL("-")) {
-    do {
-      bool success = !!std::getline(std::cin, test_string, '\n');
-      if (!success)
-        return false;
-    } while (test_string.empty());
-  } else {
-#if defined(OS_WIN)
-    test_string = base::WideToUTF8(args[(*position)++]);
-#else
-    test_string = args[(*position)++];
-#endif
-  }
-
-  DCHECK(!test_string.empty());
-  if (test_string == "QUIT")
-    return false;
-  bool enable_pixel_dumps;
-  std::string pixel_hash;
-  base::FilePath cwd;
-  *test_url =
-      GetURLForLayoutTest(test_string, &cwd, &enable_pixel_dumps, &pixel_hash);
-  return true;
-}
-
-}  // namespace
-
 TestRunnerApplicationDelegate::TestRunnerApplicationDelegate()
-    : app_(nullptr),
-      root_(nullptr),
-      content_(nullptr),
-      cmdline_position_(0u) {}
+    : app_(nullptr), root_(nullptr), content_(nullptr) {}
 
 TestRunnerApplicationDelegate::~TestRunnerApplicationDelegate() {
   if (root_)
@@ -193,14 +96,14 @@ void TestRunnerApplicationDelegate::OnEmbed(mojo::View* root) {
   content_->SetBounds(*mojo::Rect::From(gfx::Rect(kViewportSize)));
   content_->SetVisible(true);
 
-  cmdline_args_ = base::CommandLine::ForCurrentProcess()->GetArgs();
   std::cout << "#READY\n";
   std::cout.flush();
 
-  cmdline_position_ = 0;
-  GURL test_url;
-  if (GetNextTestURL(cmdline_args_, &cmdline_position_, &test_url))
-    LaunchURL(test_url);
+  auto cmdline_args = base::CommandLine::ForCurrentProcess()->GetArgs();
+  test_extractor_.reset(new test_runner::TestInfoExtractor(cmdline_args));
+
+  if (auto test_info = test_extractor_->GetNextTest())

[Ryan Sleevi, 2015/09/10 17:48:04]

This does not seem like a style-guide approved use of auto, nor does line 102
(in particular, the clause "Continue to use manifest type declarations when it
helps readability, ")

The thread from
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/5-Bt3BJzAo0/EWcU2...
explains how this usage can hide dangerous subtleties

[sadrul, 2015/09/10 18:09:09]

Done.

+    LaunchURL(test_info->url);
 }
 
 void TestRunnerApplicationDelegate::OnConnectionLost(
@@ -231,9 +134,8 @@ void TestRunnerApplicationDelegate::TestFinished() {
   std::cerr << "#EOF\n";
   std::cerr.flush();
 
-  GURL test_url;
-  if (GetNextTestURL(cmdline_args_, &cmdline_position_, &test_url))
-    LaunchURL(test_url);
+  if (auto test_info = test_extractor_->GetNextTest())

[Ryan Sleevi, 2015/09/10 17:48:03]

Same

[sadrul, 2015/09/10 18:09:08]

Done.

+    LaunchURL(test_info->url);
   else
     Terminate();
 }