Commit c68e2fab authored by Scott Violet's avatar Scott Violet Committed by Chromium LUCI CQ

sessions: wires up calls to logging session service events

I suspect I'm going to need some tuning of exit. Unfortunately
chrome shutdown path is pretty hard to observe outside of the code
itself.

BUG=1167296
TEST=SessionServiceLogTest*

Change-Id: Iae2647695be8f6e6932406bbb88df2dd2ab76504
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2641117
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: default avatarDavid Bienvenu <davidbienvenu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845789}
parent 40da1a40
...@@ -37,6 +37,7 @@ ...@@ -37,6 +37,7 @@
#include "chrome/browser/sessions/session_restore_delegate.h" #include "chrome/browser/sessions/session_restore_delegate.h"
#include "chrome/browser/sessions/session_service.h" #include "chrome/browser/sessions/session_service.h"
#include "chrome/browser/sessions/session_service_factory.h" #include "chrome/browser/sessions/session_service_factory.h"
#include "chrome/browser/sessions/session_service_log.h"
#include "chrome/browser/sessions/session_service_utils.h" #include "chrome/browser/sessions/session_service_utils.h"
#include "chrome/browser/sessions/tab_loader.h" #include "chrome/browser/sessions/tab_loader.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
...@@ -109,6 +110,7 @@ class SessionRestoreImpl : public BrowserListObserver { ...@@ -109,6 +110,7 @@ class SessionRestoreImpl : public BrowserListObserver {
bool synchronous, bool synchronous,
bool clobber_existing_tab, bool clobber_existing_tab,
bool always_create_tabbed_browser, bool always_create_tabbed_browser,
bool log_event,
const std::vector<GURL>& urls_to_open, const std::vector<GURL>& urls_to_open,
SessionRestore::CallbackList* callbacks) SessionRestore::CallbackList* callbacks)
: profile_(profile), : profile_(profile),
...@@ -116,6 +118,7 @@ class SessionRestoreImpl : public BrowserListObserver { ...@@ -116,6 +118,7 @@ class SessionRestoreImpl : public BrowserListObserver {
synchronous_(synchronous), synchronous_(synchronous),
clobber_existing_tab_(clobber_existing_tab), clobber_existing_tab_(clobber_existing_tab),
always_create_tabbed_browser_(always_create_tabbed_browser), always_create_tabbed_browser_(always_create_tabbed_browser),
log_event_(log_event),
urls_to_open_(urls_to_open), urls_to_open_(urls_to_open),
active_window_id_(SessionID::InvalidValue()), active_window_id_(SessionID::InvalidValue()),
restore_started_(base::TimeTicks::Now()), restore_started_(base::TimeTicks::Now()),
...@@ -344,9 +347,14 @@ class SessionRestoreImpl : public BrowserListObserver { ...@@ -344,9 +347,14 @@ class SessionRestoreImpl : public BrowserListObserver {
Browser* ProcessSessionWindowsAndNotify( Browser* ProcessSessionWindowsAndNotify(
std::vector<std::unique_ptr<sessions::SessionWindow>>* windows, std::vector<std::unique_ptr<sessions::SessionWindow>>* windows,
SessionID active_window_id) { SessionID active_window_id) {
int window_count = 0;
int tab_count = 0;
std::vector<RestoredTab> contents; std::vector<RestoredTab> contents;
Browser* result = Browser* result = ProcessSessionWindows(
ProcessSessionWindows(windows, active_window_id, &contents); windows, active_window_id, &contents, &window_count, &tab_count);
// TODO(sky): plumb through whether there is an error.
if (log_event_)
LogSessionServiceRestoreEvent(profile_, window_count, tab_count, false);
on_session_restored_callbacks_->Notify(static_cast<int>(contents.size())); on_session_restored_callbacks_->Notify(static_cast<int>(contents.size()));
return result; return result;
} }
...@@ -354,7 +362,9 @@ class SessionRestoreImpl : public BrowserListObserver { ...@@ -354,7 +362,9 @@ class SessionRestoreImpl : public BrowserListObserver {
Browser* ProcessSessionWindows( Browser* ProcessSessionWindows(
std::vector<std::unique_ptr<sessions::SessionWindow>>* windows, std::vector<std::unique_ptr<sessions::SessionWindow>>* windows,
SessionID active_window_id, SessionID active_window_id,
std::vector<RestoredTab>* created_contents) { std::vector<RestoredTab>* created_contents,
int* window_count,
int* tab_count) {
DVLOG(1) << "ProcessSessionWindows " << windows->size(); DVLOG(1) << "ProcessSessionWindows " << windows->size();
if (windows->empty()) { if (windows->empty()) {
...@@ -392,6 +402,7 @@ class SessionRestoreImpl : public BrowserListObserver { ...@@ -392,6 +402,7 @@ class SessionRestoreImpl : public BrowserListObserver {
} }
for (auto i = windows->begin(); i != windows->end(); ++i) { for (auto i = windows->begin(); i != windows->end(); ++i) {
++(*window_count);
// 1. Choose between restoring tabs in an existing browser or in a newly // 1. Choose between restoring tabs in an existing browser or in a newly
// created browser. // created browser.
Browser* browser = nullptr; Browser* browser = nullptr;
...@@ -446,6 +457,8 @@ class SessionRestoreImpl : public BrowserListObserver { ...@@ -446,6 +457,8 @@ class SessionRestoreImpl : public BrowserListObserver {
// However, with desks restore enabled, a window is restored to its parent // However, with desks restore enabled, a window is restored to its parent
// desk, which can be non-active desk, and left invisible but unminimized. // desk, which can be non-active desk, and left invisible but unminimized.
RestoreTabsToBrowser(*(*i), browser, initial_tab_count, created_contents); RestoreTabsToBrowser(*(*i), browser, initial_tab_count, created_contents);
(*tab_count) += (static_cast<int>(browser->tab_strip_model()->count()) -
initial_tab_count);
#if BUILDFLAG(IS_CHROMEOS_ASH) #if BUILDFLAG(IS_CHROMEOS_ASH)
DCHECK(browser->window()->IsVisible() || DCHECK(browser->window()->IsVisible() ||
browser->window()->IsMinimized() || browser->window()->IsMinimized() ||
...@@ -733,6 +746,9 @@ class SessionRestoreImpl : public BrowserListObserver { ...@@ -733,6 +746,9 @@ class SessionRestoreImpl : public BrowserListObserver {
// at least one window is created. // at least one window is created.
const bool always_create_tabbed_browser_; const bool always_create_tabbed_browser_;
// If true, LogSessionServiceRestoreEvent() is called after restore.
const bool log_event_;
// Set of URLs to open in addition to those restored from the session. // Set of URLs to open in addition to those restored from the session.
std::vector<GURL> urls_to_open_; std::vector<GURL> urls_to_open_;
...@@ -783,11 +799,12 @@ Browser* SessionRestore::RestoreSession( ...@@ -783,11 +799,12 @@ Browser* SessionRestore::RestoreSession(
} }
profile->set_restored_last_session(true); profile->set_restored_last_session(true);
// SessionRestoreImpl takes care of deleting itself when done. // SessionRestoreImpl takes care of deleting itself when done.
SessionRestoreImpl* restorer = new SessionRestoreImpl( SessionRestoreImpl* restorer =
profile, browser, (behavior & SYNCHRONOUS) != 0, new SessionRestoreImpl(profile, browser, (behavior & SYNCHRONOUS) != 0,
(behavior & CLOBBER_CURRENT_TAB) != 0, (behavior & CLOBBER_CURRENT_TAB) != 0,
(behavior & ALWAYS_CREATE_TABBED_BROWSER) != 0, urls_to_open, (behavior & ALWAYS_CREATE_TABBED_BROWSER) != 0,
SessionRestore::on_session_restored_callbacks()); /* log_event */ true, urls_to_open,
SessionRestore::on_session_restored_callbacks());
return restorer->Restore(); return restorer->Restore();
} }
...@@ -834,7 +851,7 @@ std::vector<Browser*> SessionRestore::RestoreForeignSessionWindows( ...@@ -834,7 +851,7 @@ std::vector<Browser*> SessionRestore::RestoreForeignSessionWindows(
std::vector<const sessions::SessionWindow*>::const_iterator end) { std::vector<const sessions::SessionWindow*>::const_iterator end) {
std::vector<GURL> gurls; std::vector<GURL> gurls;
SessionRestoreImpl restorer(profile, static_cast<Browser*>(nullptr), true, SessionRestoreImpl restorer(profile, static_cast<Browser*>(nullptr), true,
false, true, gurls, false, true, /* log_event */ false, gurls,
on_session_restored_callbacks()); on_session_restored_callbacks());
return restorer.RestoreForeignSession(begin, end); return restorer.RestoreForeignSession(begin, end);
} }
...@@ -847,7 +864,8 @@ WebContents* SessionRestore::RestoreForeignSessionTab( ...@@ -847,7 +864,8 @@ WebContents* SessionRestore::RestoreForeignSessionTab(
Browser* browser = chrome::FindBrowserWithWebContents(source_web_contents); Browser* browser = chrome::FindBrowserWithWebContents(source_web_contents);
Profile* profile = browser->profile(); Profile* profile = browser->profile();
std::vector<GURL> gurls; std::vector<GURL> gurls;
SessionRestoreImpl restorer(profile, browser, true, false, false, gurls, SessionRestoreImpl restorer(profile, browser, true, false, false,
/* log_event */ false, gurls,
on_session_restored_callbacks()); on_session_restored_callbacks());
return restorer.RestoreForeignTab(tab, disposition); return restorer.RestoreForeignTab(tab, disposition);
} }
......
...@@ -23,6 +23,7 @@ ...@@ -23,6 +23,7 @@
#include "chrome/browser/apps/app_service/launch_utils.h" #include "chrome/browser/apps/app_service/launch_utils.h"
#include "chrome/browser/background/background_mode_manager.h" #include "chrome/browser/background/background_mode_manager.h"
#include "chrome/browser/browser_process.h" #include "chrome/browser/browser_process.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/defaults.h" #include "chrome/browser/defaults.h"
#include "chrome/browser/prefs/session_startup_pref.h" #include "chrome/browser/prefs/session_startup_pref.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
...@@ -32,6 +33,7 @@ ...@@ -32,6 +33,7 @@
#include "chrome/browser/sessions/session_common_utils.h" #include "chrome/browser/sessions/session_common_utils.h"
#include "chrome/browser/sessions/session_data_deleter.h" #include "chrome/browser/sessions/session_data_deleter.h"
#include "chrome/browser/sessions/session_restore.h" #include "chrome/browser/sessions/session_restore.h"
#include "chrome/browser/sessions/session_service_log.h"
#include "chrome/browser/sessions/session_service_utils.h" #include "chrome/browser/sessions/session_service_utils.h"
#include "chrome/browser/sessions/tab_restore_service_factory.h" #include "chrome/browser/sessions/tab_restore_service_factory.h"
#include "chrome/browser/ui/browser_list.h" #include "chrome/browser/ui/browser_list.h"
...@@ -53,6 +55,7 @@ ...@@ -53,6 +55,7 @@
#include "components/tab_groups/tab_group_visual_data.h" #include "components/tab_groups/tab_group_visual_data.h"
#include "content/public/browser/navigation_details.h" #include "content/public/browser/navigation_details.h"
#include "content/public/browser/navigation_entry.h" #include "content/public/browser/navigation_entry.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/session_storage_namespace.h" #include "content/public/browser/session_storage_namespace.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
...@@ -108,6 +111,11 @@ SessionService::SessionService(const base::FilePath& save_path) ...@@ -108,6 +111,11 @@ SessionService::SessionService(const base::FilePath& save_path)
} }
SessionService::~SessionService() { SessionService::~SessionService() {
// Certain code paths explicitly destroy the SessionService as part of
// shutdown.
if (!did_log_exit_)
LogExitEvent();
// The BrowserList should outlive the SessionService since it's static and // The BrowserList should outlive the SessionService since it's static and
// the SessionService is a KeyedService. // the SessionService is a KeyedService.
BrowserList::RemoveObserver(this); BrowserList::RemoveObserver(this);
...@@ -322,10 +330,12 @@ void SessionService::WindowClosing(const SessionID& window_id) { ...@@ -322,10 +330,12 @@ void SessionService::WindowClosing(const SessionID& window_id) {
} }
} }
} }
if (use_pending_close) if (use_pending_close) {
LogExitEvent();
pending_window_close_ids_.insert(window_id); pending_window_close_ids_.insert(window_id);
else } else {
window_closing_ids_.insert(window_id); window_closing_ids_.insert(window_id);
}
} }
void SessionService::WindowClosed(const SessionID& window_id) { void SessionService::WindowClosed(const SessionID& window_id) {
...@@ -345,10 +355,12 @@ void SessionService::WindowClosed(const SessionID& window_id) { ...@@ -345,10 +355,12 @@ void SessionService::WindowClosed(const SessionID& window_id) {
pending_window_close_ids_.end()) { pending_window_close_ids_.end()) {
// We'll hit this if user closed the last tab in a window. // We'll hit this if user closed the last tab in a window.
has_open_trackable_browsers_ = HasOpenTrackableBrowsers(window_id); has_open_trackable_browsers_ = HasOpenTrackableBrowsers(window_id);
if (!has_open_trackable_browsers_) if (!has_open_trackable_browsers_) {
LogExitEvent();
pending_window_close_ids_.insert(window_id); pending_window_close_ids_.insert(window_id);
else } else {
ScheduleCommand(sessions::CreateWindowClosedCommand(window_id)); ScheduleCommand(sessions::CreateWindowClosedCommand(window_id));
}
} }
MaybeDeleteSessionOnlyData(); MaybeDeleteSessionOnlyData();
} }
...@@ -597,6 +609,8 @@ void SessionService::TabNavigationPathEntriesDeleted(const SessionID& window_id, ...@@ -597,6 +609,8 @@ void SessionService::TabNavigationPathEntriesDeleted(const SessionID& window_id,
void SessionService::Init() { void SessionService::Init() {
BrowserList::AddObserver(this); BrowserList::AddObserver(this);
registrar_.Add(this, chrome::NOTIFICATION_CLOSE_ALL_BROWSERS_REQUEST,
content::NotificationService::AllSources());
} }
bool SessionService::ShouldRestoreWindowOfType( bool SessionService::ShouldRestoreWindowOfType(
...@@ -651,6 +665,14 @@ bool SessionService::RestoreIfNecessary(const std::vector<GURL>& urls_to_open, ...@@ -651,6 +665,14 @@ bool SessionService::RestoreIfNecessary(const std::vector<GURL>& urls_to_open,
return false; return false;
} }
void SessionService::Observe(int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) {
// NOTE: this is necessary for the session-ending code path.
DCHECK_EQ(type, chrome::NOTIFICATION_CLOSE_ALL_BROWSERS_REQUEST);
LogExitEvent();
}
void SessionService::OnBrowserSetLastActive(Browser* browser) { void SessionService::OnBrowserSetLastActive(Browser* browser) {
if (ShouldTrackBrowser(browser)) if (ShouldTrackBrowser(browser))
ScheduleCommand( ScheduleCommand(
...@@ -859,6 +881,7 @@ void SessionService::ScheduleCommand( ...@@ -859,6 +881,7 @@ void SessionService::ScheduleCommand(
DCHECK(command); DCHECK(command);
if (ReplacePendingCommand(command_storage_manager_.get(), &command)) if (ReplacePendingCommand(command_storage_manager_.get(), &command))
return; return;
bool is_closing_command = IsClosingCommand(command.get()); bool is_closing_command = IsClosingCommand(command.get());
command_storage_manager_->ScheduleCommand(std::move(command)); command_storage_manager_->ScheduleCommand(std::move(command));
// Don't schedule a reset on tab closed/window closed. Otherwise we may // Don't schedule a reset on tab closed/window closed. Otherwise we may
...@@ -883,6 +906,8 @@ void SessionService::CommitPendingCloses() { ...@@ -883,6 +906,8 @@ void SessionService::CommitPendingCloses() {
ScheduleCommand(sessions::CreateWindowClosedCommand(*i)); ScheduleCommand(sessions::CreateWindowClosedCommand(*i));
} }
pending_window_close_ids_.clear(); pending_window_close_ids_.clear();
RemoveExitEvent();
} }
bool SessionService::IsOnlyOneTabLeft() const { bool SessionService::IsOnlyOneTabLeft() const {
...@@ -1005,3 +1030,33 @@ bool SessionService::GetAvailableRangeForTest(const SessionID& tab_id, ...@@ -1005,3 +1030,33 @@ bool SessionService::GetAvailableRangeForTest(const SessionID& tab_id,
*range = i->second; *range = i->second;
return true; return true;
} }
void SessionService::LogExitEvent() {
if (!profile_)
return;
// If there are pending closes, then we have already logged the exit.
if (!pending_window_close_ids_.empty())
return;
RemoveExitEvent();
int browser_count = 0;
int tab_count = 0;
for (auto* browser : *BrowserList::GetInstance()) {
if (browser->profile() == profile_) {
++browser_count;
tab_count += browser->tab_strip_model()->count();
}
}
did_log_exit_ = true;
LogSessionServiceExitEvent(profile_, browser_count, tab_count);
}
void SessionService::RemoveExitEvent() {
if (!did_log_exit_)
return;
RemoveLastSessionServiceEventOfType(profile_,
SessionServiceEventLogType::kExit);
did_log_exit_ = false;
}
...@@ -30,6 +30,8 @@ ...@@ -30,6 +30,8 @@
#include "components/sessions/core/tab_restore_service_client.h" #include "components/sessions/core/tab_restore_service_client.h"
#include "components/tab_groups/tab_group_id.h" #include "components/tab_groups/tab_group_id.h"
#include "components/tab_groups/tab_group_visual_data.h" #include "components/tab_groups/tab_group_visual_data.h"
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
#include "third_party/skia/include/core/SkColor.h" #include "third_party/skia/include/core/SkColor.h"
#include "ui/base/ui_base_types.h" #include "ui/base/ui_base_types.h"
...@@ -69,7 +71,8 @@ struct SessionWindow; ...@@ -69,7 +71,8 @@ struct SessionWindow;
class SessionService : public sessions::CommandStorageManagerDelegate, class SessionService : public sessions::CommandStorageManagerDelegate,
public sessions::SessionTabHelperDelegate, public sessions::SessionTabHelperDelegate,
public KeyedService, public KeyedService,
public BrowserListObserver { public BrowserListObserver,
public content::NotificationObserver {
friend class SessionServiceTestHelper; friend class SessionServiceTestHelper;
public: public:
// Creates a SessionService for the specified profile. // Creates a SessionService for the specified profile.
...@@ -248,6 +251,11 @@ class SessionService : public sessions::CommandStorageManagerDelegate, ...@@ -248,6 +251,11 @@ class SessionService : public sessions::CommandStorageManagerDelegate,
bool RestoreIfNecessary(const std::vector<GURL>& urls_to_open, bool RestoreIfNecessary(const std::vector<GURL>& urls_to_open,
Browser* browser); Browser* browser);
// content::NotificationObserver.
void Observe(int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) override;
// BrowserListObserver // BrowserListObserver
void OnBrowserAdded(Browser* browser) override {} void OnBrowserAdded(Browser* browser) override {}
void OnBrowserRemoved(Browser* browser) override {} void OnBrowserRemoved(Browser* browser) override {}
...@@ -332,6 +340,14 @@ class SessionService : public sessions::CommandStorageManagerDelegate, ...@@ -332,6 +340,14 @@ class SessionService : public sessions::CommandStorageManagerDelegate,
bool GetAvailableRangeForTest(const SessionID& tab_id, bool GetAvailableRangeForTest(const SessionID& tab_id,
std::pair<int, int>* range); std::pair<int, int>* range);
// If necessary, removes the current exit event and adds a new one. This
// does nothing if `pending_window_close_ids_` is empty, which means the user
// is potentially closing the last browser.
void LogExitEvent();
// If an exit event was logged, it is removed.
void RemoveExitEvent();
// The profile. This may be null during testing. // The profile. This may be null during testing.
Profile* profile_; Profile* profile_;
...@@ -396,6 +412,10 @@ class SessionService : public sessions::CommandStorageManagerDelegate, ...@@ -396,6 +412,10 @@ class SessionService : public sessions::CommandStorageManagerDelegate,
// tab's index hasn't changed. // tab's index hasn't changed.
std::map<SessionID, int> last_selected_tab_in_window_; std::map<SessionID, int> last_selected_tab_in_window_;
content::NotificationRegistrar registrar_;
bool did_log_exit_ = false;
base::WeakPtrFactory<SessionService> weak_factory_{this}; base::WeakPtrFactory<SessionService> weak_factory_{this};
DISALLOW_COPY_AND_ASSIGN(SessionService); DISALLOW_COPY_AND_ASSIGN(SessionService);
......
...@@ -68,8 +68,8 @@ bool DeserializeEvent(const base::Value& serialized_event, ...@@ -68,8 +68,8 @@ bool DeserializeEvent(const base::Value& serialized_event,
auto type = serialized_event.FindIntKey(kEventTypeKey); auto type = serialized_event.FindIntKey(kEventTypeKey);
if (!type) if (!type)
return false; return false;
if (*type < SessionServiceEventLogType::kMinValue || if (*type < static_cast<int>(SessionServiceEventLogType::kMinValue) ||
*type > SessionServiceEventLogType::kMaxValue) { *type > static_cast<int>(SessionServiceEventLogType::kMaxValue)) {
return false; return false;
} }
event.type = static_cast<SessionServiceEventLogType>(*type); event.type = static_cast<SessionServiceEventLogType>(*type);
...@@ -198,6 +198,18 @@ void LogSessionServiceWriteErrorEvent(Profile* profile) { ...@@ -198,6 +198,18 @@ void LogSessionServiceWriteErrorEvent(Profile* profile) {
LogSessionServiceEvent(profile, event); LogSessionServiceEvent(profile, event);
} }
void RemoveLastSessionServiceEventOfType(Profile* profile,
SessionServiceEventLogType type) {
std::list<SessionServiceEvent> events = GetSessionServiceEvents(profile);
for (auto iter = events.rbegin(); iter != events.rend(); ++iter) {
if (iter->type == type) {
events.erase(std::next(iter).base());
SaveEventsToPrefs(profile, events);
return;
}
}
}
void RegisterSessionServiceLogProfilePrefs( void RegisterSessionServiceLogProfilePrefs(
user_prefs::PrefRegistrySyncable* registry) { user_prefs::PrefRegistrySyncable* registry) {
registry->RegisterListPref(kEventPrefKey); registry->RegisterListPref(kEventPrefKey);
...@@ -211,7 +223,7 @@ void LogSessionServiceEvent(Profile* profile, ...@@ -211,7 +223,7 @@ void LogSessionServiceEvent(Profile* profile,
events.back().type == SessionServiceEventLogType::kWriteError) { events.back().type == SessionServiceEventLogType::kWriteError) {
events.back().data.write_error.error_count += 1; events.back().data.write_error.error_count += 1;
} else { } else {
events.push_back(std::move(event)); events.push_back(event);
if (events.size() >= kMaxEventCount) if (events.size() >= kMaxEventCount)
events.erase(events.begin()); events.erase(events.begin());
} }
......
...@@ -23,7 +23,7 @@ class PrefRegistrySyncable; ...@@ -23,7 +23,7 @@ class PrefRegistrySyncable;
// tracked in prefs, and only a limited amount of data is kept around. // tracked in prefs, and only a limited amount of data is kept around.
// WARNING: these values are persisted to disk, do not change. // WARNING: these values are persisted to disk, do not change.
enum SessionServiceEventLogType { enum class SessionServiceEventLogType {
// The profile was started. // The profile was started.
kStart = 0, kStart = 0,
...@@ -100,6 +100,8 @@ void LogSessionServiceRestoreEvent(Profile* profile, ...@@ -100,6 +100,8 @@ void LogSessionServiceRestoreEvent(Profile* profile,
int tab_count, int tab_count,
bool encountered_error_reading); bool encountered_error_reading);
void LogSessionServiceWriteErrorEvent(Profile* profile); void LogSessionServiceWriteErrorEvent(Profile* profile);
void RemoveLastSessionServiceEventOfType(Profile* profile,
SessionServiceEventLogType type);
void RegisterSessionServiceLogProfilePrefs( void RegisterSessionServiceLogProfilePrefs(
user_prefs::PrefRegistrySyncable* registry); user_prefs::PrefRegistrySyncable* registry);
......
// Copyright 2021 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include <stddef.h>
#include <list>
#include <memory>
#include "base/command_line.h"
#include "base/optional.h"
#include "build/chromeos_buildflags.h"
#include "chrome/browser/prefs/session_startup_pref.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/resource_coordinator/session_restore_policy.h"
#include "chrome/browser/sessions/session_restore_test_helper.h"
#include "chrome/browser/sessions/session_restore_test_utils.h"
#include "chrome/browser/sessions/session_service_log.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_commands.h"
#include "chrome/browser/ui/browser_list.h"
#include "chrome/browser/ui/startup/startup_types.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "components/keep_alive_registry/keep_alive_types.h"
#include "components/keep_alive_registry/scoped_keep_alive.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h"
#include "ui/base/page_transition_types.h"
class SessionServiceLogTest : public InProcessBrowserTest {
public:
SessionServiceLogTest() = default;
~SessionServiceLogTest() override = default;
protected:
#if BUILDFLAG(IS_CHROMEOS_ASH)
void SetUpCommandLine(base::CommandLine* command_line) override {
// TODO(nkostylev): Investigate if we can remove this switch.
command_line->AppendSwitch(switches::kCreateBrowserOnStartupForTests);
}
#endif
void PreRunTestOnMainThread() override {
InProcessBrowserTest::PreRunTestOnMainThread();
ASSERT_TRUE(browser());
profile_ = browser()->profile();
ASSERT_TRUE(profile_);
}
void SetUpOnMainThread() override {
SessionStartupPref pref(SessionStartupPref::LAST);
SessionStartupPref::SetStartupPref(browser()->profile(), pref);
}
base::Optional<SessionServiceEvent> FindMostRecentEventOfType(
SessionServiceEventLogType type) const {
auto events = GetSessionServiceEvents(profile_);
for (auto i = events.rbegin(); i != events.rend(); ++i) {
if (i->type == type)
return *i;
}
return base::nullopt;
}
std::list<SessionServiceEvent>::reverse_iterator
AdvanceToMostRecentEventOfType(
const std::list<SessionServiceEvent>& events,
std::list<SessionServiceEvent>::reverse_iterator start,
SessionServiceEventLogType type) {
while (start != events.rend() && start->type != type)
++start;
return start;
}
int GetEventCountOfType(SessionServiceEventLogType type) const {
int count = 0;
for (const auto& event : GetSessionServiceEvents(profile_)) {
if (event.type == type)
++count;
}
return count;
}
protected:
// Cached as browser() may be destroyed.
Profile* profile_ = nullptr;
std::unique_ptr<ScopedKeepAlive> keep_alive_;
};
IN_PROC_BROWSER_TEST_F(SessionServiceLogTest, ExitEvent) {
// A start event should always be logged.
EXPECT_TRUE(FindMostRecentEventOfType(SessionServiceEventLogType::kStart));
ui_test_utils::NavigateToURLWithDisposition(
browser(), GURL(url::kAboutBlankURL),
WindowOpenDisposition::NEW_FOREGROUND_TAB,
ui_test_utils::BROWSER_TEST_WAIT_FOR_LOAD_STOP);
ui_test_utils::NavigateToURLWithDisposition(
browser(), GURL(url::kAboutBlankURL),
WindowOpenDisposition::NEW_BACKGROUND_TAB,
ui_test_utils::BROWSER_TEST_WAIT_FOR_LOAD_STOP);
EXPECT_FALSE(FindMostRecentEventOfType(SessionServiceEventLogType::kExit));
const int tab_count = browser()->tab_strip_model()->count();
CloseBrowserSynchronously(browser());
auto exit_event =
FindMostRecentEventOfType(SessionServiceEventLogType::kExit);
ASSERT_TRUE(exit_event);
EXPECT_EQ(1, exit_event->data.exit.window_count);
EXPECT_EQ(tab_count, exit_event->data.exit.tab_count);
}
IN_PROC_BROWSER_TEST_F(SessionServiceLogTest, PRE_RestoreEvent) {
// A start event should always be logged.
EXPECT_TRUE(FindMostRecentEventOfType(SessionServiceEventLogType::kStart));
ui_test_utils::NavigateToURLWithDisposition(
browser(), GURL(url::kAboutBlankURL),
WindowOpenDisposition::NEW_FOREGROUND_TAB,
ui_test_utils::BROWSER_TEST_WAIT_FOR_LOAD_STOP);
ui_test_utils::NavigateToURLWithDisposition(
browser(), GURL(url::kAboutBlankURL),
WindowOpenDisposition::NEW_BACKGROUND_TAB,
ui_test_utils::BROWSER_TEST_WAIT_FOR_LOAD_STOP);
Browser* new_browser = chrome::OpenEmptyWindow(profile_);
ASSERT_EQ(2u, BrowserList::GetInstance()->size());
ui_test_utils::NavigateToURLWithDisposition(
new_browser, GURL(url::kAboutBlankURL),
WindowOpenDisposition::CURRENT_TAB,
ui_test_utils::BROWSER_TEST_WAIT_FOR_LOAD_STOP);
}
IN_PROC_BROWSER_TEST_F(SessionServiceLogTest, RestoreEvent) {
auto events = GetSessionServiceEvents(profile_);
// There should be a restore.
auto iter = events.rbegin();
iter = AdvanceToMostRecentEventOfType(events, iter,
SessionServiceEventLogType::kRestore);
ASSERT_TRUE(iter != events.rend());
EXPECT_EQ(2, iter->data.restore.window_count);
EXPECT_EQ(4, iter->data.restore.tab_count);
// Preceded by a start (for this test).
iter = AdvanceToMostRecentEventOfType(events, iter,
SessionServiceEventLogType::kStart);
ASSERT_TRUE(iter != events.rend());
// Exit.
iter = AdvanceToMostRecentEventOfType(events, iter,
SessionServiceEventLogType::kExit);
ASSERT_TRUE(iter != events.rend());
EXPECT_EQ(2, iter->data.exit.window_count);
EXPECT_EQ(4, iter->data.exit.tab_count);
// And another start from PRE_ test.
iter = AdvanceToMostRecentEventOfType(events, iter,
SessionServiceEventLogType::kStart);
ASSERT_TRUE(iter != events.rend());
}
...@@ -91,3 +91,23 @@ TEST_F(SessionServiceLogTest, WriteErrorEventsCoalesce) { ...@@ -91,3 +91,23 @@ TEST_F(SessionServiceLogTest, WriteErrorEventsCoalesce) {
EXPECT_LE(start_time, restored_event.time); EXPECT_LE(start_time, restored_event.time);
EXPECT_EQ(2, restored_event.data.write_error.error_count); EXPECT_EQ(2, restored_event.data.write_error.error_count);
} }
TEST_F(SessionServiceLogTest, RemoveLastSessionServiceEventOfType) {
LogSessionServiceExitEvent(&testing_profile_, 1, 2);
LogSessionServiceWriteErrorEvent(&testing_profile_);
LogSessionServiceExitEvent(&testing_profile_, 2, 3);
LogSessionServiceWriteErrorEvent(&testing_profile_);
RemoveLastSessionServiceEventOfType(&testing_profile_,
SessionServiceEventLogType::kExit);
auto events = GetSessionServiceEvents(&testing_profile_);
ASSERT_EQ(3u, events.size());
auto exit_event = *events.begin();
EXPECT_EQ(SessionServiceEventLogType::kExit, exit_event.type);
EXPECT_EQ(1, exit_event.data.exit.window_count);
EXPECT_EQ(2, exit_event.data.exit.tab_count);
auto iter = events.begin();
++iter;
EXPECT_EQ(SessionServiceEventLogType::kWriteError, iter->type);
++iter;
EXPECT_EQ(SessionServiceEventLogType::kWriteError, iter->type);
}
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "base/files/file_util.h" #include "base/files/file_util.h"
#include "base/files/scoped_temp_dir.h" #include "base/files/scoped_temp_dir.h"
#include "base/location.h" #include "base/location.h"
#include "base/optional.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/single_thread_task_runner.h" #include "base/single_thread_task_runner.h"
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
...@@ -28,6 +29,7 @@ ...@@ -28,6 +29,7 @@
#include "chrome/browser/profiles/profile_attributes_entry.h" #include "chrome/browser/profiles/profile_attributes_entry.h"
#include "chrome/browser/profiles/profile_attributes_storage.h" #include "chrome/browser/profiles/profile_attributes_storage.h"
#include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/sessions/session_service_log.h"
#include "chrome/browser/sessions/session_service_test_helper.h" #include "chrome/browser/sessions/session_service_test_helper.h"
#include "chrome/common/chrome_paths.h" #include "chrome/common/chrome_paths.h"
#include "chrome/common/url_constants.h" #include "chrome/common/url_constants.h"
...@@ -82,6 +84,16 @@ class SessionServiceTest : public BrowserWithTestWindowTest { ...@@ -82,6 +84,16 @@ class SessionServiceTest : public BrowserWithTestWindowTest {
BrowserWithTestWindowTest::TearDown(); BrowserWithTestWindowTest::TearDown();
} }
base::Optional<SessionServiceEvent> FindMostRecentEventOfType(
SessionServiceEventLogType type) {
auto events = GetSessionServiceEvents(browser()->profile());
for (auto i = events.rbegin(); i != events.rend(); ++i) {
if (i->type == type)
return *i;
}
return base::nullopt;
}
void DestroySessionService() { void DestroySessionService() {
// Destroy the SessionService first as it may post tasks. // Destroy the SessionService first as it may post tasks.
session_service_.reset(); session_service_.reset();
...@@ -1340,3 +1352,20 @@ TEST_F(SessionServiceTest, GetSessionsAndDestroy) { ...@@ -1340,3 +1352,20 @@ TEST_F(SessionServiceTest, GetSessionsAndDestroy) {
flag.Set(); flag.Set();
run_loop.Run(); run_loop.Run();
} }
TEST_F(SessionServiceTest, LogExit) {
EXPECT_FALSE(FindMostRecentEventOfType(SessionServiceEventLogType::kExit));
helper_.SetHasOpenTrackableBrowsers(false);
service()->WindowClosing(window_id);
auto exit_event =
FindMostRecentEventOfType(SessionServiceEventLogType::kExit);
ASSERT_TRUE(exit_event);
EXPECT_EQ(1, exit_event->data.exit.window_count);
EXPECT_EQ(browser()->tab_strip_model()->count(),
exit_event->data.exit.tab_count);
// Create another window, which should remove the exit.
SessionID window2_id = SessionID::NewUnique();
service()->SetWindowType(window2_id, Browser::TYPE_NORMAL);
EXPECT_FALSE(FindMostRecentEventOfType(SessionServiceEventLogType::kExit));
}
...@@ -32,6 +32,7 @@ ...@@ -32,6 +32,7 @@
#include "chrome/browser/profiles/profile_io_data.h" #include "chrome/browser/profiles/profile_io_data.h"
#include "chrome/browser/sessions/session_service.h" #include "chrome/browser/sessions/session_service.h"
#include "chrome/browser/sessions/session_service_factory.h" #include "chrome/browser/sessions/session_service_factory.h"
#include "chrome/browser/sessions/session_service_log.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_commands.h" #include "chrome/browser/ui/browser_commands.h"
#include "chrome/browser/ui/browser_finder.h" #include "chrome/browser/ui/browser_finder.h"
...@@ -300,6 +301,7 @@ void StartupBrowserCreatorImpl::DetermineURLsAndLaunch( ...@@ -300,6 +301,7 @@ void StartupBrowserCreatorImpl::DetermineURLsAndLaunch(
const bool is_incognito_or_guest = profile_->IsOffTheRecord(); const bool is_incognito_or_guest = profile_->IsOffTheRecord();
bool is_post_crash_launch = HasPendingUncleanExit(profile_); bool is_post_crash_launch = HasPendingUncleanExit(profile_);
bool has_incompatible_applications = false; bool has_incompatible_applications = false;
LogSessionServiceStartEvent(profile_, is_post_crash_launch);
#if defined(OS_WIN) #if defined(OS_WIN)
#if BUILDFLAG(GOOGLE_CHROME_BRANDING) #if BUILDFLAG(GOOGLE_CHROME_BRANDING)
if (is_post_crash_launch) { if (is_post_crash_launch) {
......
...@@ -1333,6 +1333,7 @@ if (!is_android) { ...@@ -1333,6 +1333,7 @@ if (!is_android) {
"../browser/sessions/closed_tab_cache_browsertest.cc", "../browser/sessions/closed_tab_cache_browsertest.cc",
"../browser/sessions/session_restore_browsertest.cc", "../browser/sessions/session_restore_browsertest.cc",
"../browser/sessions/session_restore_observer_browsertest.cc", "../browser/sessions/session_restore_observer_browsertest.cc",
"../browser/sessions/session_service_log_browsertest.cc",
"../browser/sessions/tab_restore_browsertest.cc", "../browser/sessions/tab_restore_browsertest.cc",
"../browser/sessions/tab_restore_service_browsertest.cc", "../browser/sessions/tab_restore_service_browsertest.cc",
"../browser/signin/e2e_tests/live_sign_in_test.cc", "../browser/signin/e2e_tests/live_sign_in_test.cc",
......
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