Index: chrome/browser/page_load_metrics/observers/subresource_filter_metrics_observer.cc |
diff --git a/chrome/browser/page_load_metrics/observers/subresource_filter_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/subresource_filter_metrics_observer.cc |
index 1ee8fcaf3c7321457c9448f4ce5c6810fa532c5e..97c24f2c3bd275b15945f60f2f8704a21a89fad9 100644 |
--- a/chrome/browser/page_load_metrics/observers/subresource_filter_metrics_observer.cc |
+++ b/chrome/browser/page_load_metrics/observers/subresource_filter_metrics_observer.cc |
@@ -135,6 +135,25 @@ void LogActivationDecisionMetrics(content::NavigationHandle* navigation_handle, |
} // 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) { |
+ auto* observer_manager = |
+ subresource_filter::SubresourceFilterObserverManager::FromWebContents( |
+ navigation_handle->GetWebContents()); |
+ // |observer_manager| isn't always constructed in unit tests, or if the master |
+ // feature for subresource filtering isn't enabled. |
+ if (observer_manager) |
+ scoped_observer_.Add(observer_manager); |
+ return CONTINUE_OBSERVING; |
+} |
+ |
page_load_metrics::PageLoadMetricsObserver::ObservePolicy |
SubresourceFilterMetricsObserver::FlushMetricsOnAppEnterBackground( |
const page_load_metrics::mojom::PageLoadTiming& timing, |
@@ -152,13 +171,12 @@ 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()); |
+ // If |this| is registered as an observer, it should have received the |
+ // activation decision by now. |
+ DCHECK(!scoped_observer_.IsObservingSources() || |
Bryan McQuade
2017/06/05 13:47:04
if we get to the commit and we're no longer observ
Charlie Harrison
2017/06/05 18:32:38
As discussed offline there is some confusion here.
|
+ activation_decision_.has_value()); |
+ if (activation_decision_) |
+ LogActivationDecisionMetrics(navigation_handle, *activation_decision_); |
return CONTINUE_OBSERVING; |
} |
@@ -298,6 +316,17 @@ void SubresourceFilterMetricsObserver::MediaStartedPlaying( |
played_media_ = true; |
} |
+void SubresourceFilterMetricsObserver::OnSubresourceFilterGoingAway() { |
+ scoped_observer_.RemoveAll(); |
Bryan McQuade
2017/06/05 13:47:04
does it make sense to move this logic into the bas
Charlie Harrison
2017/06/05 18:32:38
I'm not sure. I think most implementations will wa
|
+} |
+ |
+void SubresourceFilterMetricsObserver::OnPageActivationComputed( |
+ content::NavigationHandle* navigation_handle, |
+ subresource_filter::ActivationDecision activation_decision, |
+ const subresource_filter::ActivationState& activation_state) { |
+ activation_decision_ = activation_decision; |
Bryan McQuade
2017/06/05 13:47:04
can we add a DCHECK(!did_commit_); here (and set t
Charlie Harrison
2017/06/05 18:32:38
Done, great idea.
|
+} |
+ |
void SubresourceFilterMetricsObserver::OnGoingAway( |
const page_load_metrics::mojom::PageLoadTiming& timing, |
const page_load_metrics::PageLoadExtraInfo& info, |