Commit 241c941f authored by Charles Zhao's avatar Charles Zhao Committed by Commit Bot

Record discarded state in TabActivityWatcher.

(1) TabLifecycleObserver is added for TabActivityWatcher to track
    OnDiscardedStateChange event.

(2) In DidReplace(replaced_tab), the ukm_source_id and is_discarded
    state is copied to new_tab so that we can track discarded state
    after reload or closing.
(3) A browser test is added.

Change-Id: I9777b515d28d215f3409525b0201dc5916d35941

Bug: 872139
Change-Id: I9777b515d28d215f3409525b0201dc5916d35941
Reviewed-on: https://chromium-review.googlesource.com/1166763Reviewed-by: default avatarSteven Holte <holte@chromium.org>
Reviewed-by: default avatarChris Hamilton <chrisha@chromium.org>
Reviewed-by: default avatarMichael Giuffrida <michaelpg@chromium.org>
Reviewed-by: default avatarJohn Abd-El-Malek <jam@chromium.org>
Commit-Queue: Charles . <charleszhao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584217}
parent d3444394
......@@ -70,6 +70,7 @@
#include "chrome/browser/printing/print_job_manager.h"
#include "chrome/browser/printing/print_preview_dialog_controller.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/resource_coordinator/tab_activity_watcher.h"
#include "chrome/browser/resource_coordinator/tab_lifecycle_unit_source.h"
#include "chrome/browser/safe_browsing/safe_browsing_service.h"
#include "chrome/browser/shell_integration.h"
......@@ -887,6 +888,10 @@ resource_coordinator::TabManager* BrowserProcessImpl::GetTabManager() {
tab_manager_->intervention_policy_database(),
tab_manager_->usage_clock());
tab_lifecycle_unit_source_->AddObserver(tab_manager_.get());
// Add TabActivityWatcher to TabLifecycleObserver to track tab discards and
// reloads.
tab_lifecycle_unit_source_->AddTabLifecycleObserver(
resource_coordinator::TabActivityWatcher::GetInstance());
}
return tab_manager_.get();
#endif // defined(OS_ANDROID)
......
......@@ -91,6 +91,10 @@ class TabActivityWatcher::WebContentsData
// Copy the replaced tab's stats.
tab_metrics_.page_metrics = replaced_tab.tab_metrics_.page_metrics;
tab_metrics_.page_transition = replaced_tab.tab_metrics_.page_transition;
// Copy ukm_source_id_ and discarded state.
discarded_since_backgrounded_ = replaced_tab.discarded_since_backgrounded_;
previous_ukm_source_id_ = replaced_tab.ukm_source_id_;
}
// Call when the WebContents is detached from its tab. If the tab is later
......@@ -169,6 +173,7 @@ class TabActivityWatcher::WebContentsData
}
backgrounded_time_ = NowTicks();
discarded_since_backgrounded_ = false;
LogTabIfBackgrounded();
}
......@@ -181,9 +186,13 @@ class TabActivityWatcher::WebContentsData
return;
// Log the event before updating times.
const ukm::SourceId source_id = discarded_since_backgrounded_
? previous_ukm_source_id_
: ukm_source_id_;
TabActivityWatcher::GetInstance()
->tab_metrics_logger_->LogBackgroundTabShown(
ukm_source_id_, NowTicks() - backgrounded_time_, GetMRUFeatures());
source_id, NowTicks() - backgrounded_time_, GetMRUFeatures(),
discarded_since_backgrounded_);
backgrounded_time_ = base::TimeTicks();
foregrounded_time_ = NowTicks();
......@@ -348,6 +357,8 @@ class TabActivityWatcher::WebContentsData
// Updated when a navigation is finished.
ukm::SourceId ukm_source_id_ = 0;
// Recorded when a WebContents is replaced by another.
ukm::SourceId previous_ukm_source_id_ = 0;
// When the tab was created.
base::TimeTicks creation_time_;
......@@ -383,6 +394,9 @@ class TabActivityWatcher::WebContentsData
// event is logged.
tab_ranker::MRUFeatures mru_features_;
// Whether this tab is currently in discarded state.
bool discarded_since_backgrounded_ = false;
DISALLOW_COPY_AND_ASSIGN(WebContentsData);
};
......@@ -535,15 +549,28 @@ void TabActivityWatcher::OnTabClosed(WebContentsData* web_contents_data) {
NowTicks() - web_contents_data->navigation_time_);
// Log ForegroundedOrClosed event.
const ukm::SourceId source_id =
web_contents_data->discarded_since_backgrounded_
? web_contents_data->previous_ukm_source_id_
: web_contents_data->ukm_source_id_;
if (!web_contents_data->backgrounded_time_.is_null()) {
tab_metrics_logger_->LogBackgroundTabClosed(
web_contents_data->ukm_source_id_,
NowTicks() - web_contents_data->backgrounded_time_,
web_contents_data->GetMRUFeatures());
source_id, NowTicks() - web_contents_data->backgrounded_time_,
web_contents_data->GetMRUFeatures(),
web_contents_data->discarded_since_backgrounded_);
}
// Erase the pointer in |all_closing_tabs_| only when all logging finished.
all_closing_tabs_.erase(web_contents_data);
}
void TabActivityWatcher::OnDiscardedStateChange(content::WebContents* contents,
bool is_discarded) {
WebContentsData::FromWebContents(contents)->discarded_since_backgrounded_ |=
is_discarded;
}
void TabActivityWatcher::OnAutoDiscardableStateChange(
content::WebContents* contents,
bool is_auto_discardable) {}
} // namespace resource_coordinator
......@@ -10,6 +10,7 @@
#include "base/containers/flat_set.h"
#include "base/macros.h"
#include "base/optional.h"
#include "chrome/browser/resource_coordinator/tab_lifecycle_observer.h"
#include "chrome/browser/resource_coordinator/tab_ranker/tab_score_predictor.h"
#include "chrome/browser/ui/browser_list_observer.h"
#include "chrome/browser/ui/browser_tab_strip_tracker.h"
......@@ -25,7 +26,8 @@ namespace resource_coordinator {
// events to determine the end state of each background tab.
class TabActivityWatcher : public BrowserListObserver,
public TabStripModelObserver,
public BrowserTabStripTrackerDelegate {
public BrowserTabStripTrackerDelegate,
public TabLifecycleObserver {
public:
TabActivityWatcher();
~TabActivityWatcher() override;
......@@ -75,6 +77,12 @@ class TabActivityWatcher : public BrowserListObserver,
// BrowserTabStripTrackerDelegate:
bool ShouldTrackBrowser(Browser* browser) override;
// TabLifecycleObserver:
void OnDiscardedStateChange(content::WebContents* contents,
bool is_discarded) override;
void OnAutoDiscardableStateChange(content::WebContents* contents,
bool is_auto_discardable) override;
// Resets internal state.
void ResetForTesting();
......
......@@ -8,6 +8,8 @@
#include "base/macros.h"
#include "base/test/simple_test_tick_clock.h"
#include "chrome/browser/resource_coordinator/tab_lifecycle_unit_external.h"
#include "chrome/browser/resource_coordinator/tab_lifecycle_unit_source.h"
#include "chrome/browser/resource_coordinator/time.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_window.h"
......@@ -116,6 +118,7 @@ class TabActivityWatcherUkmTest : public TabActivityWatcherTest {
TabActivityWatcherTest::PreRunTestOnMainThread();
ukm_entry_checker_ = std::make_unique<UkmEntryChecker>();
test_ukm_recorder_ = std::make_unique<ukm::TestAutoSetUkmRecorder>();
}
void SetUpOnMainThread() override {
......@@ -149,7 +152,41 @@ class TabActivityWatcherUkmTest : public TabActivityWatcherTest {
ukm_entry_checker_->ExpectNewEntry(kFOCEntryName, url, expected_metrics);
}
// Uses test_ukm_recorder_ to check new event metrics including:
// (1) the number of UkmEntry with given event_name should equals to |size|.
// (2) the newest entry should have source_url equals to |url|.
// (3) the newest entry should have source_id equals to |source_id| if
// |source_id| is not 0 (skip for the case of 0).
// (4) the newest entry should contain all metrics in |expected_metrics|.
// Also returns the source_id of the newest entry.
ukm::SourceId ExpectedNewEntryWithSourceId(
const GURL& url,
const std::string& event_name,
size_t size,
const UkmMetricMap& expected_metrics,
ukm::SourceId source_id = 0) {
const std::vector<const ukm::mojom::UkmEntry*> entries =
test_ukm_recorder_->GetEntriesByName(event_name);
// Check size.
EXPECT_EQ(entries.size(), size);
const ukm::mojom::UkmEntry* entry = entries.back();
// Check source_url.
test_ukm_recorder_->ExpectEntrySourceHasUrl(entry, url);
// Check source_id.
if (source_id != 0) {
EXPECT_EQ(source_id, entry->source_id);
}
// Check expected_metrics.
for (const auto& metric : expected_metrics) {
test_ukm_recorder_->ExpectEntryMetric(entry, metric.first,
*metric.second);
}
// return entry source_id.
return entry->source_id;
}
std::unique_ptr<UkmEntryChecker> ukm_entry_checker_;
std::unique_ptr<ukm::TestAutoSetUkmRecorder> test_ukm_recorder_;
private:
DISALLOW_COPY_AND_ASSIGN(TabActivityWatcherUkmTest);
......@@ -329,4 +366,70 @@ IN_PROC_BROWSER_TEST_F(TabActivityWatcherUkmTest, TabDrag) {
EXPECT_EQ(0, ukm_entry_checker_->NumNewEntriesRecorded(kFOCEntryName));
}
// Tests discarded tab is recorded correctly.
IN_PROC_BROWSER_TEST_F(TabActivityWatcherUkmTest,
DiscardedTabGetsPreviousSourceId) {
ukm::SourceId ukm_source_id_for_tab_0 = 0;
ukm::SourceId ukm_source_id_for_tab_1 = 0;
ui_test_utils::NavigateToURL(browser(), test_urls_[0]);
EXPECT_EQ(0u, ukm_entry_checker_->NumEntries(kEntryName));
// Adding a new foreground tab logs the previously active tab.
AddTabAtIndex(1, test_urls_[1], ui::PAGE_TRANSITION_LINK);
{
SCOPED_TRACE("");
ukm_entry_checker_->ExpectNewEntry(
kEntryName, test_urls_[0],
{{TabManager_TabMetrics::kNavigationEntryCountName, 2}});
ukm_source_id_for_tab_0 = ExpectedNewEntryWithSourceId(
test_urls_[0], kEntryName, 1,
{{TabManager_TabMetrics::kNavigationEntryCountName, 2}});
}
// Discard the first tab.
content::WebContents* first_contents =
browser()->tab_strip_model()->GetWebContentsAt(0);
resource_coordinator::TabLifecycleUnitSource::GetInstance()
->GetTabLifecycleUnitExternal(first_contents)
->DiscardTab();
// Switching to first tab logs a forgrounded event for test_urls_[0]
// and a backgrounded event for test_urls_[1].
browser()->tab_strip_model()->ActivateTabAt(0, kIsUserGesture);
{
SCOPED_TRACE("");
ukm_entry_checker_->ExpectNewEntry(kEntryName, test_urls_[1],
kBasicMetricValues);
ukm_source_id_for_tab_1 = ExpectedNewEntryWithSourceId(
test_urls_[1], kEntryName, 2, kBasicMetricValues);
ExpectedNewEntryWithSourceId(
test_urls_[0], kFOCEntryName, 1,
{{TabManager_Background_ForegroundedOrClosed::kIsForegroundedName, 1},
{TabManager_Background_ForegroundedOrClosed::kIsDiscardedName, 1}},
ukm_source_id_for_tab_0);
}
// Discard the second tab.
content::WebContents* second_content =
browser()->tab_strip_model()->GetWebContentsAt(1);
resource_coordinator::TabLifecycleUnitSource::GetInstance()
->GetTabLifecycleUnitExternal(second_content)
->DiscardTab();
CloseBrowserSynchronously(browser());
{
SCOPED_TRACE("");
ExpectedNewEntryWithSourceId(
test_urls_[1], kFOCEntryName, 2,
{{TabManager_Background_ForegroundedOrClosed::kIsForegroundedName, 0},
{TabManager_Background_ForegroundedOrClosed::kIsDiscardedName, 1}},
ukm_source_id_for_tab_1);
}
}
} // namespace resource_coordinator
......@@ -59,6 +59,7 @@ void LogBackgroundTabForegroundedOrClosed(
base::TimeDelta inactive_duration,
const tab_ranker::MRUFeatures& mru_features,
bool is_foregrounded,
bool is_discarded,
int sequence_id) {
if (!ukm_source_id)
return;
......@@ -69,6 +70,7 @@ void LogBackgroundTabForegroundedOrClosed(
.SetMRUIndex(mru_features.index)
.SetTimeFromBackgrounded(inactive_duration.InMilliseconds())
.SetTotalTabCount(mru_features.total)
.SetIsDiscarded(is_discarded)
.Record(ukm::UkmRecorder::Get());
}
......@@ -217,19 +219,21 @@ void TabMetricsLogger::LogBackgroundTab(ukm::SourceId ukm_source_id,
void TabMetricsLogger::LogBackgroundTabShown(
ukm::SourceId ukm_source_id,
base::TimeDelta inactive_duration,
const tab_ranker::MRUFeatures& mru_features) {
const tab_ranker::MRUFeatures& mru_features,
const bool is_discarded) {
LogBackgroundTabForegroundedOrClosed(ukm_source_id, inactive_duration,
mru_features, true /* is_shown */,
++sequence_id_);
is_discarded, ++sequence_id_);
}
void TabMetricsLogger::LogBackgroundTabClosed(
ukm::SourceId ukm_source_id,
base::TimeDelta inactive_duration,
const tab_ranker::MRUFeatures& mru_features) {
const tab_ranker::MRUFeatures& mru_features,
const bool is_discarded) {
LogBackgroundTabForegroundedOrClosed(ukm_source_id, inactive_duration,
mru_features, false /* is_shown */,
++sequence_id_);
is_discarded, ++sequence_id_);
}
void TabMetricsLogger::LogTabLifetime(ukm::SourceId ukm_source_id,
......
......@@ -66,13 +66,15 @@ class TabMetricsLogger {
// shown after being inactive.
void LogBackgroundTabShown(ukm::SourceId ukm_source_id,
base::TimeDelta inactive_duration,
const tab_ranker::MRUFeatures& mru_metrics);
const tab_ranker::MRUFeatures& mru_metrics,
const bool is_discarded);
// Logs TabManager.Background.ForegroundedOrClosed UKM for a tab that was
// closed after being inactive.
void LogBackgroundTabClosed(ukm::SourceId ukm_source_id,
base::TimeDelta inactive_duration,
const tab_ranker::MRUFeatures& mru_metrics);
const tab_ranker::MRUFeatures& mru_metrics,
const bool is_discarded);
// Logs TabManager.TabLifetime UKM for a closed tab.
void LogTabLifetime(ukm::SourceId ukm_source_id,
......
......@@ -160,10 +160,11 @@ class TabMetricsLoggerUKMTest : public ::testing::Test {
TEST_F(TabMetricsLoggerUKMTest, LogBackgroundTabShown) {
const tab_ranker::MRUFeatures& mru_metrics{4, 7};
const int64_t inactive_duration_ms = 1234;
const bool is_discarded = false;
GetLogger()->LogBackgroundTabShown(
GetSourceId(), base::TimeDelta::FromMilliseconds(inactive_duration_ms),
mru_metrics);
mru_metrics, is_discarded);
// Checks that the size is logged correctly.
EXPECT_EQ(1U, GetTestUkmRecorder()->sources_count());
......@@ -182,15 +183,19 @@ TEST_F(TabMetricsLoggerUKMTest, LogBackgroundTabShown) {
inactive_duration_ms);
GetTestUkmRecorder()->ExpectEntryMetric(entries[0], "TotalTabCount",
mru_metrics.total);
GetTestUkmRecorder()->ExpectEntryMetric(entries[0], "IsDiscarded",
is_discarded);
}
// Checks the closed event is logged correctly.
TEST_F(TabMetricsLoggerUKMTest, LogBackgroundTabClosed) {
const tab_ranker::MRUFeatures& mru_metrics{4, 7};
const int64_t inactive_duration_ms = 1234;
const bool is_discarded = true;
GetLogger()->LogBackgroundTabClosed(
GetSourceId(), base::TimeDelta::FromMilliseconds(inactive_duration_ms),
mru_metrics);
mru_metrics, is_discarded);
// Checks that the size is logged correctly.
EXPECT_EQ(1U, GetTestUkmRecorder()->sources_count());
......@@ -209,15 +214,18 @@ TEST_F(TabMetricsLoggerUKMTest, LogBackgroundTabClosed) {
inactive_duration_ms);
GetTestUkmRecorder()->ExpectEntryMetric(entries[0], "TotalTabCount",
mru_metrics.total);
GetTestUkmRecorder()->ExpectEntryMetric(entries[0], "IsDiscarded",
is_discarded);
}
// Checks the sequence id is logged as sequentially incremental sequence across
// different events.
TEST_F(TabMetricsLoggerUKMTest, SequenceIdShouldBeLoggedSequentially) {
const bool is_discarded = false;
GetLogger()->LogBackgroundTabShown(GetSourceId(), base::TimeDelta(),
tab_ranker::MRUFeatures());
tab_ranker::MRUFeatures(), is_discarded);
GetLogger()->LogBackgroundTabClosed(GetSourceId(), base::TimeDelta(),
tab_ranker::MRUFeatures());
tab_ranker::MRUFeatures(), is_discarded);
EXPECT_EQ(2U, GetTestUkmRecorder()->sources_count());
EXPECT_EQ(2U, GetTestUkmRecorder()->entries_count());
......
......@@ -3632,6 +3632,12 @@ be describing additional metrics about the same event.
Collects the duration in MS from when the tab is backgrounded to when it is
brought to foreground or closed.
</summary>
<metric name="IsDiscarded">
<summary>
Boolean value indicating whether or not the tab is discarded since it was
backgrounded.
</summary>
</metric>
<metric name="IsForegrounded">
<summary>
Boolean value indicating whether or not it's triggered because the tab is
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment