Enables UKM recorder in iOS

Bug: 738938
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I75950dc5af48af4dcb8b26db9c093cc8837034fd
Reviewed-on: https://chromium-review.googlesource.com/776035
Commit-Queue: Moe Ahmadi (OOO until Nov. 27) <mahmadi@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517724}

components/ukm/ukm_recorder_impl.h

@@ -15,6 +15,7 @@
 
 namespace metrics {
 class UkmBrowserTest;
+class UkmEGTestHelper;
 }
 
 namespace ukm {
@@ -53,6 +54,7 @@ class UkmRecorderImpl : public UkmRecorder {
 
  private:
   friend ::metrics::UkmBrowserTest;
+  friend ::metrics::UkmEGTestHelper;
   friend ::ukm::debug::DebugPage;
 
   // UkmRecorder:

components/ukm/ukm_service.h

@@ -25,6 +25,7 @@ class PrefService;
 namespace metrics {
 class MetricsServiceClient;
 class UkmBrowserTest;
+class UkmEGTestHelper;
 }
 
 namespace ukm {
@@ -76,8 +77,9 @@ class UkmService : public UkmRecorderImpl {
   static void RegisterPrefs(PrefRegistrySimple* registry);
 
  private:
-  friend ::ukm::debug::DebugPage;
   friend ::metrics::UkmBrowserTest;
+  friend ::metrics::UkmEGTestHelper;
+  friend ::ukm::debug::DebugPage;
 
   FRIEND_TEST_ALL_PREFIXES(UkmServiceTest, AddEntryWithEmptyMetrics);
   FRIEND_TEST_ALL_PREFIXES(UkmServiceTest, EntryBuilderAndSerialization);

ios/chrome/browser/ios_chrome_main_parts.mm

@@ -136,13 +136,6 @@ void IOSChromeMainParts::PreCreateThreads() {
 }
 
 void IOSChromeMainParts::PreMainMessageLoopRun() {
-  // This must occur at PreMainMessageLoopRun because |SetupMetrics()| uses the
-  // blocking pool, which is disabled until the CreateThreads phase of startup.
-  SetupMetrics();
-
-  // Now that the file thread has been started, start recording.
-  StartMetricsRecording();
-
   application_context_->PreMainMessageLoopRun();
 
   // ContentSettingsPattern need to be initialized before creating the
@@ -159,6 +152,15 @@ void IOSChromeMainParts::PreMainMessageLoopRun() {
   ios::ChromeBrowserState* last_used_browser_state =
       browser_state_manager->GetLastUsedBrowserState();
 
+  // This must occur at PreMainMessageLoopRun because |SetupMetrics()| uses the
+  // blocking pool, which is disabled until the CreateThreads phase of startup.
+  // TODO(crbug.com/786494): Investigate whether metrics recording can be
+  // initialized consistently across iOS and non-iOS platforms
+  SetupMetrics();
+
+  // Now that the file thread has been started, start recording.
+  StartMetricsRecording();
+
 #if BUILDFLAG(ENABLE_RLZ)
   // Init the RLZ library. This just schedules a task on the file thread to be
   // run sometime later. If this is the first run we record the installation

ios/chrome/browser/metrics/BUILD.gn

@@ -7,6 +7,8 @@ source_set("metrics") {
   sources = [
     "field_trial_synchronizer.cc",
     "field_trial_synchronizer.h",
+    "incognito_web_state_observer.h",
+    "incognito_web_state_observer.mm",
     "ios_chrome_metrics_service_accessor.cc",
     "ios_chrome_metrics_service_accessor.h",
     "ios_chrome_metrics_service_client.h",
@@ -53,6 +55,7 @@ source_set("metrics") {
     "//ios/chrome/browser/translate",
     "//ios/chrome/browser/variations",
     "//ios/chrome/browser/variations:ios_chrome_ui_string_overrider_factory",
+    "//ios/chrome/browser/web_state_list",
     "//ios/chrome/common",
     "//ios/web",
   ]
@@ -143,19 +146,34 @@ source_set("eg_tests") {
   testonly = true
   sources = [
     "tab_usage_recorder_egtest.mm",
+    "ukm_egtest.mm",
   ]
   deps = [
     ":metrics_internal",
     ":test_support",
     "//base",
     "//base/test:test_support",
+    "//components/browser_sync",
+    "//components/metrics",
+    "//components/metrics_services_manager",
     "//components/strings",
+    "//components/ukm",
+    "//ios/chrome/app/strings:ios_strings_grit",
+    "//ios/chrome/browser",
+    "//ios/chrome/browser/metrics",
+    "//ios/chrome/browser/signin",
+    "//ios/chrome/browser/sync",
     "//ios/chrome/browser/ui",
+    "//ios/chrome/browser/ui/authentication",
+    "//ios/chrome/browser/ui/authentication:authentication_ui",
+    "//ios/chrome/browser/ui/authentication:eg_test_support",
     "//ios/chrome/browser/ui/settings",
+    "//ios/chrome/browser/ui/tab_switcher:egtest_support",
     "//ios/chrome/browser/ui/toolbar/public",
     "//ios/chrome/browser/ui/tools_menu",
     "//ios/chrome/test/app:test_support",
     "//ios/chrome/test/earl_grey:test_support",
+    "//ios/public/provider/chrome/browser/signin:test_support",
     "//ios/testing:ios_test_support",
     "//ios/web:earl_grey_test_support",
     "//ios/web/public/test",

ios/chrome/browser/metrics/incognito_web_state_observer.h

+// Copyright 2017 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.
+
+#ifndef IOS_CHROME_BROWSER_METRICS_INCOGNITO_WEB_STATE_OBSERVER_H_
+#define IOS_CHROME_BROWSER_METRICS_INCOGNITO_WEB_STATE_OBSERVER_H_
+
+#include "base/macros.h"
+
+#include "base/scoped_observer.h"
+#import "ios/chrome/browser/tabs/tab_model_list_observer.h"
+#import "ios/chrome/browser/web_state_list/web_state_list.h"
+#import "ios/chrome/browser/web_state_list/web_state_list_observer.h"
+
+// Interface for getting notified when WebStates get added/removed to/from an
+// incognito browser state. For example, implementations can invoke
+// TabModelList::IsOffTheRecordSessionActive() in the body of the observer
+// methods to learn if incognito session is currently active (i.e., at least one
+// incognito tab is open).
+class IncognitoWebStateObserver : public TabModelListObserver,
+                                  public WebStateListObserver {
+ public:
+  IncognitoWebStateObserver();
+  ~IncognitoWebStateObserver() override;
+
+  // TabModelListObserver:
+  void TabModelRegisteredWithBrowserState(
+      TabModel* tab_model,
+      ios::ChromeBrowserState* browser_state) override;
+  void TabModelUnregisteredFromBrowserState(
+      TabModel* tab_model,
+      ios::ChromeBrowserState* browser_state) override;
+
+  // WebStateListObserver:
+  void WebStateInsertedAt(WebStateList* web_state_list,
+                          web::WebState* web_state,
+                          int index,
+                          bool activating) override;
+  void WebStateDetachedAt(WebStateList* web_state_list,
+                          web::WebState* web_state,
+                          int index) override;
+  void WebStateReplacedAt(WebStateList* web_state_list,
+                          web::WebState* old_web_state,
+                          web::WebState* new_web_state,
+                          int index) override;
+
+ protected:
+  virtual void OnIncognitoWebStateAdded() = 0;
+  virtual void OnIncognitoWebStateRemoved() = 0;
+
+ private:
+  ScopedObserver<WebStateList, IncognitoWebStateObserver> scoped_observer_;
+
+  DISALLOW_COPY_AND_ASSIGN(IncognitoWebStateObserver);
+};
+
+#endif  // IOS_CHROME_BROWSER_METRICS_INCOGNITO_WEB_STATE_OBSERVER_H_

ios/chrome/browser/metrics/incognito_web_state_observer.mm

+// Copyright 2017 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.
+
+#import "ios/chrome/browser/metrics/incognito_web_state_observer.h"
+
+#include <vector>
+
+#include "ios/chrome/browser/application_context.h"
+#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
+#include "ios/chrome/browser/browser_state/chrome_browser_state_manager.h"
+#import "ios/chrome/browser/tabs/tab_model.h"
+#import "ios/chrome/browser/tabs/tab_model_list.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+IncognitoWebStateObserver::IncognitoWebStateObserver()
+    : scoped_observer_(this) {
+  TabModelList::AddObserver(this);
+
+  // Observe all existing off-the-record TabModels' WebStateLists.
+  std::vector<ios::ChromeBrowserState*> browser_states =
+      GetApplicationContext()
+          ->GetChromeBrowserStateManager()
+          ->GetLoadedBrowserStates();
+
+  for (ios::ChromeBrowserState* browser_state : browser_states) {
+    DCHECK(!browser_state->IsOffTheRecord());
+
+    if (!browser_state->HasOffTheRecordChromeBrowserState())
+      continue;
+    ios::ChromeBrowserState* otr_browser_state =
+        browser_state->GetOffTheRecordChromeBrowserState();
+
+    NSArray<TabModel*>* tab_models =
+        TabModelList::GetTabModelsForChromeBrowserState(otr_browser_state);
+    for (TabModel* tab_model in tab_models)
+      scoped_observer_.Add([tab_model webStateList]);
+  }
+}
+
+IncognitoWebStateObserver::~IncognitoWebStateObserver() {
+  TabModelList::RemoveObserver(this);
+}
+
+void IncognitoWebStateObserver::TabModelRegisteredWithBrowserState(
+    TabModel* tab_model,
+    ios::ChromeBrowserState* browser_state) {
+  if (browser_state->IsOffTheRecord() &&
+      !scoped_observer_.IsObserving([tab_model webStateList])) {
+    scoped_observer_.Add([tab_model webStateList]);
+  }
+}
+
+void IncognitoWebStateObserver::TabModelUnregisteredFromBrowserState(
+    TabModel* tab_model,
+    ios::ChromeBrowserState* browser_state) {
+  if (browser_state->IsOffTheRecord()) {
+    DCHECK(scoped_observer_.IsObserving([tab_model webStateList]));
+    scoped_observer_.Remove([tab_model webStateList]);
+  }
+}
+
+void IncognitoWebStateObserver::WebStateInsertedAt(WebStateList* web_state_list,
+                                                   web::WebState* web_state,
+                                                   int index,
+                                                   bool activating) {
+  OnIncognitoWebStateAdded();
+}
+
+void IncognitoWebStateObserver::WebStateDetachedAt(WebStateList* web_state_list,
+                                                   web::WebState* web_state,
+                                                   int index) {
+  OnIncognitoWebStateRemoved();
+}
+
+void IncognitoWebStateObserver::WebStateReplacedAt(WebStateList* web_state_list,
+                                                   web::WebState* old_web_state,
+                                                   web::WebState* new_web_state,
+                                                   int index) {
+  // This is invoked when a Tab is replaced by another Tab without any visible
+  // UI change. There is nothing to do since the number of Tabs haven't changed.
+}

ios/chrome/browser/metrics/ios_chrome_metrics_service_accessor.cc

@@ -9,8 +9,26 @@
 #include "components/prefs/pref_service.h"
 #include "ios/chrome/browser/application_context.h"
 
+namespace {
+
+const bool* g_metrics_consent_for_testing = nullptr;
+
+}  // namespace
+
+// static
+void IOSChromeMetricsServiceAccessor::SetMetricsAndCrashReportingForTesting(
+    const bool* value) {
+  DCHECK_NE(g_metrics_consent_for_testing == nullptr, value == nullptr)
+      << "Unpaired set/reset";
+
+  g_metrics_consent_for_testing = value;
+}
+
 // static
 bool IOSChromeMetricsServiceAccessor::IsMetricsAndCrashReportingEnabled() {
+  if (g_metrics_consent_for_testing)
+    return *g_metrics_consent_for_testing;
+
   return IsMetricsReportingEnabled(GetApplicationContext()->GetLocalState());
 }
 

ios/chrome/browser/metrics/ios_chrome_metrics_service_accessor.h

@@ -22,6 +22,12 @@ class CrashesDOMHandler;
 // Since these methods are private, each user has to be explicitly declared
 // as a 'friend' below.
 class IOSChromeMetricsServiceAccessor : public metrics::MetricsServiceAccessor {
+ public:
+  // If arg is non-null, the value will be returned from future calls to
+  // IsMetricsAndCrashReportingEnabled(). Pointer must be valid until it is
+  // reset to null here.
+  static void SetMetricsAndCrashReportingForTesting(const bool* value);
+
  private:
   friend class IOSChromeMetricsServicesManagerClient;
 

ios/chrome/browser/metrics/ios_chrome_metrics_service_client.h

@@ -20,6 +20,7 @@
 #include "components/omnibox/browser/omnibox_event_global_tracker.h"
 #include "components/ukm/observers/history_delete_observer.h"
 #include "components/ukm/observers/sync_disable_observer.h"
+#import "ios/chrome/browser/metrics/incognito_web_state_observer.h"
 #include "ios/web/public/web_state/global_web_state_observer.h"
 
 class IOSChromeStabilityMetricsProvider;
@@ -40,11 +41,11 @@ class UkmService;
 
 // IOSChromeMetricsServiceClient provides an implementation of
 // MetricsServiceClient that depends on //ios/chrome/.
-class IOSChromeMetricsServiceClient
-    : public metrics::MetricsServiceClient,
-      public ukm::HistoryDeleteObserver,
-      public ukm::SyncDisableObserver,
-      public web::GlobalWebStateObserver {
+class IOSChromeMetricsServiceClient : public IncognitoWebStateObserver,
+                                      public metrics::MetricsServiceClient,
+                                      public ukm::HistoryDeleteObserver,
+                                      public ukm::SyncDisableObserver,
+                                      public web::GlobalWebStateObserver {
  public:
   ~IOSChromeMetricsServiceClient() override;
 
@@ -86,6 +87,10 @@ class IOSChromeMetricsServiceClient
   void WebStateDidStartLoading(web::WebState* web_state) override;
   void WebStateDidStopLoading(web::WebState* web_state) override;
 
+  // IncognitoWebStateObserver:
+  void OnIncognitoWebStateAdded() override;
+  void OnIncognitoWebStateRemoved() override;
+
   metrics::EnableMetricsDefault GetMetricsReportingDefaultState() override;
 
  private:

ios/chrome/browser/metrics/ios_chrome_metrics_service_client.mm

@@ -57,7 +57,7 @@
 #include "ios/chrome/browser/sync/ios_chrome_profile_sync_service_factory.h"
 #include "ios/chrome/browser/sync/ios_chrome_sync_client.h"
 #include "ios/chrome/browser/tab_parenting_global_observer.h"
-#include "ios/chrome/browser/tabs/tab_model_list.h"
+#import "ios/chrome/browser/tabs/tab_model_list.h"
 #include "ios/chrome/browser/translate/translate_ranker_metrics_provider.h"
 #include "ios/chrome/common/channel_info.h"
 #include "ios/web/public/web_thread.h"
@@ -302,6 +302,16 @@ void IOSChromeMetricsServiceClient::OnSyncPrefsChanged(bool must_purge) {
   UpdateRunningServices();
 }
 
+void IOSChromeMetricsServiceClient::OnIncognitoWebStateAdded() {
+  // Signal service manager to enable/disable UKM based on new state.
+  UpdateRunningServices();
+}
+
+void IOSChromeMetricsServiceClient::OnIncognitoWebStateRemoved() {
+  // Signal service manager to enable/disable UKM based on new state.
+  UpdateRunningServices();
+}
+
 bool IOSChromeMetricsServiceClient::IsHistorySyncEnabledOnAllProfiles() {
   return SyncDisableObserver::IsHistorySyncEnabledOnAllProfiles();
 }

ios/chrome/browser/metrics/ios_chrome_metrics_services_manager_client.mm

@@ -116,9 +116,5 @@ IOSChromeMetricsServicesManagerClient::GetMetricsStateManager() {
 }
 
 bool IOSChromeMetricsServicesManagerClient::IsIncognitoSessionActive() {
-  // return ::IsOffTheRecordSessionActive();
-  // TODO(crbug.com/734091): Conservatively set to true until there is a test
-  // to ensure it gets re-queried when an incognito tab is opened.  This
-  // effectively disables UKM.
-  return true;
+  return TabModelList::IsOffTheRecordSessionActive();
 }

ios/chrome/test/earl_grey/BUILD.gn

@@ -208,6 +208,7 @@ source_set("test_support") {
     "//ios/chrome/browser/ui/omnibox:omnibox_internal",
     "//ios/chrome/browser/ui/payments:payments_ui",
     "//ios/chrome/browser/ui/settings:settings",
+    "//ios/chrome/browser/ui/settings/cells",
     "//ios/chrome/browser/ui/static_content",
     "//ios/chrome/browser/ui/toolbar/public",
     "//ios/chrome/browser/ui/tools_menu",

ios/chrome/test/earl_grey/chrome_actions.h

@@ -24,6 +24,9 @@ id<GREYAction> LongPressElementForContextMenu(const std::string& element_id,
 // state.
 id<GREYAction> TurnCollectionViewSwitchOn(BOOL on);
 
+// Action to turn the switch of a SyncSwitchCell to the given |on| state.
+id<GREYAction> TurnSyncSwitchOn(BOOL on);
+
 }  // namespace chrome_test_util
 
 #endif  // IOS_CHROME_TEST_EARL_GREY_CHROME_ACTIONS_H_

ios/chrome/test/earl_grey/chrome_actions.mm

@@ -6,6 +6,7 @@
 
 #import "base/mac/foundation_util.h"
 #import "ios/chrome/browser/ui/collection_view/cells/collection_view_switch_item.h"
+#import "ios/chrome/browser/ui/settings/cells/sync_switch_item.h"
 #import "ios/chrome/test/app/chrome_test_util.h"
 #import "ios/web/public/test/earl_grey/web_view_actions.h"
 
@@ -36,7 +37,27 @@ id<GREYAction> TurnCollectionViewSwitchOn(BOOL on) {
               base::mac::ObjCCastStrict<CollectionViewSwitchCell>(
                   collectionViewCell);
           UISwitch* switchView = switchCell.switchView;
-          if (switchView.on ^ on) {
+          if (switchView.on != on) {
+            id<GREYAction> longPressAction = [GREYActions
+                actionForLongPressWithDuration:kGREYLongPressDefaultDuration];
+            return [longPressAction perform:switchView error:errorOrNil];
+          }
+          return YES;
+        }];
+}
+
+id<GREYAction> TurnSyncSwitchOn(BOOL on) {
+  id<GREYMatcher> constraints = grey_not(grey_systemAlertViewShown());
+  NSString* actionName = [NSString
+      stringWithFormat:@"Turn sync switch to %@ state", on ? @"ON" : @"OFF"];
+  return [GREYActionBlock
+      actionWithName:actionName
+         constraints:constraints
+        performBlock:^BOOL(id syncSwitchCell, __strong NSError** errorOrNil) {
+          SyncSwitchCell* switchCell =
+              base::mac::ObjCCastStrict<SyncSwitchCell>(syncSwitchCell);
+          UISwitch* switchView = switchCell.switchView;
+          if (switchView.on != on) {
             id<GREYAction> longPressAction = [GREYActions
                 actionForLongPressWithDuration:kGREYLongPressDefaultDuration];
             return [longPressAction perform:switchView error:errorOrNil];

ios/chrome/test/earl_grey/chrome_matchers.h

@@ -74,6 +74,11 @@ id<GREYMatcher> ShowTabsButton();
 id<GREYMatcher> CollectionViewSwitchCell(NSString* accessibilityIdentifier,
                                          BOOL isOn);
 
+// Matcher for SyncSwitchCell.
+// TODO(crbug.com/684139): Update |is_on| to something more obvious from
+// callsites.
+id<GREYMatcher> SyncSwitchCell(NSString* accessibilityLabel, BOOL is_on);
+
 // Matcher for the Open in New Tab option in the context menu when long pressing
 // a link.
 id<GREYMatcher> OpenLinkInNewTabButton();

ios/chrome/test/earl_grey/chrome_matchers.mm

@@ -20,6 +20,7 @@
 #import "ios/chrome/browser/ui/payments/payment_request_picker_view_controller.h"
 #import "ios/chrome/browser/ui/payments/payment_request_view_controller.h"
 #import "ios/chrome/browser/ui/settings/accounts_collection_view_controller.h"
+#import "ios/chrome/browser/ui/settings/cells/sync_switch_item.h"
 #import "ios/chrome/browser/ui/settings/clear_browsing_data_collection_view_controller.h"
 #import "ios/chrome/browser/ui/settings/import_data_collection_view_controller.h"
 #import "ios/chrome/browser/ui/settings/settings_collection_view_controller.h"
@@ -170,6 +171,14 @@ id<GREYMatcher> CollectionViewSwitchCell(NSString* accessibilityIdentifier,
                     nil);
 }
 
+id<GREYMatcher> SyncSwitchCell(NSString* accessibilityLabel, BOOL is_on) {
+  return grey_allOf(grey_accessibilityLabel(accessibilityLabel),
+                    grey_accessibilityValue(
+                        is_on ? l10n_util::GetNSString(IDS_IOS_SETTING_ON)
+                              : l10n_util::GetNSString(IDS_IOS_SETTING_OFF)),
+                    grey_sufficientlyVisible(), nil);
+}
+
 id<GREYMatcher> OpenLinkInNewTabButton() {
   return ButtonWithAccessibilityLabelId(IDS_IOS_CONTENT_CONTEXT_OPENLINKNEWTAB);
 }

services/metrics/public/cpp/ukm_recorder.cc

@@ -13,11 +13,7 @@
 
 namespace ukm {
 
-#if defined(OS_IOS)
-const base::Feature kUkmFeature = {"Ukm", base::FEATURE_DISABLED_BY_DEFAULT};
-#else
 const base::Feature kUkmFeature = {"Ukm", base::FEATURE_ENABLED_BY_DEFAULT};
-#endif
 
 UkmRecorder::UkmRecorder() = default;