Keep ServiceWorkerMetrics::Site in ServiceWorkerVersion

content/browser/service_worker/service_worker_version.h

Index: content/browser/service_worker/service_worker_version.h
diff --git a/content/browser/service_worker/service_worker_version.h b/content/browser/service_worker/service_worker_version.h
index c6dc1eb91de3f5298969696b782c7c78c823a57b..30c0383fa94f411e55901aeedace3eb61fcea23e 100644
--- a/content/browser/service_worker/service_worker_version.h
+++ b/content/browser/service_worker/service_worker_version.h
@@ -139,10 +139,8 @@ class CONTENT_EXPORT ServiceWorkerVersion
   // or when an installed version is loaded from the storage. When a new version
   // is not installed yet, returns base::nullopt.
   base::Optional<bool> has_fetch_handler() const { return has_fetch_handler_; }
-  void set_has_fetch_handler(bool has_fetch_handler) {
-    DCHECK(!has_fetch_handler_);
-    has_fetch_handler_ = has_fetch_handler;
-  }
+  // This also updates |site_for_uma_| when it was Site::OTHER.
+  void set_has_fetch_handler(bool has_fetch_handler);
 
   const std::vector<GURL>& foreign_fetch_scopes() const {
     return foreign_fetch_scopes_;
@@ -158,6 +156,8 @@ class CONTENT_EXPORT ServiceWorkerVersion
     foreign_fetch_origins_ = origins;
   }
 
+  ServiceWorkerMetrics::Site site_for_uma() const { return site_for_uma_; }

[shimazu, 2016/08/18 04:47:36]

I think storing Site as |site_for_uma_| is adding a bit complexity of keeping
the state up to date. If there is no other concern (about performance?), I
prefer calling SiteFromURL(script_url_, has_fetch_handler_).
WDYT?

[horo, 2016/08/18 05:44:54]

We will use this Site for UMA of all fetch events.
https://codereview.chromium.org/2218943002/
I don't think calling SiteFromURL() every time is a good idea.

For example, HistogramCustomizer in the renderer process keep
common_host_histogram_suffix_ which is set in the page navigation
and will be used when recording UMAs.

[shimazu, 2016/08/18 06:53:24]

I see. Let's follow HistogramCustomizer:)

# The reason of the comment was that I considered SiteFromURL wouldn't take much
time (I guess it might be at most ~1us or so?).

+
   // This sets the new status and also run status change callbacks
   // if there're any (see RegisterStatusChangeCallback).
   void SetStatus(Status status);
@@ -670,6 +670,8 @@ class CONTENT_EXPORT ServiceWorkerVersion
   std::vector<url::Origin> foreign_fetch_origins_;
   base::Optional<bool> has_fetch_handler_;
 
+  ServiceWorkerMetrics::Site site_for_uma_;
+
   Status status_ = NEW;
   std::unique_ptr<EmbeddedWorkerInstance> embedded_worker_;
   std::vector<StatusCallback> start_callbacks_;