Keep ServiceWorkerMetrics::Site in ServiceWorkerVersion

To reduce the string comparison in SiteFromURL().

This CL depends on https://codereview.chromium.org/2251633002/.

https://codereview.chromium.org/2249063004/ depends on this CL.

BUG=561209

Review-Url: https://codereview.chromium.org/2250623003
Cr-Commit-Position: refs/heads/master@{#413393}

content/browser/service_worker/service_worker_controllee_request_handler.cc

@@ -287,9 +287,7 @@ void ServiceWorkerControlleeRequestHandler::
             ServiceWorkerVersion::FetchHandlerExistence::UNKNOWN);
 
   ServiceWorkerMetrics::CountControlledPageLoad(
-      stripped_url_, active_version->fetch_handler_existence() ==
-                         ServiceWorkerVersion::FetchHandlerExistence::EXISTS,
-      is_main_frame_load_);
+      active_version->site_for_uma(), stripped_url_, is_main_frame_load_);
 
   bool is_forwarded =
       MaybeForwardToServiceWorker(job_.get(), active_version.get());
@@ -321,9 +319,7 @@ void ServiceWorkerControlleeRequestHandler::OnVersionStatusChanged(
   DCHECK_NE(version->fetch_handler_existence(),
             ServiceWorkerVersion::FetchHandlerExistence::UNKNOWN);
   ServiceWorkerMetrics::CountControlledPageLoad(
-      stripped_url_, version->fetch_handler_existence() ==
-                         ServiceWorkerVersion::FetchHandlerExistence::EXISTS,
-      is_main_frame_load_);
+      version->site_for_uma(), stripped_url_, is_main_frame_load_);
 
   provider_host_->AssociateRegistration(registration,
                                         false /* notify_controllerchange */);

content/browser/service_worker/service_worker_metrics.cc

@@ -144,21 +144,6 @@ void RecordURLMetricOnUI(const GURL& url) {
       "ServiceWorker.ControlledPageUrl", url);
 }
 
-ServiceWorkerMetrics::Site SiteFromURL(const GURL& gurl) {
-  // UIThreadSearchTermsData::GoogleBaseURLValue() returns the google base
-  // URL, but not available in content layer.
-  static const char google_like_scope_prefix[] = "https://www.google.";
-  static const char ntp_scope_path[] = "/_/chrome/";
-  if (base::StartsWith(gurl.spec(), google_like_scope_prefix,
-                       base::CompareCase::INSENSITIVE_ASCII) &&
-      base::StartsWith(gurl.path(), ntp_scope_path,
-                       base::CompareCase::SENSITIVE)) {
-    return ServiceWorkerMetrics::Site::NEW_TAB_PAGE;
-  }
-
-  return ServiceWorkerMetrics::Site::OTHER;
-}
-
 enum EventHandledRatioType {
   EVENT_HANDLED_NONE,
   EVENT_HANDLED_SOME,
@@ -213,6 +198,21 @@ const char* ServiceWorkerMetrics::EventTypeToString(EventType event_type) {
   return "error";
 }
 
+ServiceWorkerMetrics::Site ServiceWorkerMetrics::SiteFromURL(const GURL& url) {
+  // UIThreadSearchTermsData::GoogleBaseURLValue() returns the google base
+  // URL, but not available in content layer.
+  static const char google_like_scope_prefix[] = "https://www.google.";
+  static const char ntp_scope_path[] = "/_/chrome/";
+  if (base::StartsWith(url.spec(), google_like_scope_prefix,
+                       base::CompareCase::INSENSITIVE_ASCII) &&
+      base::StartsWith(url.path(), ntp_scope_path,
+                       base::CompareCase::SENSITIVE)) {
+    return ServiceWorkerMetrics::Site::NEW_TAB_PAGE;
+  }
+
+  return ServiceWorkerMetrics::Site::OTHER;
+}
+
 bool ServiceWorkerMetrics::IsNavigationHintEvent(EventType event_type) {
   return event_type == EventType::NAVIGATION_HINT_LINK_MOUSE_DOWN ||
          event_type == EventType::NAVIGATION_HINT_LINK_TAP_UNCONFIRMED ||
@@ -223,10 +223,6 @@ bool ServiceWorkerMetrics::ShouldExcludeSiteFromHistogram(Site site) {
   return site == ServiceWorkerMetrics::Site::NEW_TAB_PAGE;
 }
 
-bool ServiceWorkerMetrics::ShouldExcludeURLFromHistogram(const GURL& url) {
-  return ShouldExcludeSiteFromHistogram(SiteFromURL(url));
-}
-
 void ServiceWorkerMetrics::CountInitDiskCacheResult(bool result) {
   UMA_HISTOGRAM_BOOLEAN("ServiceWorker.DiskCache.InitResult", result);
 }
@@ -278,14 +274,10 @@ void ServiceWorkerMetrics::RecordDeleteAndStartOverResult(
                             result, NUM_DELETE_AND_START_OVER_RESULT_TYPES);
 }
 
-void ServiceWorkerMetrics::CountControlledPageLoad(const GURL& url,
-                                                   bool has_fetch_handler,
+void ServiceWorkerMetrics::CountControlledPageLoad(Site site,
+                                                   const GURL& url,
                                                    bool is_main_frame_load) {
-  Site site = SiteFromURL(url);
-  if (site == Site::OTHER) {
-    site = (has_fetch_handler) ? Site::WITH_FETCH_HANDLER
-                               : Site::WITHOUT_FETCH_HANDLER;
-  }
+  DCHECK_NE(site, Site::OTHER);
   UMA_HISTOGRAM_ENUMERATION("ServiceWorker.PageLoad", static_cast<int>(site),
                             static_cast<int>(Site::NUM_TYPES));
   if (is_main_frame_load) {

content/browser/service_worker/service_worker_metrics.h

@@ -115,7 +115,8 @@ class ServiceWorkerMetrics {
 
   // Used for UMA. Append only.
   enum class Site {
-    OTHER,  // Obsolete
+    OTHER,  // Obsolete for UMA. Use WITH_FETCH_HANDLER or
+            // WITHOUT_FETCH_HANDLER.
     NEW_TAB_PAGE,
     WITH_FETCH_HANDLER,
     WITHOUT_FETCH_HANDLER,
@@ -136,6 +137,9 @@ class ServiceWorkerMetrics {
   // Converts an event type to a string. Used for tracing.
   static const char* EventTypeToString(EventType event_type);
 
+  // If the |url| is not a special site, returns Site::OTHER.
+  static Site SiteFromURL(const GURL& url);
+
   // Returns true when the event is for a navigation hint.
   static bool IsNavigationHintEvent(EventType event_type);
 
@@ -143,7 +147,6 @@ class ServiceWorkerMetrics {
   // makes the results largely skewed. Some metrics don't follow this policy
   // and hence don't call this function.
   static bool ShouldExcludeSiteFromHistogram(Site site);
-  static bool ShouldExcludeURLFromHistogram(const GURL& url);
 
   // Used for ServiceWorkerDiskCache.
   static void CountInitDiskCacheResult(bool result);
@@ -161,8 +164,8 @@ class ServiceWorkerMetrics {
   static void RecordDeleteAndStartOverResult(DeleteAndStartOverResult result);
 
   // Counts the number of page loads controlled by a Service Worker.
-  static void CountControlledPageLoad(const GURL& url,
-                                      bool has_fetch_handler,
+  static void CountControlledPageLoad(Site site,
+                                      const GURL& url,
                                       bool is_main_frame_load);
 
   // Records the result of trying to start a worker. |is_installed| indicates

content/browser/service_worker/service_worker_url_request_job.cc

@@ -679,7 +679,7 @@ void ServiceWorkerURLRequestJob::DidPrepareFetchEvent(
       initial_worker_status_ != EmbeddedWorkerStatus::RUNNING) {
     return;
   }
-  if (ServiceWorkerMetrics::ShouldExcludeURLFromHistogram(request()->url()))
+  if (version->should_exclude_from_uma())
     return;
   ServiceWorkerMetrics::RecordActivatedWorkerPreparationTimeForMainFrame(
       worker_ready_time_ - request()->creation_time(), initial_worker_status_,

content/browser/service_worker/service_worker_version.cc

@@ -306,11 +306,12 @@ ServiceWorkerVersion::ServiceWorkerVersion(
       script_url_(script_url),
       scope_(registration->pattern()),
       fetch_handler_existence_(FetchHandlerExistence::UNKNOWN),
+      site_for_uma_(ServiceWorkerMetrics::SiteFromURL(scope_)),
       context_(context),
       script_cache_map_(this, context),
       ping_controller_(new PingController(this)),
       should_exclude_from_uma_(
-          ServiceWorkerMetrics::ShouldExcludeURLFromHistogram(scope_)),
+          ServiceWorkerMetrics::ShouldExcludeSiteFromHistogram(site_for_uma_)),
       weak_factory_(this) {
   DCHECK_NE(kInvalidServiceWorkerVersionId, version_id);
   DCHECK(context_);
@@ -411,6 +412,19 @@ ServiceWorkerVersionInfo ServiceWorkerVersion::GetInfo() {
   return info;
 }
 
+void ServiceWorkerVersion::set_fetch_handler_existence(
+    FetchHandlerExistence existence) {
+  DCHECK_EQ(fetch_handler_existence_, FetchHandlerExistence::UNKNOWN);
+  DCHECK_NE(existence, FetchHandlerExistence::UNKNOWN);
+  fetch_handler_existence_ = existence;
+  if (site_for_uma_ != ServiceWorkerMetrics::Site::OTHER)
+    return;
+  if (existence == FetchHandlerExistence::EXISTS)
+    site_for_uma_ = ServiceWorkerMetrics::Site::WITH_FETCH_HANDLER;
+  else
+    site_for_uma_ = ServiceWorkerMetrics::Site::WITHOUT_FETCH_HANDLER;
+}
+
 void ServiceWorkerVersion::StartWorker(ServiceWorkerMetrics::EventType purpose,
                                        const StatusCallback& callback) {
   TRACE_EVENT_INSTANT2(

content/browser/service_worker/service_worker_version.h

@@ -148,11 +148,10 @@ class CONTENT_EXPORT ServiceWorkerVersion
   FetchHandlerExistence fetch_handler_existence() const {
     return fetch_handler_existence_;
   }
-  void set_fetch_handler_existence(FetchHandlerExistence existence) {
-    DCHECK_EQ(fetch_handler_existence_, FetchHandlerExistence::UNKNOWN);
-    DCHECK_NE(existence, FetchHandlerExistence::UNKNOWN);
-    fetch_handler_existence_ = existence;
-  }
+  // This also updates |site_for_uma_| when it was Site::OTHER.
+  void set_fetch_handler_existence(FetchHandlerExistence existence);
+
+  bool should_exclude_from_uma() const { return should_exclude_from_uma_; }
 
   const std::vector<GURL>& foreign_fetch_scopes() const {
     return foreign_fetch_scopes_;
@@ -168,6 +167,8 @@ class CONTENT_EXPORT ServiceWorkerVersion
     foreign_fetch_origins_ = origins;
   }
 
+  ServiceWorkerMetrics::Site site_for_uma() const { return site_for_uma_; }
+
   // This sets the new status and also run status change callbacks
   // if there're any (see RegisterStatusChangeCallback).
   void SetStatus(Status status);
@@ -679,6 +680,7 @@ class CONTENT_EXPORT ServiceWorkerVersion
   std::vector<GURL> foreign_fetch_scopes_;
   std::vector<url::Origin> foreign_fetch_origins_;
   FetchHandlerExistence fetch_handler_existence_;
+  ServiceWorkerMetrics::Site site_for_uma_;
 
   Status status_ = NEW;
   std::unique_ptr<EmbeddedWorkerInstance> embedded_worker_;