[chromedriver] Write command line file to both /data/local and /data/local/tmp for Chrome on Androi…

chrome/test/chromedriver/chrome/device_manager.cc

Index: chrome/test/chromedriver/chrome/device_manager.cc
diff --git a/chrome/test/chromedriver/chrome/device_manager.cc b/chrome/test/chromedriver/chrome/device_manager.cc
index bd62a2f310d7dfb7b15f350a31cb14e41e237815..b856a43db7488cae64036ac4f8fdea82c4841de7 100644
--- a/chrome/test/chromedriver/chrome/device_manager.cc
+++ b/chrome/test/chromedriver/chrome/device_manager.cc
@@ -16,6 +16,8 @@
 #include "chrome/test/chromedriver/chrome/adb.h"
 #include "chrome/test/chromedriver/chrome/status.h"
 
+const char* kChromeCmdLineFile = "/data/local/chrome-command-line";

[bulach, 2014/01/15 12:58:43]

you may want to create  bug and a TODO, link this patch to it, and date it to
m36 ? :)

then, could do with:

const char* kChromeCmdLineFile = "/data/local/tmp/chrome-command-line";
const char* kChromeCmdLineBeforeM33 = "/data/local/chrome-command-line";

[craigdh, 2014/01/22 22:11:44]

So after some discussion it was decided to not set the debug app automatically,
in which case we'll need to keep writing to both these locations. Some of the
reasons were:

- What if the user wants to have another app set as debug app? No workaround if
we set it. This way we can support both.
- Element of least surprise: we don't magically switch where chrome reads the
command line file from for tests running afterwards.

+
 Device::Device(
     const std::string& device_serial, Adb* adb,
     base::Callback<void()> release_callback)
@@ -38,7 +40,7 @@ Status Device::SetUp(const std::string& package,
         active_package_ + " was launched and has not been quit");
 
   Status status = adb_->CheckAppInstalled(serial_, package);
-  if (!status.IsOk())
+  if (status.IsError())
     return status;
 
   std::string known_activity;
@@ -59,13 +61,13 @@ Status Device::SetUp(const std::string& package,
              package.find("webview") == std::string::npos) {
     known_activity = "com.google.android.apps.chrome.Main";
     device_socket = "chrome_devtools_remote";
-    command_line_file = "/data/local/chrome-command-line";
+    command_line_file = kChromeCmdLineFile;
     exec_name = "chrome";
   }
 
   if (!use_running_app) {
     status = adb_->ClearAppData(serial_, package);
-    if (!status.IsOk())
+    if (status.IsError())
       return status;
 
     if (!known_activity.empty()) {
@@ -78,15 +80,27 @@ Status Device::SetUp(const std::string& package,
     }
 
     if (!command_line_file.empty()) {
-      status = adb_->SetCommandLineFile(serial_, command_line_file, exec_name,
-                                        args);
-      if (!status.IsOk())
-        return status;
+      // If Chrome is set as the debug app it looks in /data/local/tmp/.
+      // There's no way to know if this is set, so write to both locations.
+      if (command_line_file == kChromeCmdLineFile) {
+        status = adb_->SetCommandLineFile(
+            serial_, kChromeCmdLineFile, exec_name, args);
+        Status status2 = adb_->SetCommandLineFile(
+            serial_, "/data/local/tmp/chrome-command-line", exec_name, args);

[bulach, 2014/01/15 12:58:43]

see above, this would then become kChromeCmdLineFileBeforeM33, with a TODO to
remove this block.

+        if (status.IsError() && status2.IsError())
+          return Status(kUnknownError,
+              "Failed to set Chrome's command line file on device " + serial_);
+      } else {
+        status = adb_->SetCommandLineFile(
+            serial_, command_line_file, exec_name, args);
+        if (status.IsError())
+          return status;
+      }
     }
 
     status = adb_->Launch(serial_, package,
                           known_activity.empty() ? activity : known_activity);
-    if (!status.IsOk())
+    if (status.IsError())
       return status;
 
     active_package_ = package;
@@ -106,7 +120,7 @@ Status Device::ForwardDevtoolsPort(const std::string& package,
     Status status = adb_->GetPidByName(serial_,
                                        process.empty() ? package : process,
                                        &pid);
-    if (!status.IsOk()) {
+    if (status.IsError()) {
       if (process.empty())
         status.AddDetails(
             "process name must be specified if not equal to package name");
@@ -122,7 +136,7 @@ Status Device::TearDown() {
   if (!active_package_.empty()) {
     std::string response;
     Status status = adb_->ForceStop(serial_, active_package_);
-    if (!status.IsOk())
+    if (status.IsError())
       return status;
     active_package_ = "";
   }
@@ -138,7 +152,7 @@ DeviceManager::~DeviceManager() {}
 Status DeviceManager::AcquireDevice(scoped_ptr<Device>* device) {
   std::vector<std::string> devices;
   Status status = adb_->GetDevices(&devices);
-  if (!status.IsOk())
+  if (status.IsError())
     return status;
 
   if (devices.empty())
@@ -162,7 +176,7 @@ Status DeviceManager::AcquireSpecificDevice(
     const std::string& device_serial, scoped_ptr<Device>* device) {
   std::vector<std::string> devices;
   Status status = adb_->GetDevices(&devices);
-  if (!status.IsOk())
+  if (status.IsError())
     return status;
 
   if (std::find(devices.begin(), devices.end(), device_serial) == devices.end())