[chromedriver] Change browser info parser to support Lollipop WebView apps

chrome/test/chromedriver/chrome/browser_info.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/test/chromedriver/chrome/browser_info.h"
 
 #include "base/json/json_reader.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
@@ skipping 19 matching lines @@
 
 Status ParseBrowserInfo(const std::string& data, BrowserInfo* browser_info) {
   scoped_ptr<base::Value> value(base::JSONReader::Read(data));
   if (!value.get())
     return Status(kUnknownError, "version info not in JSON");
 
   base::DictionaryValue* dict;
   if (!value->GetAsDictionary(&dict))
     return Status(kUnknownError, "version info not a dictionary");
 
-  std::string browser;
-  if (!dict->GetString("Browser", &browser)) {
-    return Status(kUnknownError,
-                  "version info doesn't include string 'Browser'");
-  }
+  bool is_android = dict->HasKey("Android-Package");
+  std::string browser_string;
+  if (!dict->GetString("Browser", &browser_string))
+    return Status(kUnknownError, "version doesn't include 'Browser'");
+
+  Status status = ParseBrowserString(is_android, browser_string, browser_info);
+  if (status.IsError())
+    return status;
 
   std::string blink_version;
-  if (!dict->GetString("WebKit-Version", &blink_version)) {
-    return Status(kUnknownError,
-                  "version info doesn't include string 'WebKit-Version'");
-  }
-
-  Status status = ParseBrowserString(browser, &browser_info->browser_name,
-      &browser_info->browser_version, &browser_info->build_no);
-
-  if (status.IsError())
-    return status;
+  if (!dict->GetString("WebKit-Version", &blink_version))
+    return Status(kUnknownError, "version doesn't include 'WebKit-Version'");
 
   return ParseBlinkVersionString(blink_version, &browser_info->blink_revision);
 }
 
-Status ParseBrowserString(const std::string& browser_string,
-                          std::string* browser_name,
-                          std::string* browser_version,
-                          int* build_no) {
+Status ParseBrowserString(bool is_android,
+                          const std::string& browser_string,
+                          BrowserInfo* browser_info) {
   if (browser_string.empty()) {
-    *browser_name = "content shell";
-    return Status(kOk);
-  }
-
-  if (browser_string.find("Version/") == 0u) {
-    *browser_name = "webview";
+    browser_info->browser_name = "content shell";
     return Status(kOk);
   }
 
   std::string prefix = "Chrome/";
+  int build_no;
   if (browser_string.find(prefix) == 0u) {
-    *browser_name = "chrome";
-    *browser_version = browser_string.substr(prefix.length());
+    std::string version = browser_string.substr(prefix.length());
+    std::vector<std::string> version_parts;
+    base::SplitString(version, '.', &version_parts);
 
-    std::vector<std::string> version_parts;
-    base::SplitString(*browser_version, '.', &version_parts);
     if (version_parts.size() != 4 ||
-        !base::StringToInt(version_parts[2], build_no)) {
-      return Status(kUnknownError,
-                    "unrecognized Chrome version: " + *browser_version);
+        !base::StringToInt(version_parts[2], &build_no)) {
+      return Status(kUnknownError, "unrecognized Chrome version: " + version);
     }
 
+    if (build_no != 0) {

[stgao, 2014/12/05 22:10:54]

Would it be possible that |build_no| is un-initialized?

[samuong, 2014/12/05 22:17:57]

The call to StringToInt() should return false if it leaves build_no
uninitialized, so this is already checked for.

+      browser_info->browser_name = "chrome";
+      browser_info->browser_version = browser_string.substr(prefix.length());
+      browser_info->build_no = build_no;
+      return Status(kOk);
+    }
+  }
+
+  if (browser_string.find("Version/") == 0u || (is_android && build_no == 0)) {

[stgao, 2014/12/05 22:10:54]

Maybe add some comment so that we know which condition is for which case?

[samuong, 2014/12/05 22:17:57]

Done.

+    browser_info->browser_name = "webview";
     return Status(kOk);
   }
 
   return Status(kUnknownError,
                 "unrecognized Chrome version: " + browser_string);
 }
 
 Status ParseBlinkVersionString(const std::string& blink_version,
                                int* blink_revision) {
   size_t before = blink_version.find('@');
@@ skipping 15 matching lines @@
   return Status(kOk);
 }
 
 bool IsGitHash(const std::string& revision) {
   const int kShortGitHashLength = 7;
   const int kFullGitHashLength = 40;
   return kShortGitHashLength <= revision.size()
       && revision.size() <= kFullGitHashLength
       && base::ContainsOnlyChars(revision, "0123456789abcdefABCDEF");
 }