[ios] Move metrics collection out of app url load service

Moved feature_engagement metrics to browser level
(already existing through notification)
Changed inIncognito to isUserInitiated on new tab notifications
(specific notifier are already tied to a browserState which knows its
incognito state)

Bug: 907527
Change-Id: I86fa5cef147c5c59765f03887a9d18866d2a7f5a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1528974
Commit-Queue: David Jean <djean@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#645291}

ios/chrome/browser/ui/browser_view/browser_view_controller.mm

@@ -254,11 +254,6 @@ void Record(ContextMenuHistogram action, bool is_image, bool is_link) {
   }
 }
 
-bool IsHelpURL(GURL& URL) {
-  GURL helpUrl(l10n_util::GetStringUTF16(IDS_IOS_TOOLS_MENU_HELP_URL));
-  return helpUrl == URL;
-}
-
 // Histogram that tracks user actions related to the WKWebView 3D touch link
 // preview API. These values are persisted to logs. Entries should not be
 // renumbered and numeric values should never be reused.
@@ -1184,14 +1179,12 @@ NSString* const kBrowserViewControllerSnackbarCategory =
         ->UpdateSnapshotWithCallback(nil);
   }
 
-  [self.tabModel
-      insertTabWithLoadParams:web_navigation_util::CreateWebLoadParams(
-                                  GURL(kChromeUINewTabURL),
-                                  ui::PAGE_TRANSITION_TYPED, nullptr)
-                       opener:nil
-                  openedByDOM:NO
-                      atIndex:self.tabModel.count
-                 inBackground:NO];
+  UrlLoadingServiceFactory::GetForBrowserState(self.browserState)
+      ->Load(UrlLoadParams::InNewTab(
+          web_navigation_util::CreateWebLoadParams(
+              GURL(kChromeUINewTabURL), ui::PAGE_TRANSITION_TYPED, nullptr),
+          /* in_incognito */ self.isOffTheRecord,
+          /* in_background */ NO, kLastTab));
 }
 
 - (void)appendTabAddedCompletion:(ProceduralBlock)tabAddedCompletion {
@@ -3414,11 +3407,10 @@ NSString* const kBrowserViewControllerSnackbarCategory =
       web_navigation_util::CreateWebLoadParams(
           result, ui::PAGE_TRANSITION_TYPED, &postContent);
   if (inNewTab) {
-    [self.tabModel insertTabWithLoadParams:loadParams
-                                    opener:nil
-                               openedByDOM:NO
-                                   atIndex:self.tabModel.count
-                              inBackground:NO];
+    UrlLoadingServiceFactory::GetForBrowserState(self.browserState)
+        ->Load(UrlLoadParams::InNewTab(loadParams,
+                                       /* in_incognito */ self.isOffTheRecord,
+                                       /* in_background */ NO, kLastTab));
   } else {
     UrlLoadingServiceFactory::GetForBrowserState(self.browserState)
         ->Load(UrlLoadParams::InCurrentTab(loadParams));
@@ -3464,11 +3456,11 @@ NSString* const kBrowserViewControllerSnackbarCategory =
   }
 }
 
-- (void)newTabWillLoadURL:(GURL)URL inIncognito:(BOOL)inIncognito {
-  if (!IsHelpURL(URL)) {
+- (void)newTabWillLoadURL:(GURL)URL isUserInitiated:(BOOL)isUserInitiated {
+  if (isUserInitiated) {
     // Send either the "New Tab Opened" or "New Incognito Tab" opened to the
     // feature_engagement::Tracker based on |inIncognito|.
-    feature_engagement::NotifyNewTabEvent(_browserState, inIncognito);
+    feature_engagement::NotifyNewTabEvent(_browserState, self.isOffTheRecord);
   }
 }
 
@@ -4283,6 +4275,7 @@ NSString* const kBrowserViewControllerSnackbarCategory =
       UrlLoadParams::InNewTab(helpUrl,
                               /* in_incognito */ NO,
                               /* in_background */ NO, kCurrentTab);
+  params->user_initiated = NO;
   UrlLoadingServiceFactory::GetForBrowserState(self.browserState)->Load(params);
 }
 

ios/chrome/browser/url_loading/app_url_loading_service.mm

@@ -5,7 +5,6 @@
 #import "ios/chrome/browser/url_loading/app_url_loading_service.h"
 
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
-#include "ios/chrome/browser/feature_engagement/tracker_util.h"
 #import "ios/chrome/browser/main/browser.h"
 #import "ios/chrome/browser/snapshots/snapshot_tab_helper.h"
 #import "ios/chrome/browser/tabs/tab.h"
@@ -74,18 +73,7 @@ void AppUrlLoadingService::LoadUrlInNewTab(UrlLoadParams* params) {
 
     // TODO(crbug.com/907527): move the following lines to Browser level making
     // openNewTabFromOriginPoint a delegate there. openNewTabFromOriginPoint is
-    // only called frok here. Use notifier newTabWillLoadURL and
-    // paramrs.is_user_initiated to trigger NotifyNewTabEventForCommand. Add
-    // notifier newTabDidLoadURL afterward.
-
-    // Either send or don't send the "New Tab Opened" or "Incognito Tab Opened"
-    // events to the feature_engagement::Tracker based on
-    // |params.user_initiated| and |params.in_incognito|.
-    if (params->user_initiated) {
-      feature_engagement::NotifyNewTabEvent([delegate_ currentBrowserState],
-                                            params -> in_incognito);
-    }
-
+    // only called from here.
     [delegate_ openNewTabFromOriginPoint:params->origin_point
                             focusOmnibox:params->should_focus_omnibox];
   }

ios/chrome/browser/url_loading/url_loading_notifier.h

@@ -47,12 +47,14 @@ class UrlLoadingNotifier : public KeyedService {
   // The loader initiated the |url| loading successfully.
   void TabDidLoadUrl(const GURL& url, ui::PageTransition transition_type);
 
-  // The loader will load |url| in a new tab. Next state will be
-  // NewTabDidLoadUrl.
-  void NewTabWillLoadUrl(const GURL& url, bool in_incognito);
+  // The loader will load |url| in a new tab. |user_initiated| is true of the
+  // request is explicitly user initiated, and false otherwise (like the
+  // opening on an NTP on startup or requesting the help page). Next state will
+  // be NewTabDidLoadUrl.
+  void NewTabWillLoadUrl(const GURL& url, bool user_initiated);
 
   // The loader initiated the |url| loading in a new tab successfully.
-  void NewTabDidLoadUrl(const GURL& url, bool in_incognito);
+  void NewTabDidLoadUrl(const GURL& url, bool user_initiated);
 
   // The loader will switch to an existing tab with |URL| instead of loading it.
   // Next state will be: DidSwitchToTabWithUrl.

ios/chrome/browser/url_loading/url_loading_notifier.mm

@@ -53,14 +53,16 @@ void UrlLoadingNotifier::TabDidLoadUrl(const GURL& url,
     observer.TabDidLoadUrl(url, transition_type);
 }
 
-void UrlLoadingNotifier::NewTabWillLoadUrl(const GURL& url, bool in_incognito) {
+void UrlLoadingNotifier::NewTabWillLoadUrl(const GURL& url,
+                                           bool user_initiated) {
   for (auto& observer : observers_)
-    observer.NewTabWillLoadUrl(url, in_incognito);
+    observer.NewTabWillLoadUrl(url, user_initiated);
 }
 
-void UrlLoadingNotifier::NewTabDidLoadUrl(const GURL& url, bool in_incognito) {
+void UrlLoadingNotifier::NewTabDidLoadUrl(const GURL& url,
+                                          bool user_initiated) {
   for (auto& observer : observers_)
-    observer.NewTabDidLoadUrl(url, in_incognito);
+    observer.NewTabDidLoadUrl(url, user_initiated);
 }
 
 void UrlLoadingNotifier::WillSwitchToTabWithUrl(const GURL& url,

ios/chrome/browser/url_loading/url_loading_observer_bridge.h

@@ -46,11 +46,11 @@
 // The loader will load |URL| in a new tab. Next state will be:
 // newTabDidLoadURL.
 // Invoked by UrlLoadingObserverBridge::NewTabWillLoadUrl.
-- (void)newTabWillLoadURL:(GURL)URL inIncognito:(BOOL)inIncognito;
+- (void)newTabWillLoadURL:(GURL)URL isUserInitiated:(BOOL)isUserInitiated;
 
 // The loader initiated the |URL| loading in a new tab successfully.
 // Invoked by UrlLoadingObserverBridge::NewTabDidLoadUrl.
-- (void)newTabDidLoadURL:(GURL)URL inIncognito:(BOOL)inIncognito;
+- (void)newTabDidLoadURL:(GURL)URL isUserInitiated:(BOOL)isUserInitiated;
 
 // The loader will switch to an existing tab with |URL| instead of loading it.
 // Next state will be: didSwitchToTabWithURL. Invoked by
@@ -76,8 +76,8 @@ class UrlLoadingObserverBridge {
   void TabDidReloadUrl(const GURL& url, ui::PageTransition transition_type);
   void TabDidLoadUrl(const GURL& url, ui::PageTransition transition_type);
 
-  void NewTabWillLoadUrl(const GURL& url, bool in_incognito);
-  void NewTabDidLoadUrl(const GURL& url, bool in_incognito);
+  void NewTabWillLoadUrl(const GURL& url, bool user_initiated);
+  void NewTabDidLoadUrl(const GURL& url, bool user_initiated);
 
   void WillSwitchToTabWithUrl(const GURL& url, int new_web_state_index);
   void DidSwitchToTabWithUrl(const GURL& url, int new_web_state_index);

ios/chrome/browser/url_loading/url_loading_observer_bridge.mm

@@ -54,16 +54,18 @@ void UrlLoadingObserverBridge::TabDidLoadUrl(
 }
 
 void UrlLoadingObserverBridge::NewTabWillLoadUrl(const GURL& url,
-                                                 bool in_incognito) {
-  if ([owner_ respondsToSelector:@selector(newTabWillLoadURL:inIncognito:)]) {
-    [owner_ newTabWillLoadURL:url inIncognito:in_incognito];
+                                                 bool user_initiated) {
+  if ([owner_ respondsToSelector:@selector(newTabWillLoadURL:
+                                             isUserInitiated:)]) {
+    [owner_ newTabWillLoadURL:url isUserInitiated:user_initiated];
   }
 }
 
 void UrlLoadingObserverBridge::NewTabDidLoadUrl(const GURL& url,
-                                                bool in_incognito) {
-  if ([owner_ respondsToSelector:@selector(newTabDidLoadURL:inIncognito:)]) {
-    [owner_ newTabDidLoadURL:url inIncognito:in_incognito];
+                                                bool user_initiated) {
+  if ([owner_ respondsToSelector:@selector(newTabDidLoadURL:
+                                            isUserInitiated:)]) {
+    [owner_ newTabDidLoadURL:url isUserInitiated:user_initiated];
   }
 }
 

ios/chrome/browser/url_loading/url_loading_params.h

@@ -27,6 +27,13 @@ struct UrlLoadParams {
   // disposition set to WindowOpenDisposition::CURRENT_TAB.
   static UrlLoadParams* InCurrentTab(const GURL& url);
 
+  // Initializes a UrlLoadParams intended to open in a new page.
+  static UrlLoadParams* InNewTab(
+      const web::NavigationManager::WebLoadParams& web_params,
+      bool in_incognito,
+      bool in_background,
+      OpenPosition append_to);
+
   // Initializes a UrlLoadParams intended to open in a new page.
   static UrlLoadParams* InNewTab(const GURL& url,
                                  const GURL& virtual_url,

ios/chrome/browser/url_loading/url_loading_params.mm

@@ -26,6 +26,22 @@ UrlLoadParams* UrlLoadParams::InCurrentTab(const GURL& url) {
   return InCurrentTab(web::NavigationManager::WebLoadParams(url));
 }
 
+UrlLoadParams* UrlLoadParams::InNewTab(
+    const web::NavigationManager::WebLoadParams& web_params,
+    bool in_incognito,
+    bool in_background,
+    OpenPosition append_to) {
+  UrlLoadParams* params = new UrlLoadParams();
+  params->disposition = in_background
+                            ? WindowOpenDisposition::NEW_BACKGROUND_TAB
+                            : WindowOpenDisposition::NEW_FOREGROUND_TAB;
+  params->web_params = web_params;
+  params->in_incognito = in_incognito;
+  params->append_to = append_to;
+  params->user_initiated = true;
+  return params;
+}
+
 UrlLoadParams* UrlLoadParams::InNewTab(const GURL& url,
                                        const GURL& virtual_url,
                                        const web::Referrer& referrer,
@@ -105,7 +121,15 @@ UrlLoadParams* UrlLoadParams::SwitchToTab(
   return params;
 }
 
-UrlLoadParams::UrlLoadParams() : web_params(GURL()) {}
+UrlLoadParams::UrlLoadParams()
+    : web_params(GURL()),
+      disposition(WindowOpenDisposition::NEW_FOREGROUND_TAB),
+      in_incognito(false),
+      append_to(kLastTab),
+      origin_point(CGPointZero),
+      from_chrome(false),
+      user_initiated(false),
+      should_focus_omnibox(false) {}
 
 UrlLoadParams::UrlLoadParams(const UrlLoadParams& other)
     : web_params(other.web_params), disposition(other.disposition) {}

ios/chrome/browser/url_loading/url_loading_service.mm

@@ -267,7 +267,7 @@ void UrlLoadingService::LoadUrlInNewTab(UrlLoadParams* params) {
   // Notify only after checking incognito match, otherwise the delegate will
   // take of changing the mode and try again. Notifying before the checks can
   // lead to be calling it twice, and calling 'did' below once.
-  notifier_->NewTabWillLoadUrl(params->web_params.url, params->in_incognito);
+  notifier_->NewTabWillLoadUrl(params->web_params.url, params->user_initiated);
 
   TabModel* tab_model = browser_->GetTabModel();
 
@@ -275,22 +275,15 @@ void UrlLoadingService::LoadUrlInNewTab(UrlLoadParams* params) {
   if (params->append_to == kCurrentTab)
     adjacent_tab = tab_model.currentTab;
 
-  GURL captured_url = params->web_params.url;
-  GURL captured_virtual_url = params->web_params.virtual_url;
-  web::Referrer captured_referrer = params->web_params.referrer;
   auto openTab = ^{
-    web::NavigationManager::WebLoadParams web_params(captured_url);
-    web_params.referrer = captured_referrer;
-    web_params.transition_type = ui::PAGE_TRANSITION_LINK;
-    web_params.virtual_url = captured_virtual_url;
     [tab_model
-        insertTabWithLoadParams:web_params
+        insertTabWithLoadParams:params->web_params
                          opener:adjacent_tab
                     openedByDOM:NO
                         atIndex:TabModelConstants::kTabPositionAutomatically
                    inBackground:params->in_background()];
 
-    notifier_->NewTabDidLoadUrl(captured_url, params->in_incognito);
+    notifier_->NewTabDidLoadUrl(params->web_params.url, params->user_initiated);
   };
 
   if (!params->in_background()) {