[PM] Cleanup the site data writer logic

This cleanup some things inherited from the previous implementation,
some properties were tracked multiple time (in the PageNode itself, in
the SiteDataWriter and in the NodeData object), this removes some of the
duplication and privileges using data directly from the PageNode.

This also renames the NotifySiteVisibilityChanged function to a pair of
NotifySiteForegrounded/NotifySiteBackgrounded function for consistency
with the other functions of the SiteDataWriter class.

Change-Id: I6412e2f79d8f330214567a439c33727e87210ce0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2283297Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792438}

chrome/browser/performance_manager/persistence/site_data/site_data_cache_facade.cc

@@ -85,7 +85,7 @@ void SiteDataCacheFacade::WaitUntilCacheInitializedForTesting() {
                            auto* cache =
                                cache_factory->GetDataCacheForBrowserContext(
                                    browser_context_id);
-                           if (cache->IsRecordingForTesting()) {
+                           if (cache->IsRecording()) {
                              static_cast<SiteDataCacheImpl*>(cache)
                                  ->SetInitializationCallbackForTesting(
                                      std::move(quit_closure));
@@ -104,7 +104,8 @@ void SiteDataCacheFacade::OnURLsDeleted(
            SiteDataCacheFactory* cache_factory) {
           auto* cache =
               cache_factory->GetDataCacheForBrowserContext(browser_context_id);
-          static_cast<SiteDataCacheImpl*>(cache)->ClearAllSiteData();
+          if (cache->IsRecording())
+            static_cast<SiteDataCacheImpl*>(cache)->ClearAllSiteData();
         },
         browser_context_->UniqueId());
     SiteDataCacheFacadeFactory::GetInstance()
@@ -132,8 +133,10 @@ void SiteDataCacheFacade::OnURLsDeleted(
            SiteDataCacheFactory* cache_factory) {
           auto* cache =
               cache_factory->GetDataCacheForBrowserContext(browser_context_id);
-          static_cast<SiteDataCacheImpl*>(cache)->ClearSiteDataForOrigins(
-              origins_to_remove);
+          if (cache->IsRecording()) {
+            static_cast<SiteDataCacheImpl*>(cache)->ClearSiteDataForOrigins(
+                origins_to_remove);
+          }
         },
         browser_context_->UniqueId(), std::move(origins_to_remove));
     SiteDataCacheFacadeFactory::GetInstance()

components/performance_manager/decorators/site_data_recorder.cc

@@ -43,7 +43,8 @@ class SiteDataNodeData : public NodeAttachedDataImpl<SiteDataNodeData>,
  public:
   struct Traits : public NodeAttachedDataOwnedByNodeType<PageNodeImpl> {};
 
-  explicit SiteDataNodeData(const PageNodeImpl* page_node) {}
+  explicit SiteDataNodeData(const PageNodeImpl* page_node)
+      : page_node_(page_node) {}
   ~SiteDataNodeData() override = default;
 
   // NodeAttachedData:
@@ -66,6 +67,8 @@ class SiteDataNodeData : public NodeAttachedDataImpl<SiteDataNodeData>,
   void OnTitleUpdated();
   void OnFaviconUpdated();
 
+  void Reset();
+
   SiteDataWriter* writer() const override {
     DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
     return writer_.get();
@@ -90,22 +93,17 @@ class SiteDataNodeData : public NodeAttachedDataImpl<SiteDataNodeData>,
   void MaybeNotifyBackgroundFeatureUsage(void (SiteDataWriter::*method)(),
                                          FeatureType feature_type);
 
-  // Update |backgrounded_time_| depending on the visibility of the page.
-  void UpdateBackgroundedTime();
+  TabVisibility GetPageNodeVisibility() {
+    return page_node_->is_visible() ? TabVisibility::kForeground
+                                    : TabVisibility::kBackground;
+  }
 
   // The SiteDataCache used to serve writers for the PageNode owned by this
   // object.
   SiteDataCache* data_cache_ = nullptr;
 
-  bool is_visible_ = false;
-  bool is_loaded_ = false;
-
-  // The Origin tracked by the writer.
-  url::Origin last_origin_;
-
-  // The time at which this tab has been backgrounded, null if this tab is
-  // currently visible.
-  base::TimeTicks backgrounded_time_;
+  // The PageNode that owns this object.
+  const PageNodeImpl* page_node_ = nullptr;
 
   // The time at which this tab switched to the loaded state, null if this tab
   // is not currently loaded.
@@ -123,35 +121,32 @@ void SiteDataNodeData::OnMainFrameUrlChanged(const GURL& url,
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   url::Origin origin = url::Origin::Create(url);
 
-  if (origin == last_origin_)
+  if (writer_ && origin == writer_->Origin())
     return;
 
   // If the origin has changed then the writer should be invalidated.
-  writer_.reset();
+  Reset();
 
   if (!url.SchemeIsHTTPOrHTTPS())
     return;
 
-  last_origin_ = origin;
-  writer_ = data_cache_->GetWriterForOrigin(
-      origin, page_is_visible ? TabVisibility::kForeground
-                              : TabVisibility::kBackground);
+  writer_ = data_cache_->GetWriterForOrigin(origin);
 
-  is_visible_ = page_is_visible;
-  UpdateBackgroundedTime();
+  // The writer is assumed to be in an unloaded state by default, set the proper
+  // loading state if necessary.
+  if (!page_node_->is_loading())
+    OnIsLoadingChanged(false);
 }
 
 void SiteDataNodeData::OnIsLoadingChanged(bool is_loading) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (!writer_)
     return;
-  if (is_loading) {
-    is_loaded_ = false;
-    writer_->NotifySiteUnloaded();
+  if (is_loading && !loaded_time_.is_null()) {
+    writer_->NotifySiteUnloaded(GetPageNodeVisibility());
     loaded_time_ = base::TimeTicks();
-  } else {
-    is_loaded_ = true;
-    writer_->NotifySiteLoaded();
+  } else if (!is_loading) {
+    writer_->NotifySiteLoaded(GetPageNodeVisibility());
     loaded_time_ = base::TimeTicks::Now();
   }
 }
@@ -160,10 +155,11 @@ void SiteDataNodeData::OnIsVisibleChanged(bool is_visible) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (!writer_)
     return;
-  is_visible_ = is_visible;
-  UpdateBackgroundedTime();
-  writer_->NotifySiteVisibilityChanged(is_visible ? TabVisibility::kForeground
-                                                  : TabVisibility::kBackground);
+  if (is_visible) {
+    writer_->NotifySiteForegrounded(!page_node_->is_loading());
+  } else {
+    writer_->NotifySiteBackgrounded(!page_node_->is_loading());
+  }
 }
 
 void SiteDataNodeData::OnIsAudibleChanged(bool audible) {
@@ -190,6 +186,15 @@ void SiteDataNodeData::OnFaviconUpdated() {
       FeatureType::kFaviconChange);
 }
 
+void SiteDataNodeData::Reset() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  if (writer_ && !loaded_time_.is_null() && !page_node_->is_loading()) {
+    writer_->NotifySiteUnloaded(GetPageNodeVisibility());
+    loaded_time_ = base::TimeTicks();
+  }
+  writer_.reset();
+}
+
 bool SiteDataNodeData::ShouldIgnoreFeatureUsageEvent(FeatureType feature_type) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   // The feature usage should be ignored if there's no writer for this page.
@@ -197,11 +202,11 @@ bool SiteDataNodeData::ShouldIgnoreFeatureUsageEvent(FeatureType feature_type) {
     return true;
 
   // Ignore all features happening before the website gets fully loaded.
-  if (!is_loaded_)
+  if (page_node_->is_loading())
     return true;
 
   // Ignore events if the tab is not in background.
-  if (is_visible_)
+  if (GetPageNodeVisibility() == TabVisibility::kForeground)
     return true;
 
   if (feature_type == FeatureType::kTitleChange ||
@@ -215,8 +220,7 @@ bool SiteDataNodeData::ShouldIgnoreFeatureUsageEvent(FeatureType feature_type) {
 
   // Ignore events happening shortly after the tab being backgrounded, they're
   // usually false positives.
-  DCHECK(!backgrounded_time_.is_null());
-  if ((base::TimeTicks::Now() - backgrounded_time_ <
+  if ((page_node_->TimeSinceLastVisibilityChange() <
        kFeatureUsagePostBackgroundGracePeriod)) {
     return true;
   }
@@ -235,15 +239,6 @@ void SiteDataNodeData::MaybeNotifyBackgroundFeatureUsage(
   (writer_.get()->*method)();
 }
 
-void SiteDataNodeData::UpdateBackgroundedTime() {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  if (is_visible_) {
-    backgrounded_time_ = base::TimeTicks();
-  } else {
-    backgrounded_time_ = base::TimeTicks::Now();
-  }
-}
-
 SiteDataNodeData* GetSiteDataNodeDataFromPageNode(const PageNode* page_node) {
   auto* page_node_impl = PageNodeImpl::FromNode(page_node);
   DCHECK(page_node_impl);
@@ -254,12 +249,6 @@ SiteDataNodeData* GetSiteDataNodeDataFromPageNode(const PageNode* page_node) {
 
 }  // namespace
 
-// static
-SiteDataRecorder::Data* SiteDataRecorder::Data::GetForTesting(
-    const PageNode* page_node) {
-  return GetSiteDataNodeDataFromPageNode(page_node);
-}
-
 SiteDataRecorder::SiteDataRecorder() {
   DETACH_FROM_SEQUENCE(sequence_checker_);
 }
@@ -281,6 +270,12 @@ void SiteDataRecorder::OnPageNodeAdded(const PageNode* page_node) {
   SetPageNodeDataCache(page_node);
 }
 
+void SiteDataRecorder::OnBeforePageNodeRemoved(const PageNode* page_node) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  auto* data = GetSiteDataNodeDataFromPageNode(page_node);
+  data->Reset();
+}
+
 void SiteDataRecorder::OnMainFrameUrlChanged(const PageNode* page_node) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   auto* data = GetSiteDataNodeDataFromPageNode(page_node);
@@ -342,4 +337,10 @@ void SiteDataRecorder::SetPageNodeDataCache(const PageNode* page_node) {
 SiteDataNodeData::Data::Data() = default;
 SiteDataNodeData::Data::~Data() = default;
 
+// static
+SiteDataRecorder::Data* SiteDataRecorder::Data::GetForTesting(
+    const PageNode* page_node) {
+  return GetSiteDataNodeDataFromPageNode(page_node);
+}
+
 }  // namespace performance_manager

components/performance_manager/decorators/site_data_recorder.h

@@ -33,6 +33,7 @@ class SiteDataRecorder : public GraphOwned,
 
   // PageNode::ObserverDefaultImpl:
   void OnPageNodeAdded(const PageNode* page_node) override;
+  void OnBeforePageNodeRemoved(const PageNode* page_node) override;
   void OnMainFrameUrlChanged(const PageNode* page_node) override;
   void OnIsLoadingChanged(const PageNode* page_node) override;
   void OnIsVisibleChanged(const PageNode* page_node) override;

components/performance_manager/decorators/site_data_recorder_unittest.cc

@@ -40,7 +40,7 @@ class LenientMockDataWriter : public SiteDataWriter {
  public:
   LenientMockDataWriter(const url::Origin& origin,
                         scoped_refptr<internal::SiteDataImpl> impl)
-      : SiteDataWriter(impl, TabVisibility::kBackground), origin_(origin) {}
+      : SiteDataWriter(impl), origin_(origin) {}
   ~LenientMockDataWriter() override {
     if (on_destroy_indicator_)
       *on_destroy_indicator_ = true;
@@ -48,10 +48,10 @@ class LenientMockDataWriter : public SiteDataWriter {
   LenientMockDataWriter(const LenientMockDataWriter& other) = delete;
   LenientMockDataWriter& operator=(const LenientMockDataWriter&) = delete;
 
-  MOCK_METHOD0(NotifySiteLoaded, void());
-  MOCK_METHOD0(NotifySiteUnloaded, void());
-  MOCK_METHOD1(NotifySiteVisibilityChanged,
-               void(performance_manager::TabVisibility));
+  MOCK_METHOD1(NotifySiteLoaded, void(TabVisibility));
+  MOCK_METHOD1(NotifySiteUnloaded, void(TabVisibility));
+  MOCK_METHOD1(NotifySiteForegrounded, void(bool));
+  MOCK_METHOD1(NotifySiteBackgrounded, void(bool));
   MOCK_METHOD0(NotifyUpdatesFaviconInBackground, void());
   MOCK_METHOD0(NotifyUpdatesTitleInBackground, void());
   MOCK_METHOD0(NotifyUsesAudioInBackground, void());
@@ -87,14 +87,13 @@ class MockDataCache : public SiteDataCache {
     return nullptr;
   }
   std::unique_ptr<SiteDataWriter> GetWriterForOrigin(
-      const url::Origin& origin,
-      performance_manager::TabVisibility tab_visibility) override {
+      const url::Origin& origin) override {
     scoped_refptr<internal::SiteDataImpl> fake_impl = base::WrapRefCounted(
         new internal::SiteDataImpl(origin, &delegate_, &data_store_));
 
     return std::make_unique<MockDataWriter>(origin, fake_impl);
   }
-  bool IsRecordingForTesting() const override { return true; }
+  bool IsRecording() const override { return true; }
   int Size() const override { return 0; }
 
  private:
@@ -241,13 +240,11 @@ TEST_F(SiteDataRecorderTest, FeatureEventsGetForwardedWhenInBackground) {
     EXPECT_EQ(url::Origin::Create(kTestUrl1), mock_writer->Origin());
 
     node_impl = PageNodeImpl::FromNode(page_node.get());
-    EXPECT_CALL(*mock_writer, NotifySiteLoaded());
+    EXPECT_CALL(*mock_writer, NotifySiteLoaded(TabVisibility::kBackground));
     node_impl->SetIsLoading(false);
     ::testing::Mock::VerifyAndClear(mock_writer);
 
-    EXPECT_CALL(*mock_writer,
-                NotifySiteVisibilityChanged(
-                    performance_manager::TabVisibility::kForeground));
+    EXPECT_CALL(*mock_writer, NotifySiteForegrounded(true));
   }));
 
   web_contents()->WasShown();
@@ -263,9 +260,7 @@ TEST_F(SiteDataRecorderTest, FeatureEventsGetForwardedWhenInBackground) {
     node_impl->SetIsAudible(true);
     ::testing::Mock::VerifyAndClear(mock_writer);
 
-    EXPECT_CALL(*mock_writer,
-                NotifySiteVisibilityChanged(
-                    performance_manager::TabVisibility::kBackground));
+    EXPECT_CALL(*mock_writer, NotifySiteBackgrounded(true));
   }));
   web_contents()->WasHidden();
 
@@ -289,12 +284,8 @@ TEST_F(SiteDataRecorderTest, FeatureEventsGetForwardedWhenInBackground) {
     ::testing::Mock::VerifyAndClear(mock_writer);
 
     // Brievly switch the tab to foreground to reset the last backgrounded time.
-    EXPECT_CALL(*mock_writer,
-                NotifySiteVisibilityChanged(
-                    performance_manager::TabVisibility::kForeground));
-    EXPECT_CALL(*mock_writer,
-                NotifySiteVisibilityChanged(
-                    performance_manager::TabVisibility::kBackground));
+    EXPECT_CALL(*mock_writer, NotifySiteForegrounded(true));
+    EXPECT_CALL(*mock_writer, NotifySiteBackgrounded(true));
   }));
   web_contents()->WasShown();
   web_contents()->WasHidden();
@@ -320,7 +311,11 @@ TEST_F(SiteDataRecorderTest, FeatureEventsGetForwardedWhenInBackground) {
     node_impl->OnFaviconUpdated();
     node_impl->OnTitleUpdated();
     ::testing::Mock::VerifyAndClear(mock_writer);
+
+    EXPECT_CALL(*mock_writer, NotifySiteUnloaded(TabVisibility::kBackground));
   }));
+
+  NavigatePageNodeOnUIThread(web_contents(), GURL("about://blank"));
 }
 
 TEST_F(SiteDataRecorderTest, FeatureEventsIgnoredWhenLoadingInBackground) {
@@ -355,15 +350,11 @@ TEST_F(SiteDataRecorderTest, VisibilityEvent) {
 
     // Test that the visibility events get forwarded to the writer.
 
-    EXPECT_CALL(*mock_writer,
-                NotifySiteVisibilityChanged(
-                    performance_manager::TabVisibility::kForeground));
+    EXPECT_CALL(*mock_writer, NotifySiteForegrounded(false));
     node_impl->SetIsVisible(true);
     ::testing::Mock::VerifyAndClear(mock_writer);
 
-    EXPECT_CALL(*mock_writer,
-                NotifySiteVisibilityChanged(
-                    performance_manager::TabVisibility::kBackground));
+    EXPECT_CALL(*mock_writer, NotifySiteBackgrounded(false));
     node_impl->SetIsVisible(false);
     ::testing::Mock::VerifyAndClear(mock_writer);
   }));
@@ -380,11 +371,11 @@ TEST_F(SiteDataRecorderTest, LoadEvent) {
 
     // Test that the load/unload events get forwarded to the writer.
 
-    EXPECT_CALL(*mock_writer, NotifySiteLoaded());
+    EXPECT_CALL(*mock_writer, NotifySiteLoaded(TabVisibility::kBackground));
     node_impl->SetIsLoading(false);
     ::testing::Mock::VerifyAndClear(mock_writer);
 
-    EXPECT_CALL(*mock_writer, NotifySiteUnloaded());
+    EXPECT_CALL(*mock_writer, NotifySiteUnloaded(TabVisibility::kBackground));
     node_impl->SetIsLoading(true);
     ::testing::Mock::VerifyAndClear(mock_writer);
   }));

components/performance_manager/persistence/site_data/non_recording_site_data_cache.cc

@@ -36,14 +36,13 @@ std::unique_ptr<SiteDataReader> NonRecordingSiteDataCache::GetReaderForOrigin(
 }
 
 std::unique_ptr<SiteDataWriter> NonRecordingSiteDataCache::GetWriterForOrigin(
-    const url::Origin& origin,
-    performance_manager::TabVisibility tab_visibility) {
+    const url::Origin& origin) {
   // Return a fake data writer.
   SiteDataWriter* writer = new NoopSiteDataWriter();
   return base::WrapUnique(writer);
 }
 
-bool NonRecordingSiteDataCache::IsRecordingForTesting() const {
+bool NonRecordingSiteDataCache::IsRecording() const {
   return false;
 }
 

components/performance_manager/persistence/site_data/non_recording_site_data_cache.h

@@ -30,9 +30,8 @@ class NonRecordingSiteDataCache : public SiteDataCache,
   std::unique_ptr<SiteDataReader> GetReaderForOrigin(
       const url::Origin& origin) override;
   std::unique_ptr<SiteDataWriter> GetWriterForOrigin(
-      const url::Origin& origin,
-      performance_manager::TabVisibility tab_visibility) override;
-  bool IsRecordingForTesting() const override;
+      const url::Origin& origin) override;
+  bool IsRecording() const override;
   int Size() const override;
 
   // SiteDataCacheInspector:

components/performance_manager/persistence/site_data/non_recording_site_data_cache_unittest.cc

@@ -74,31 +74,25 @@ TEST_F(NonRecordingSiteDataCacheTest, EndToEnd) {
   // recording data cache aren't recorded.
   auto reader = non_recording_data_cache_->GetReaderForOrigin(kTestOrigin);
   EXPECT_TRUE(reader);
-  auto fake_writer = non_recording_data_cache_->GetWriterForOrigin(
-      kTestOrigin, performance_manager::TabVisibility::kBackground);
+  auto fake_writer = non_recording_data_cache_->GetWriterForOrigin(kTestOrigin);
   EXPECT_TRUE(fake_writer);
-  auto real_writer = recording_data_cache_->GetWriterForOrigin(
-      kTestOrigin, performance_manager::TabVisibility::kBackground);
+  auto real_writer = recording_data_cache_->GetWriterForOrigin(kTestOrigin);
   EXPECT_TRUE(real_writer);
 
   EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureUsageUnknown,
             reader->UpdatesTitleInBackground());
-  fake_writer->NotifySiteLoaded();
+  fake_writer->NotifySiteLoaded(TabVisibility::kBackground);
   fake_writer->NotifyUpdatesTitleInBackground();
   EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureUsageUnknown,
             reader->UpdatesTitleInBackground());
 
-  real_writer->NotifySiteLoaded();
+  real_writer->NotifySiteLoaded(TabVisibility::kBackground);
   real_writer->NotifyUpdatesTitleInBackground();
   EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureInUse,
             reader->UpdatesTitleInBackground());
 
-  // These unload events shouldn't be registered, make sure that they aren't by
-  // unloading the site more time than it has been loaded.
-  fake_writer->NotifySiteUnloaded();
-  fake_writer->NotifySiteUnloaded();
-
-  real_writer->NotifySiteUnloaded();
+  fake_writer->NotifySiteUnloaded(TabVisibility::kBackground);
+  real_writer->NotifySiteUnloaded(TabVisibility::kBackground);
 }
 
 TEST_F(NonRecordingSiteDataCacheTest, InspectorWorks) {
@@ -121,8 +115,7 @@ TEST_F(NonRecordingSiteDataCacheTest, InspectorWorks) {
   {
     // Add an entry through the writing data cache, see that it's reflected in
     // the inspector interface.
-    auto writer = recording_data_cache_->GetWriterForOrigin(
-        kTestOrigin, performance_manager::TabVisibility::kBackground);
+    auto writer = recording_data_cache_->GetWriterForOrigin(kTestOrigin);
 
     EXPECT_EQ(1U, inspector->GetAllInMemoryOrigins().size());
     EXPECT_TRUE(inspector->GetDataForOrigin(kTestOrigin, &is_dirty, &data));
@@ -130,8 +123,9 @@ TEST_F(NonRecordingSiteDataCacheTest, InspectorWorks) {
     ASSERT_NE(nullptr, data.get());
 
     // Touch the underlying data, see that the dirty bit updates.
-    writer->NotifySiteLoaded();
+    writer->NotifySiteLoaded(TabVisibility::kBackground);
     EXPECT_TRUE(inspector->GetDataForOrigin(kTestOrigin, &is_dirty, &data));
+    writer->NotifySiteUnloaded(TabVisibility::kBackground);
   }
 
   // Make sure the interface is unregistered from the browser context on

components/performance_manager/persistence/site_data/noop_site_data_writer.cc

@@ -8,12 +8,13 @@ namespace performance_manager {
 
 NoopSiteDataWriter::~NoopSiteDataWriter() = default;
 
-void NoopSiteDataWriter::NotifySiteLoaded() {}
+void NoopSiteDataWriter::NotifySiteLoaded(TabVisibility visibility) {}
 
-void NoopSiteDataWriter::NotifySiteUnloaded() {}
+void NoopSiteDataWriter::NotifySiteUnloaded(TabVisibility visibility) {}
 
-void NoopSiteDataWriter::NotifySiteVisibilityChanged(
-    performance_manager::TabVisibility visibility) {}
+void NoopSiteDataWriter::NotifySiteForegrounded(bool is_loaded) {}
+
+void NoopSiteDataWriter::NotifySiteBackgrounded(bool is_loaded) {}
 
 void NoopSiteDataWriter::NotifyUpdatesFaviconInBackground() {}
 
@@ -26,8 +27,11 @@ void NoopSiteDataWriter::NotifyLoadTimePerformanceMeasurement(
     base::TimeDelta cpu_usage_estimate,
     uint64_t private_footprint_kb_estimate) {}
 
-NoopSiteDataWriter::NoopSiteDataWriter()
-    : SiteDataWriter(nullptr, performance_manager::TabVisibility::kForeground) {
+const url::Origin& NoopSiteDataWriter::Origin() const {
+  static url::Origin dummy_origin;
+  return dummy_origin;
 }
 
+NoopSiteDataWriter::NoopSiteDataWriter() : SiteDataWriter(nullptr) {}
+
 }  // namespace performance_manager

components/performance_manager/persistence/site_data/noop_site_data_writer.h

@@ -7,6 +7,7 @@
 
 #include "base/macros.h"
 #include "components/performance_manager/persistence/site_data/site_data_writer.h"
+#include "url/origin.h"
 
 namespace performance_manager {
 
@@ -16,10 +17,10 @@ class NoopSiteDataWriter : public SiteDataWriter {
   ~NoopSiteDataWriter() override;
 
   // Implementation of SiteDataWriter:
-  void NotifySiteLoaded() override;
-  void NotifySiteUnloaded() override;
-  void NotifySiteVisibilityChanged(
-      performance_manager::TabVisibility visibility) override;
+  void NotifySiteLoaded(TabVisibility visibility) override;
+  void NotifySiteUnloaded(TabVisibility visibility) override;
+  void NotifySiteForegrounded(bool is_loaded) override;
+  void NotifySiteBackgrounded(bool is_loaded) override;
   void NotifyUpdatesFaviconInBackground() override;
   void NotifyUpdatesTitleInBackground() override;
   void NotifyUsesAudioInBackground() override;
@@ -27,6 +28,7 @@ class NoopSiteDataWriter : public SiteDataWriter {
       base::TimeDelta load_duration,
       base::TimeDelta cpu_usage_estimate,
       uint64_t private_footprint_kb_estimate) override;
+  const url::Origin& Origin() const override;
 
  private:
   friend class NonRecordingSiteDataCache;

components/performance_manager/persistence/site_data/site_data_cache.h

@@ -26,17 +26,12 @@ class SiteDataCache {
       const url::Origin& origin) = 0;
 
   // Returns a SiteDataWriter for the given origin.
-  //
-  // |tab_visibility| indicates the current visibility of the tab. The writer
-  // starts in an unloaded state, NotifyTabLoaded() must be called explicitly
-  // afterwards if the site is loaded.
   virtual std::unique_ptr<SiteDataWriter> GetWriterForOrigin(
-      const url::Origin& origin,
-      performance_manager::TabVisibility tab_visibility) = 0;
+      const url::Origin& origin) = 0;
 
-  // Indicate if the SiteDataWriter served by this data cache
-  // actually persist information.
-  virtual bool IsRecordingForTesting() const = 0;
+  // Indicate if the SiteDataWriter served by this data cache actually persists
+  // information.
+  virtual bool IsRecording() const = 0;
 
   // Returns the number of element in the cache.
   virtual int Size() const = 0;

components/performance_manager/persistence/site_data/site_data_cache_factory.cc

@@ -81,7 +81,7 @@ bool SiteDataCacheFactory::IsDataCacheRecordingForTesting(
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   auto it = data_cache_map_.find(browser_context_id);
   CHECK(it != data_cache_map_.end());
-  return it->second->IsRecordingForTesting();
+  return it->second->IsRecording();
 }
 
 void SiteDataCacheFactory::SetCacheForTesting(

components/performance_manager/persistence/site_data/site_data_cache_impl.cc

@@ -54,16 +54,15 @@ std::unique_ptr<SiteDataReader> SiteDataCacheImpl::GetReaderForOrigin(
 }
 
 std::unique_ptr<SiteDataWriter> SiteDataCacheImpl::GetWriterForOrigin(
-    const url::Origin& origin,
-    performance_manager::TabVisibility tab_visibility) {
+    const url::Origin& origin) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   internal::SiteDataImpl* impl = GetOrCreateFeatureImpl(origin);
   DCHECK(impl);
-  SiteDataWriter* data_writer = new SiteDataWriter(impl, tab_visibility);
+  SiteDataWriter* data_writer = new SiteDataWriter(impl);
   return base::WrapUnique(data_writer);
 }
 
-bool SiteDataCacheImpl::IsRecordingForTesting() const {
+bool SiteDataCacheImpl::IsRecording() const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   return true;
 }

components/performance_manager/persistence/site_data/site_data_cache_impl.h

@@ -41,9 +41,8 @@ class SiteDataCacheImpl : public SiteDataCache,
   std::unique_ptr<SiteDataReader> GetReaderForOrigin(
       const url::Origin& origin) override;
   std::unique_ptr<SiteDataWriter> GetWriterForOrigin(
-      const url::Origin& origin,
-      performance_manager::TabVisibility tab_visibility) override;
-  bool IsRecordingForTesting() const override;
+      const url::Origin& origin) override;
+  bool IsRecording() const override;
   int Size() const override;
 
   const SiteDataMap& origin_data_map_for_testing() const {

components/performance_manager/persistence/site_data/site_data_cache_impl_unittest.cc

@@ -78,8 +78,7 @@ class SiteDataCacheImplTest : public ::testing::Test {
     EXPECT_TRUE(reader_);
 
     ASSERT_FALSE(writer_);
-    writer_ = data_cache_->GetWriterForOrigin(
-        TestOrigin1(), performance_manager::TabVisibility::kBackground);
+    writer_ = data_cache_->GetWriterForOrigin(TestOrigin1());
     EXPECT_TRUE(writer_);
 
     ASSERT_FALSE(data_);
@@ -89,7 +88,7 @@ class SiteDataCacheImplTest : public ::testing::Test {
 
     EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureUsageUnknown,
               reader_->UpdatesTitleInBackground());
-    writer_->NotifySiteLoaded();
+    writer_->NotifySiteLoaded(TabVisibility::kBackground);
     writer_->NotifyUpdatesTitleInBackground();
     EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureInUse,
               reader_->UpdatesTitleInBackground());
@@ -101,8 +100,7 @@ class SiteDataCacheImplTest : public ::testing::Test {
     EXPECT_TRUE(reader2_);
 
     ASSERT_FALSE(writer2_);
-    writer2_ = data_cache_->GetWriterForOrigin(
-        TestOrigin2(), performance_manager::TabVisibility::kBackground);
+    writer2_ = data_cache_->GetWriterForOrigin(TestOrigin2());
     EXPECT_TRUE(writer2_);
 
     ASSERT_FALSE(data2_);
@@ -112,7 +110,7 @@ class SiteDataCacheImplTest : public ::testing::Test {
 
     EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureUsageUnknown,
               reader2_->UpdatesFaviconInBackground());
-    writer2_->NotifySiteLoaded();
+    writer2_->NotifySiteLoaded(TabVisibility::kBackground);
     writer2_->NotifyUpdatesFaviconInBackground();
     EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureInUse,
               reader2_->UpdatesFaviconInBackground());
@@ -141,19 +139,18 @@ class SiteDataCacheImplTest : public ::testing::Test {
 TEST_F(SiteDataCacheImplTest, EndToEnd) {
   auto reader = data_cache_->GetReaderForOrigin(TestOrigin1());
   EXPECT_TRUE(reader);
-  auto writer = data_cache_->GetWriterForOrigin(
-      TestOrigin1(), performance_manager::TabVisibility::kBackground);
+  auto writer = data_cache_->GetWriterForOrigin(TestOrigin1());
   EXPECT_TRUE(writer);
 
   EXPECT_EQ(1U, data_cache_->origin_data_map_for_testing().size());
 
   EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureUsageUnknown,
             reader->UpdatesTitleInBackground());
-  writer->NotifySiteLoaded();
+  writer->NotifySiteLoaded(TabVisibility::kBackground);
   writer->NotifyUpdatesTitleInBackground();
   EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureInUse,
             reader->UpdatesTitleInBackground());
-  writer->NotifySiteUnloaded();
+  writer->NotifySiteUnloaded(TabVisibility::kBackground);
   EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureInUse,
             reader->UpdatesTitleInBackground());
 
@@ -204,8 +201,8 @@ TEST_F(SiteDataCacheImplTest, ClearSiteDataForOrigins) {
   EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureInUse,
             reader2_->UpdatesFaviconInBackground());
 
-  writer_->NotifySiteUnloaded();
-  writer2_->NotifySiteUnloaded();
+  writer_->NotifySiteUnloaded(TabVisibility::kBackground);
+  writer2_->NotifySiteUnloaded(TabVisibility::kBackground);
 }
 
 TEST_F(SiteDataCacheImplTest, ClearAllSiteData) {
@@ -226,8 +223,8 @@ TEST_F(SiteDataCacheImplTest, ClearAllSiteData) {
   EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureUsageUnknown,
             reader2_->UpdatesFaviconInBackground());
 
-  writer_->NotifySiteUnloaded();
-  writer2_->NotifySiteUnloaded();
+  writer_->NotifySiteUnloaded(TabVisibility::kBackground);
+  writer2_->NotifySiteUnloaded(TabVisibility::kBackground);
 }
 
 TEST_F(SiteDataCacheImplTest, InspectorWorks) {
@@ -250,8 +247,7 @@ TEST_F(SiteDataCacheImplTest, InspectorWorks) {
 
   {
     // Add an entry, see that it's reflected in the inspector interface.
-    auto writer = data_cache_->GetWriterForOrigin(
-        TestOrigin1(), performance_manager::TabVisibility::kBackground);
+    auto writer = data_cache_->GetWriterForOrigin(TestOrigin1());
 
     EXPECT_EQ(1U, inspector->GetAllInMemoryOrigins().size());
     EXPECT_TRUE(inspector->GetDataForOrigin(TestOrigin1(), &is_dirty, &data));
@@ -259,9 +255,10 @@ TEST_F(SiteDataCacheImplTest, InspectorWorks) {
     ASSERT_NE(nullptr, data.get());
 
     // Touch the underlying data, see that the dirty bit updates.
-    writer->NotifySiteLoaded();
+    writer->NotifySiteLoaded(TabVisibility::kBackground);
     EXPECT_TRUE(inspector->GetDataForOrigin(TestOrigin1(), &is_dirty, &data));
     EXPECT_TRUE(is_dirty);
+    writer->NotifySiteUnloaded(TabVisibility::kBackground);
   }
 
   // Make sure the interface is unregistered from the browser context on

components/performance_manager/persistence/site_data/site_data_impl.cc

@@ -67,6 +67,7 @@ void SiteDataImpl::NotifySiteUnloaded(TabVisibility tab_visibility) {
   if (tab_visibility == TabVisibility::kBackground)
     DecrementNumLoadedBackgroundTabs();
 
+  DCHECK_GT(loaded_tabs_count_, 0U);
   loaded_tabs_count_--;
   // Only update the last loaded time when there's no more loaded instance of
   // this origin.

components/performance_manager/persistence/site_data/site_data_writer.cc

@@ -8,66 +8,49 @@
 
 namespace performance_manager {
 
-SiteDataWriter::~SiteDataWriter() {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  if (is_loaded_)
-    NotifySiteUnloaded();
-}
+SiteDataWriter::~SiteDataWriter() = default;
 
-void SiteDataWriter::NotifySiteLoaded() {
+void SiteDataWriter::NotifySiteLoaded(TabVisibility visibility) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  DCHECK(!is_loaded_);
-  is_loaded_ = true;
   impl_->NotifySiteLoaded();
 
-  if (tab_visibility_ == performance_manager::TabVisibility::kBackground)
+  if (visibility == TabVisibility::kBackground)
     impl_->NotifyLoadedSiteBackgrounded();
 }
 
-void SiteDataWriter::NotifySiteUnloaded() {
+void SiteDataWriter::NotifySiteUnloaded(TabVisibility visibility) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  DCHECK(is_loaded_);
 
-  is_loaded_ = false;
-
-  impl_->NotifySiteUnloaded(tab_visibility_);
+  impl_->NotifySiteUnloaded(visibility);
 }
 
-void SiteDataWriter::NotifySiteVisibilityChanged(
-    performance_manager::TabVisibility visibility) {
+void SiteDataWriter::NotifySiteForegrounded(bool is_loaded) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  // Ignore this if we receive the same event multiple times.
-  if (tab_visibility_ == visibility)
-    return;
+  if (is_loaded)
+    impl_->NotifyLoadedSiteForegrounded();
+}
 
-  tab_visibility_ = visibility;
+void SiteDataWriter::NotifySiteBackgrounded(bool is_loaded) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  if (is_loaded_) {
-    if (visibility == performance_manager::TabVisibility::kBackground) {
-      impl_->NotifyLoadedSiteBackgrounded();
-    } else {
-      impl_->NotifyLoadedSiteForegrounded();
-    }
-  }
+  if (is_loaded)
+    impl_->NotifyLoadedSiteBackgrounded();
 }
 
 void SiteDataWriter::NotifyUpdatesFaviconInBackground() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  DCHECK_EQ(performance_manager::TabVisibility::kBackground, tab_visibility_);
   impl_->NotifyUpdatesFaviconInBackground();
 }
 
 void SiteDataWriter::NotifyUpdatesTitleInBackground() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  DCHECK_EQ(performance_manager::TabVisibility::kBackground, tab_visibility_);
   impl_->NotifyUpdatesTitleInBackground();
 }
 
 void SiteDataWriter::NotifyUsesAudioInBackground() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  DCHECK_EQ(performance_manager::TabVisibility::kBackground, tab_visibility_);
   // TODO(sebmarchand): Do not advance the background audio observation time
   // when the WebContents has never played audio.
   impl_->NotifyUsesAudioInBackground();
@@ -82,10 +65,12 @@ void SiteDataWriter::NotifyLoadTimePerformanceMeasurement(
                                               private_footprint_kb_estimate);
 }
 
-SiteDataWriter::SiteDataWriter(
-    scoped_refptr<internal::SiteDataImpl> impl,
-    performance_manager::TabVisibility tab_visibility)
-    : impl_(std::move(impl)), tab_visibility_(tab_visibility) {
+const url::Origin& SiteDataWriter::Origin() const {
+  return impl_->origin();
+}
+
+SiteDataWriter::SiteDataWriter(scoped_refptr<internal::SiteDataImpl> impl)
+    : impl_(std::move(impl)) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 }
 

components/performance_manager/persistence/site_data/site_data_writer.h

@@ -20,12 +20,12 @@ class SiteDataWriter {
   virtual ~SiteDataWriter();
 
   // Records tab load/unload events.
-  virtual void NotifySiteLoaded();
-  virtual void NotifySiteUnloaded();
+  virtual void NotifySiteLoaded(TabVisibility visibility);
+  virtual void NotifySiteUnloaded(TabVisibility visibility);
 
   // Records visibility change events.
-  virtual void NotifySiteVisibilityChanged(
-      performance_manager::TabVisibility visibility);
+  virtual void NotifySiteForegrounded(bool is_loaded);
+  virtual void NotifySiteBackgrounded(bool is_loaded);
 
   // Records feature usage.
   virtual void NotifyUpdatesFaviconInBackground();
@@ -38,6 +38,8 @@ class SiteDataWriter {
       base::TimeDelta cpu_usage_estimate,
       uint64_t private_footprint_kb_estimate);
 
+  virtual const url::Origin& Origin() const;
+
   internal::SiteDataImpl* impl_for_testing() const { return impl_.get(); }
 
  protected:
@@ -47,19 +49,12 @@ class SiteDataWriter {
 
   // Protected constructor, these objects are meant to be created by a site data
   // store.
-  SiteDataWriter(scoped_refptr<internal::SiteDataImpl> impl,
-                 performance_manager::TabVisibility tab_visibility);
+  explicit SiteDataWriter(scoped_refptr<internal::SiteDataImpl> impl);
 
  private:
   // The SiteDataImpl object we delegate to.
   const scoped_refptr<internal::SiteDataImpl> impl_;
 
-  // The visibility of the tab using this writer.
-  performance_manager::TabVisibility tab_visibility_;
-
-  // Indicates if the tab using this writer is loaded.
-  bool is_loaded_ = false;
-
   SEQUENCE_CHECKER(sequence_checker_);
 
   DISALLOW_COPY_AND_ASSIGN(SiteDataWriter);

components/performance_manager/persistence/site_data/site_data_writer_unittest.cc

@@ -26,8 +26,7 @@ class SiteDataWriterTest : public ::testing::Test {
             new internal::SiteDataImpl(url::Origin::Create(GURL("foo.com")),
                                        &delegate_,
                                        &data_store_))) {
-    SiteDataWriter* writer = new SiteDataWriter(
-        test_impl_.get(), performance_manager::TabVisibility::kBackground);
+    SiteDataWriter* writer = new SiteDataWriter(test_impl_.get());
     writer_ = base::WrapUnique(writer);
   }
 
@@ -59,42 +58,42 @@ class SiteDataWriterTest : public ::testing::Test {
 TEST_F(SiteDataWriterTest, TestModifiers) {
   // Make sure that we initially have no information about any of the features
   // and that the site is in an unloaded state.
-  EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureUsageUnknown,
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
             test_impl_->UpdatesFaviconInBackground());
-  EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureUsageUnknown,
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
             test_impl_->UpdatesTitleInBackground());
-  EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureUsageUnknown,
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
             test_impl_->UsesAudioInBackground());
 
   // Test the OnTabLoaded function.
   EXPECT_FALSE(TabIsLoaded());
-  writer_->NotifySiteLoaded();
+  writer_->NotifySiteLoaded(TabVisibility::kBackground);
   EXPECT_TRUE(TabIsLoaded());
 
   // Test all the modifiers.
 
   writer_->NotifyUpdatesFaviconInBackground();
-  EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureInUse,
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureInUse,
             test_impl_->UpdatesFaviconInBackground());
-  EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureUsageUnknown,
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
             test_impl_->UpdatesTitleInBackground());
-  EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureUsageUnknown,
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
             test_impl_->UsesAudioInBackground());
 
   writer_->NotifyUpdatesTitleInBackground();
-  EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureInUse,
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureInUse,
             test_impl_->UpdatesFaviconInBackground());
-  EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureInUse,
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureInUse,
             test_impl_->UpdatesTitleInBackground());
-  EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureUsageUnknown,
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
             test_impl_->UsesAudioInBackground());
 
   writer_->NotifyUsesAudioInBackground();
-  EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureInUse,
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureInUse,
             test_impl_->UpdatesFaviconInBackground());
-  EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureInUse,
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureInUse,
             test_impl_->UpdatesTitleInBackground());
-  EXPECT_EQ(performance_manager::SiteFeatureUsage::kSiteFeatureInUse,
+  EXPECT_EQ(SiteFeatureUsage::kSiteFeatureInUse,
             test_impl_->UsesAudioInBackground());
 
   writer_->NotifyLoadTimePerformanceMeasurement(
@@ -108,7 +107,7 @@ TEST_F(SiteDataWriterTest, TestModifiers) {
   EXPECT_EQ(1u, test_impl_->private_footprint_kb_estimate().num_datums());
   EXPECT_EQ(1005.0, test_impl_->private_footprint_kb_estimate().value());
 
-  writer_->NotifySiteUnloaded();
+  writer_->NotifySiteUnloaded(TabVisibility::kBackground);
 }
 
 TEST_F(SiteDataWriterTest, LoadAndBackgroundStateTransitions) {
@@ -154,42 +153,38 @@ TEST_F(SiteDataWriterTest, LoadAndBackgroundStateTransitions) {
   EXPECT_FALSE(TabIsLoaded());
 
   // Transition #4: Unloaded + Bg -> Loaded + Bg.
-  writer_->NotifySiteLoaded();
+  writer_->NotifySiteLoaded(TabVisibility::kBackground);
   EXPECT_TRUE(TabIsLoadedAndInBackground());
 
   // Transition #8: Loaded + Bg -> Loaded + Fg.
-  writer_->NotifySiteVisibilityChanged(
-      performance_manager::TabVisibility::kForeground);
+  writer_->NotifySiteForegrounded(true);
   EXPECT_TRUE(TabIsLoaded());
   EXPECT_FALSE(TabIsLoadedAndInBackground());
 
   // Transition #5: Loaded + Fg -> Unloaded + Fg.
-  writer_->NotifySiteUnloaded();
+  writer_->NotifySiteUnloaded(TabVisibility::kForeground);
   EXPECT_FALSE(TabIsLoaded());
 
   // Transition #1: Unloaded + Fg -> Unloaded + Bg.
-  writer_->NotifySiteVisibilityChanged(
-      performance_manager::TabVisibility::kBackground);
+  writer_->NotifySiteBackgrounded(false);
   EXPECT_FALSE(TabIsLoaded());
 
   // Transition #2: Unloaded + Bg -> Unloaded + Fg.
-  writer_->NotifySiteVisibilityChanged(
-      performance_manager::TabVisibility::kForeground);
+  writer_->NotifySiteForegrounded(false);
   EXPECT_FALSE(TabIsLoaded());
 
   // Transition #6: Unloaded + Fg -> Loaded + Fg.
-  writer_->NotifySiteLoaded();
+  writer_->NotifySiteLoaded(TabVisibility::kForeground);
   EXPECT_TRUE(TabIsLoaded());
   EXPECT_FALSE(TabIsLoadedAndInBackground());
 
   // Transition #7: Loaded + Fg -> Loaded + Bg.
-  writer_->NotifySiteVisibilityChanged(
-      performance_manager::TabVisibility::kBackground);
+  writer_->NotifySiteBackgrounded(true);
   EXPECT_TRUE(TabIsLoaded());
   EXPECT_TRUE(TabIsLoadedAndInBackground());
 
   // Transition #3: Loaded + Bg -> Unloaded + Bg.
-  writer_->NotifySiteUnloaded();
+  writer_->NotifySiteUnloaded(TabVisibility::kBackground);
   EXPECT_FALSE(TabIsLoaded());
   EXPECT_FALSE(TabIsLoadedAndInBackground());
 }