[subresource_filter] Use the SubresourceFilterObserver for PageLoadMetrics

chrome/browser/page_load_metrics/observers/subresource_filter_metrics_observer.cc

 // 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.
 
 #include "chrome/browser/page_load_metrics/observers/subresource_filter_metrics_observer.h"
 
 #include "chrome/browser/page_load_metrics/page_load_metrics_util.h"
 #include "components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h"
 #include "components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.h"
 #include "components/subresource_filter/core/common/activation_decision.h"
@@ skipping 117 matching lines @@
           navigation_handle)) {
     UMA_HISTOGRAM_ENUMERATION(
         internal::kHistogramSubresourceFilterActivationDecisionReload,
         static_cast<int>(decision),
         static_cast<int>(ActivationDecision::ACTIVATION_DECISION_MAX));
   }
 }
 
 }  // namespace
 
+SubresourceFilterMetricsObserver::SubresourceFilterMetricsObserver()
+    : scoped_observer_(this) {}
+SubresourceFilterMetricsObserver::~SubresourceFilterMetricsObserver() = default;
+
+page_load_metrics::PageLoadMetricsObserver::ObservePolicy
+SubresourceFilterMetricsObserver::OnStart(
+    content::NavigationHandle* navigation_handle,
+    const GURL& currently_committed_url,
+    bool started_in_foreground) {
+  navigation_handle_ = navigation_handle;
+  auto* observer_manager =
+      subresource_filter::SubresourceFilterObserverManager::FromWebContents(
+          navigation_handle->GetWebContents());
+  // |observer_manager| isn't constructed if the feature for subresource
+  // filtering isn't enabled.
+  if (observer_manager) {
+    scoped_observer_.Add(observer_manager);
+    return CONTINUE_OBSERVING;
+  }
+  return STOP_OBSERVING;
+}
+
 page_load_metrics::PageLoadMetricsObserver::ObservePolicy
 SubresourceFilterMetricsObserver::FlushMetricsOnAppEnterBackground(
     const page_load_metrics::mojom::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& info) {
   // FlushMetricsOnAppEnterBackground is invoked on Android in cases where the
   // app is about to be backgrounded, as part of the Activity.onPause()
   // flow. After this method is invoked, Chrome may be killed without further
   // notification, so we record final metrics collected up to this point.
   if (info.did_commit)
     OnGoingAway(timing, info, base::TimeTicks::Now());
   return STOP_OBSERVING;
 }
 
 page_load_metrics::PageLoadMetricsObserver::ObservePolicy
 SubresourceFilterMetricsObserver::OnCommit(
     content::NavigationHandle* navigation_handle,
     ukm::SourceId source_id) {
-  const auto* subres_filter =
-      ContentSubresourceFilterDriverFactory::FromWebContents(
-          navigation_handle->GetWebContents());
-  if (subres_filter)
-    LogActivationDecisionMetrics(
-        navigation_handle,
-        subres_filter->GetActivationDecisionForLastCommittedPageLoad());
+  did_commit_ = true;
+  navigation_handle_ = nullptr;
+  DCHECK(scoped_observer_.IsObservingSources());
+  DCHECK(activation_decision_.has_value());
+  LogActivationDecisionMetrics(navigation_handle, *activation_decision_);
+  scoped_observer_.RemoveAll();
   return CONTINUE_OBSERVING;
 }
 
 void SubresourceFilterMetricsObserver::OnComplete(
     const page_load_metrics::mojom::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& info) {
   OnGoingAway(timing, info, base::TimeTicks());
 }
 
 void SubresourceFilterMetricsObserver::OnLoadedResource(
@@ skipping 119 matching lines @@
     UMA_HISTOGRAM_BOOLEAN(internal::kHistogramSubresourceFilterCount, true);
   }
 }
 
 void SubresourceFilterMetricsObserver::MediaStartedPlaying(
     const content::WebContentsObserver::MediaPlayerInfo& video_type,
     bool is_in_main_frame) {
   played_media_ = true;
 }
 
+void SubresourceFilterMetricsObserver::OnSubresourceFilterGoingAway() {
+  scoped_observer_.RemoveAll();

[Bryan McQuade, 2017/06/07 00:03:24]

can this ever be invoked before commit, in which case we might not get an
activation decision? should we guard against this possibility in OnCommit?

[Charlie Harrison, 2017/06/07 00:26:40]

It shouldn't happen but we can guard against it in case the API changes. I've
added an early return if we don't have an activation_decision_.

+}
+
+void SubresourceFilterMetricsObserver::OnPageActivationComputed(
+    content::NavigationHandle* navigation_handle,
+    subresource_filter::ActivationDecision activation_decision,
+    const subresource_filter::ActivationState& activation_state) {
+  // Make sure we don't get notifications from subsequent navigations.
+  if (navigation_handle != navigation_handle_)
+    return;
+  // Ensure this will always be called at most once before commit.
+  DCHECK(!did_commit_);
+  DCHECK(!activation_decision_);
+  activation_decision_ = activation_decision;
+}
+
 void SubresourceFilterMetricsObserver::OnGoingAway(
     const page_load_metrics::mojom::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& info,
     base::TimeTicks app_background_time) {
   if (!subresource_filter_observed_)
     return;
 
   PAGE_RESOURCE_COUNT_HISTOGRAM(
       internal::kHistogramSubresourceFilterNetworkResources,
       num_network_resources_);
@@ skipping 56 matching lines @@
         foreground_duration.value());
     if (timing.paint_timing->first_paint &&
         timing.paint_timing->first_paint < foreground_duration) {
       PAGE_LOAD_LONG_HISTOGRAM(
           internal::kHistogramSubresourceFilterForegroundDurationAfterPaint,
           foreground_duration.value() -
               timing.paint_timing->first_paint.value());
     }
   }
 }