Make GetOrCreateGUID synchronous

This is part 2 of 6 for FieldTrial refactoring for WebView

Part 1: https://chromium-review.googlesource.com/c/562098/
Part 2: https://chromium-review.googlesource.com/c/561920/
Part 3: https://chromium-review.googlesource.com/c/561922/
Part 4: https://chromium-review.googlesource.com/c/561980/
Part 5: https://chromium-review.googlesource.com/c/562417/
Part 6: https://chromium-review.googlesource.com/c/562021/

This CL makes the reading on the client ID happen synchronously
at WebView startup only if the enable-webview-finch command
line flag is provided. This behavior is necessary in order to be
able to set up field trials as early as possible in the WebView
startup process. GetOrCreateGUID() is now a static member of
the AwMetricsServiceClient class.

BUG=678288

Change-Id: I69153f87243fa84f6816785090bfa963e34cf0b2
Reviewed-on: https://chromium-review.googlesource.com/561920
Commit-Queue: Kyle Milka <kmilka@google.com>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Selim Gurun <sgurun@chromium.org>
Reviewed-by: Paul Miller <paulmiller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485390}

android_webview/browser/aw_browser_context.cc

@@ -186,16 +186,10 @@ void AwBrowserContext::PreMainMessageLoopRun() {
 
   blacklist_manager_.reset(CreateURLBlackListManager(user_pref_service_.get()));
 
-  // UMA uses randomly-generated GUIDs (globally unique identifiers) to
-  // anonymously identify logs. Every WebView-using app on every device
-  // is given a GUID, stored in this file in the app's data directory.
-  const FilePath guid_file_path =
-      GetPath().Append(FILE_PATH_LITERAL("metrics_guid"));
   AwMetricsServiceClient::GetInstance()->Initialize(
       user_pref_service_.get(),
-      content::BrowserContext::GetDefaultStoragePartition(this)->
-          GetURLRequestContext(),
-      guid_file_path);
+      content::BrowserContext::GetDefaultStoragePartition(this)
+          ->GetURLRequestContext());
 
   web_restriction_provider_.reset(
       new web_restrictions::WebRestrictionsClient());

android_webview/browser/aw_browser_main_parts.cc

@@ -7,6 +7,7 @@
 #include "android_webview/browser/aw_browser_context.h"
 #include "android_webview/browser/aw_browser_terminator.h"
 #include "android_webview/browser/aw_content_browser_client.h"
+#include "android_webview/browser/aw_metrics_service_client.h"
 #include "android_webview/browser/aw_result_codes.h"
 #include "android_webview/browser/aw_safe_browsing_config_helper.h"
 #include "android_webview/browser/deferred_gpu_command_service.h"
@@ -150,6 +151,11 @@ int AwBrowserMainParts::PreCreateThreads() {
         base::MakeUnique<AwBrowserTerminator>());
   }
 
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableWebViewFinch)) {
+    AwMetricsServiceClient::GetOrCreateGUID();
+  }
+
   return content::RESULT_CODE_NORMAL_EXIT;
 }
 

android_webview/browser/aw_metrics_service_client.cc

@@ -5,6 +5,7 @@
 #include "android_webview/browser/aw_metrics_service_client.h"
 
 #include "android_webview/browser/aw_metrics_log_uploader.h"
+#include "android_webview/common/aw_switches.h"
 #include "android_webview/common/aw_version_info_values.h"
 #include "android_webview/jni/AwMetricsServiceClient_jni.h"
 #include "base/android/build_info.h"
@@ -13,6 +14,8 @@
 #include "base/files/file_util.h"
 #include "base/guid.h"
 #include "base/i18n/rtl.h"
+#include "base/lazy_instance.h"
+#include "base/path_service.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "components/metrics/call_stack_profile_metrics_provider.h"
 #include "components/metrics/enabled_state_provider.h"
@@ -38,6 +41,13 @@ namespace {
 
 const int kUploadIntervalMinutes = 30;
 
+// A GUID in text form is composed of 32 hex digits and 4 hyphens.
+const size_t GUID_SIZE = 32 + 4;
+
+// Client ID of the app, read and cached synchronously at startup
+base::LazyInstance<std::string>::Leaky g_client_id_guid =
+    LAZY_INSTANCE_INITIALIZER;
+
 // Callbacks for metrics::MetricsStateManager::Create. Store/LoadClientInfo
 // allow Windows Chrome to back up ClientInfo. They're no-ops for WebView.
 
@@ -48,30 +58,6 @@ std::unique_ptr<metrics::ClientInfo> LoadClientInfo() {
   return client_info;
 }
 
-// A GUID in text form is composed of 32 hex digits and 4 hyphens.
-const size_t GUID_SIZE = 32 + 4;
-
-void GetOrCreateGUID(const base::FilePath guid_file_path, std::string* guid) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::FILE);
-
-  // Try to read an existing GUID.
-  if (base::ReadFileToStringWithMaxSize(guid_file_path, guid, GUID_SIZE)) {
-    if (base::IsValidGUID(*guid))
-      return;
-    else
-      LOG(ERROR) << "Overwriting invalid GUID";
-  }
-
-  // We must write a new GUID.
-  *guid = base::GenerateGUID();
-  if (!base::WriteFile(guid_file_path, guid->c_str(), guid->size())) {
-    // If writing fails, proceed anyway with the new GUID. It won't be persisted
-    // to the next run, but we can still collect metrics with this 1-time GUID.
-    LOG(ERROR) << "Failed to write new GUID";
-  }
-  return;
-}
-
 version_info::Channel GetChannelFromPackageName() {
   JNIEnv* env = base::android::AttachCurrentThread();
   std::string package_name = base::android::ConvertJavaStringToUTF8(
@@ -89,10 +75,47 @@ AwMetricsServiceClient* AwMetricsServiceClient::GetInstance() {
   return g_lazy_instance_.Pointer();
 }
 
+void AwMetricsServiceClient::GetOrCreateGUID() {
+  // Check for cached GUID
+  if (g_client_id_guid.Get().length() == GUID_SIZE)
+    return;
+
+  // UMA uses randomly-generated GUIDs (globally unique identifiers) to
+  // anonymously identify logs. Every WebView-using app on every device
+  // is given a GUID, stored in this file in the app's data directory.
+  base::FilePath user_data_dir;
+  if (!PathService::Get(base::DIR_ANDROID_APP_DATA, &user_data_dir)) {
+    LOG(ERROR) << "Failed to get app data directory for Android WebView";
+
+    // Generate a 1-time GUID so metrics can still be collected
+    g_client_id_guid.Get() = base::GenerateGUID();
+    return;
+  }
+
+  const base::FilePath guid_file_path =
+      user_data_dir.Append(FILE_PATH_LITERAL("metrics_guid"));
+
+  // Try to read an existing GUID.
+  if (base::ReadFileToStringWithMaxSize(guid_file_path, &g_client_id_guid.Get(),
+                                        GUID_SIZE)) {
+    if (base::IsValidGUID(g_client_id_guid.Get()))
+      return;
+    LOG(ERROR) << "Overwriting invalid GUID";
+  }
+
+  // We must write a new GUID.
+  g_client_id_guid.Get() = base::GenerateGUID();
+  if (!base::WriteFile(guid_file_path, g_client_id_guid.Get().c_str(),
+                       g_client_id_guid.Get().size())) {
+    // If writing fails, proceed anyway with the new GUID. It won't be persisted
+    // to the next run, but we can still collect metrics with this 1-time GUID.
+    LOG(ERROR) << "Failed to write new GUID";
+  }
+}
+
 void AwMetricsServiceClient::Initialize(
     PrefService* pref_service,
-    net::URLRequestContextGetter* request_context,
-    const base::FilePath guid_file_path) {
+    net::URLRequestContextGetter* request_context) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   DCHECK(pref_service_ == nullptr);  // Initialize should only happen once.
@@ -101,22 +124,26 @@ void AwMetricsServiceClient::Initialize(
   request_context_ = request_context;
   channel_ = GetChannelFromPackageName();
 
-  std::string* guid = new std::string;
-  // Initialization happens on the UI thread, but getting the GUID should happen
-  // on the file I/O thread. So we start to initialize, then post to get the
-  // GUID, and then pick up where we left off, back on the UI thread, in
-  // InitializeWithGUID.
+  // If Finch is enabled for WebView the GUID will already have been read at
+  // startup
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableWebViewFinch)) {
+    InitializeWithGUID();
+  } else {
     content::BrowserThread::PostTaskAndReply(
         content::BrowserThread::FILE, FROM_HERE,
-      base::Bind(&GetOrCreateGUID, guid_file_path, guid),
+        base::Bind(&AwMetricsServiceClient::GetOrCreateGUID),
         base::Bind(&AwMetricsServiceClient::InitializeWithGUID,
-                 base::Unretained(this), base::Owned(guid)));
+                   base::Unretained(this)));
+  }
 }
 
-void AwMetricsServiceClient::InitializeWithGUID(std::string* guid) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-
-  pref_service_->SetString(metrics::prefs::kMetricsClientID, *guid);
+void AwMetricsServiceClient::InitializeWithGUID() {
+  // The guid must have already been initialized at this point, either
+  // synchronously or asynchronously depending on the kEnableWebViewFinch flag
+  DCHECK_EQ(g_client_id_guid.Get().length(), GUID_SIZE);
+  pref_service_->SetString(metrics::prefs::kMetricsClientID,
+                           g_client_id_guid.Get());
 
   metrics_state_manager_ = metrics::MetricsStateManager::Create(
       pref_service_, this, base::Bind(&StoreClientInfo),

android_webview/browser/aw_metrics_service_client.h

@@ -44,9 +44,12 @@ class AwMetricsServiceClient : public metrics::MetricsServiceClient,
 
  public:
   static AwMetricsServiceClient* GetInstance();
+
+  // Retrieve the client ID or generate one if none exists
+  static void GetOrCreateGUID();
+
   void Initialize(PrefService* pref_service,
-                  net::URLRequestContextGetter* request_context,
-                  const base::FilePath guid_file_path);
+                  net::URLRequestContextGetter* request_context);
 
   // metrics::EnabledStateProvider implementation
   bool IsConsentGiven() override;
@@ -79,7 +82,7 @@ class AwMetricsServiceClient : public metrics::MetricsServiceClient,
   AwMetricsServiceClient();
   ~AwMetricsServiceClient() override;
 
-  void InitializeWithGUID(std::string* guid);
+  void InitializeWithGUID();
 
   bool is_enabled_;
   PrefService* pref_service_;

android_webview/common/aw_switches.cc

@@ -6,6 +6,7 @@
 
 namespace switches {
 
+const char kEnableWebViewFinch[] = "enable-webview-finch";
 const char kSyncOnDrawHardware[] = "sync-on-draw-hardware";
 const char kWebViewSandboxedRenderer[] = "webview-sandboxed-renderer";
 

android_webview/common/aw_switches.h

@@ -7,6 +7,7 @@
 
 namespace switches {
 
+extern const char kEnableWebViewFinch[];
 extern const char kSyncOnDrawHardware[];
 extern const char kWebViewSandboxedRenderer[];
 extern const char kWebViewEnableSafeBrowsingSupport[];