RC: Minor fixes to the LocalSiteCharacteristicsWebContentsObserver

- Ignore navigation events happening in a subframe or within a document.
- The DataWriters assume that a site is unloaded by default, mark them
  as loaded if needed.

Bug: 773382
Change-Id: I786018b7e43925ff78849f508dfbe36d4dd891ef
Reviewed-on: https://chromium-review.googlesource.com/1099188
Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567747}

chrome/browser/resource_coordinator/local_site_characteristics_webcontents_observer.cc

@@ -67,6 +67,7 @@ void LocalSiteCharacteristicsWebContentsObserver::WebContentsDestroyed() {
     PageSignalReceiver::GetInstance()->RemoveObserver(this);
   }
   writer_.reset();
+  writer_origin_ = url::Origin();
 }
 
 void LocalSiteCharacteristicsWebContentsObserver::DidFinishNavigation(
@@ -74,6 +75,13 @@ void LocalSiteCharacteristicsWebContentsObserver::DidFinishNavigation(
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(navigation_handle);
 
+  // Ignore the navigation events happening in a subframe of in the same
+  // document.
+  if (!navigation_handle->IsInMainFrame() ||
+      navigation_handle->IsSameDocument()) {
+    return;
+  }
+
   first_time_title_set_ = false;
   first_time_favicon_set_ = false;
 
@@ -103,6 +111,12 @@ void LocalSiteCharacteristicsWebContentsObserver::DidFinishNavigation(
       new_origin,
       ContentVisibilityToRCVisibility(web_contents()->GetVisibility()));
 
+  // The writer is initially in an unloaded state, load it if necessary.
+  if (TabLoadTracker::Get()->GetLoadingState(web_contents()) ==
+      LoadingState::LOADED) {
+    writer_->NotifySiteLoaded();
+  }
+
   writer_origin_ = new_origin;
 }
 
@@ -156,11 +170,11 @@ void LocalSiteCharacteristicsWebContentsObserver::OnLoadingStateChange(
   if (!writer_)
     return;
 
-  if (new_loading_state == TabLoadTracker::LoadingState::LOADED) {
+  // Ignore the transitions from/to an UNLOADED state.
+  if (new_loading_state == LoadingState::LOADED) {
     writer_->NotifySiteLoaded();
-  } else {
-    if (old_loading_state == TabLoadTracker::LoadingState::LOADED)
-      writer_->NotifySiteUnloaded();
+  } else if (old_loading_state == LoadingState::LOADED) {
+    writer_->NotifySiteUnloaded();
   }
 }
 
@@ -176,9 +190,13 @@ void LocalSiteCharacteristicsWebContentsObserver::
 
 bool LocalSiteCharacteristicsWebContentsObserver::
     ShouldIgnoreFeatureUsageEvent() {
+  // The feature usage should be ignored if there's no writer for this tab.
   if (!writer_)
     return true;
 
+  // Features happening before the site gets loaded are also ignored.
+  // TODO(sebmarchand): Consider recording audio/notification usage before the
+  // site gets fully loaded.
   if (TabLoadTracker::Get()->GetLoadingState(web_contents()) !=
       LoadingState::LOADED) {
     return true;