Skip to content

Commit

Permalink
[Merge to M110] Add timing histograms to estimate the impact of Close…
Browse files Browse the repository at this point in the history
…dTabCache

ClosedTabCache is a browser feature to make tab restores instant.
This CL adds the following histogram to measure the duration
between restoring a for Tab, Window or Group (RestoreType)
respectively.

- TabRestore.{RestoreType}.TimeSinceClosedUntilRestored

These are important to understand the gain in performance for ClosedTabCache when storing a tab in cache for certain time.

(cherry picked from commit b642065)

Bug: 1392386
Change-Id: I85a64381ae8d782a336768df35cfbb3d824fca16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4109456
Reviewed-by: Dana Fried <dfried@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Sreeja Kamishetty <sreejakshetty@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1092672}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4205692
Reviewed-by: Daniel Yip <danielyip@google.com>
Owners-Override: Daniel Yip <danielyip@google.com>
Auto-Submit: Sreeja Kamishetty <sreejakshetty@chromium.org>
Cr-Commit-Position: refs/branch-heads/5481@{#825}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
sreejakshetty authored and Daniel Yip committed Jan 31, 2023
1 parent 721d1b5 commit 0c27545
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 11 deletions.
71 changes: 60 additions & 11 deletions chrome/browser/sessions/tab_restore_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1609,13 +1609,40 @@ IN_PROC_BROWSER_TEST_F(TabRestoreTest,
browser()->tab_strip_model()->GetActiveWebContents()->GetURL());
}

// Check that TabManager.TimeSinceTablosedUntilRestored histogram is not
// Check that TabRestore.Tab.TimeBetweenClosedAndRestored histogram is recorded
// on tab restore.
IN_PROC_BROWSER_TEST_F(TabRestoreTest,
TimeBetweenTabClosedAndRestoredRecorded) {
base::HistogramTester histogram_tester;
const char kTimeBetweenTabClosedAndRestored[] =
"TabRestore.Tab.TimeBetweenClosedAndRestored";

int starting_tab_count = browser()->tab_strip_model()->count();
AddSomeTabs(browser(), 3);

// Close the tab in the middle.
int closed_tab_index = starting_tab_count + 1;
CloseTab(closed_tab_index);

EXPECT_EQ(
histogram_tester.GetAllSamples(kTimeBetweenTabClosedAndRestored).size(),
0U);

// Restore the tab. This should record the kTimeBetweenTabClosedAndRestored
// histogram.
RestoreTab(0, closed_tab_index);
EXPECT_EQ(
histogram_tester.GetAllSamples(kTimeBetweenTabClosedAndRestored).size(),
1U);
}

// Check that TabRestore.Window.TimeBetweenClosedAndRestored histogram is
// recorded on window restore.
IN_PROC_BROWSER_TEST_F(TabRestoreTest,
TimeSinceTabClosedNotRecordedOnWindowRestore) {
TimeBetweenWindowClosedAndRestoredRecorded) {
base::HistogramTester histogram_tester;
const char kTimeSinceTabClosedUntilRestored[] =
"TabManager.TimeSinceTabClosedUntilRestored";
const char kTimeBetweenWindowClosedAndRestored[] =
"TabRestore.Window.TimeBetweenClosedAndRestored";

// Create a new window.
ui_test_utils::NavigateToURLWithDisposition(
Expand All @@ -1634,20 +1661,42 @@ IN_PROC_BROWSER_TEST_F(TabRestoreTest,
// Close the window.
CloseBrowserSynchronously(browser());

EXPECT_EQ(
histogram_tester.GetAllSamples(kTimeSinceTabClosedUntilRestored).size(),
0U);
EXPECT_EQ(histogram_tester.GetAllSamples(kTimeBetweenWindowClosedAndRestored)
.size(),
0U);

// Restore the window.
// Restore the window. This should record kTimeBetweenWindowClosedAndRestored
// histogram.
content::WindowedNotificationObserver load_stop_observer(
content::NOTIFICATION_LOAD_STOP,
content::NotificationService::AllSources());
chrome::RestoreTab(active_browser_list_->get(0));

// Check that TabManager.TimeSinceTablosedUntilRestored was not recorded.
EXPECT_EQ(histogram_tester.GetAllSamples(kTimeBetweenWindowClosedAndRestored)
.size(),
1U);
}

// // Check that TabRestore.Group.TimeBetweenClosedAndRestored histogram is
// recorded on group restore.
IN_PROC_BROWSER_TEST_F(TabRestoreTest,
TimeBetweenGroupClosedAndRestoredRecorded) {
base::HistogramTester histogram_tester;
const char kTimeBetweenGroupClosedAndRestored[] =
"TabRestore.Group.TimeBetweenClosedAndRestored";

AddSomeTabs(browser(), 3);
tab_groups::TabGroupId group =
browser()->tab_strip_model()->AddToNewGroup({1, 2});
CloseGroup(group);

// Restore closed group. This should record kTimeBetweenGroupClosedAndRestored
// histogram.
ASSERT_NO_FATAL_FAILURE(RestoreGroup(group, 0, 1));

EXPECT_EQ(
histogram_tester.GetAllSamples(kTimeSinceTabClosedUntilRestored).size(),
0U);
histogram_tester.GetAllSamples(kTimeBetweenGroupClosedAndRestored).size(),
1U);
}

IN_PROC_BROWSER_TEST_F(TabRestoreTest, PRE_PRE_RestoreAfterMultipleRestarts) {
Expand Down
21 changes: 21 additions & 0 deletions components/sessions/core/tab_restore_service_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,13 @@ std::vector<LiveTab*> TabRestoreServiceHelper::RestoreEntryById(
switch (entry.type) {
case TabRestoreService::TAB: {
auto& tab = static_cast<const Tab&>(entry);

if (tab.timestamp != base::Time() &&
!tab.timestamp.ToDeltaSinceWindowsEpoch().is_zero()) {
UMA_HISTOGRAM_LONG_TIMES("TabRestore.Tab.TimeBetweenClosedAndRestored",
TimeNow() - tab.timestamp);
}

LiveTab* restored_tab = nullptr;
context = RestoreTab(tab, context, disposition, &restored_tab);
live_tabs.push_back(restored_tab);
Expand All @@ -457,6 +464,13 @@ std::vector<LiveTab*> TabRestoreServiceHelper::RestoreEntryById(
LiveTabContext* current_context = context;
auto& window = static_cast<Window&>(entry);

if (window.timestamp != base::Time() &&
!window.timestamp.ToDeltaSinceWindowsEpoch().is_zero()) {
UMA_HISTOGRAM_LONG_TIMES(
"TabRestore.Window.TimeBetweenClosedAndRestored",
TimeNow() - window.timestamp);
}

// When restoring a window, either the entire window can be restored, or a
// single tab within it. If the entry's ID matches the one to restore, or
// the entry corresponds to an application, then the entire window will be
Expand Down Expand Up @@ -579,6 +593,13 @@ std::vector<LiveTab*> TabRestoreServiceHelper::RestoreEntryById(
case TabRestoreService::GROUP: {
auto& group = static_cast<Group&>(entry);

if (group.timestamp != base::Time() &&
!group.timestamp.ToDeltaSinceWindowsEpoch().is_zero()) {
UMA_HISTOGRAM_LONG_TIMES(
"TabRestore.Group.TimeBetweenClosedAndRestored",
TimeNow() - group.timestamp);
}

// When restoring a group, either the entire group can be restored, or a
// single tab within it. If the entry's ID matches the one to restore,
// then the entire group will be restored.
Expand Down
16 changes: 16 additions & 0 deletions tools/metrics/histograms/metadata/tab/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1330,6 +1330,22 @@ chromium-metrics-reviews@google.com.
</summary>
</histogram>

<histogram name="TabRestore.{RestoreType}.TimeBetweenClosedAndRestored"
units="ms" expires_after="2023-07-10">
<owner>sreejakshetty@chromium.org</owner>
<owner>chrome-brapp-loading@google.com</owner>
<summary>
Recorded when the user restores a {RestoreType}. The value is the time
duration between the previous closing of the {RestoreType} and the
{RestoreType} being restored.
</summary>
<token key="RestoreType">
<variant name="Group" summary="tab group"/>
<variant name="Tab" summary="tab"/>
<variant name="Window" summary="window"/>
</token>
</histogram>

<histogram name="Tabs.CountAtResume" units="tabs" expires_after="2022-11-02">
<owner>marq@chromium.org</owner>
<owner>olivierrobin@chromium.org</owner>
Expand Down

0 comments on commit 0c27545

Please sign in to comment.