diff --git a/ash/public/cpp/ash_pref_names.cc b/ash/public/cpp/ash_pref_names.cc index 097dc22c5f852b..5b5544ea237a8d 100644 --- a/ash/public/cpp/ash_pref_names.cc +++ b/ash/public/cpp/ash_pref_names.cc @@ -169,6 +169,10 @@ const char kContextualTooltips[] = "settings.contextual_tooltip.shown_info"; // name will appear in this list as an empty string. The desk names are stored // as UTF8 strings. const char kDesksNamesList[] = "ash.desks.desks_names_list"; +// This list stores the metrics of virtual desks. Like |kDesksNamesList|, this +// list stores entries in the same order of the desks in the overview desks bar. +// Values are stored as dictionaries. +const char kDesksMetricsList[] = "ash.desks.desks_metrics_list"; // An integer index of a user's active desk. const char kDesksActiveDesk[] = "ash.desks.active_desk"; diff --git a/ash/public/cpp/ash_pref_names.h b/ash/public/cpp/ash_pref_names.h index 28aaf876f37e87..37057159391bf0 100644 --- a/ash/public/cpp/ash_pref_names.h +++ b/ash/public/cpp/ash_pref_names.h @@ -66,6 +66,7 @@ ASH_PUBLIC_EXPORT extern const char kAltTabPerDesk[]; ASH_PUBLIC_EXPORT extern const char kContextualTooltips[]; ASH_PUBLIC_EXPORT extern const char kDesksNamesList[]; +ASH_PUBLIC_EXPORT extern const char kDesksMetricsList[]; ASH_PUBLIC_EXPORT extern const char kDesksActiveDesk[]; ASH_PUBLIC_EXPORT extern const char kDockedMagnifierEnabled[]; diff --git a/ash/wm/desks/desk.cc b/ash/wm/desks/desk.cc index 09b4a51674d5ed..78a13b3cdb26f2 100644 --- a/ash/wm/desks/desk.cc +++ b/ash/wm/desks/desk.cc @@ -25,6 +25,7 @@ #include "ash/wm/workspace_controller.h" #include "base/containers/adapters.h" #include "base/containers/contains.h" +#include "base/metrics/histogram_functions.h" #include "base/stl_util.h" #include "chromeos/ui/base/window_properties.h" #include "ui/aura/client/aura_constants.h" @@ -36,6 +37,9 @@ namespace ash { namespace { +// Prefix for the desks lifetime histogram. +constexpr char kDeskLifetimeHistogramNamePrefix[] = "Ash.Desks.DeskLifetime_"; + void UpdateBackdropController(aura::Window* desk_container) { auto* workspace_controller = GetWorkspaceController(desk_container); // Work might have already been cleared when the display is removed. See @@ -180,7 +184,8 @@ class DeskContainerObserver : public aura::WindowObserver { // Desk: Desk::Desk(int associated_container_id) - : container_id_(associated_container_id) { + : container_id_(associated_container_id), + creation_time_(base::Time::Now()) { // For the very first default desk added during initialization, there won't be // any root windows yet. That's OK, OnRootWindowAdded() will be called // explicitly by the RootWindowController when they're initialized. @@ -489,6 +494,15 @@ void Desk::SetDeskBeingRemoved() { is_desk_being_removed_ = true; } +void Desk::RecordLifetimeHistogram() { + // Desk index is 1-indexed in histograms. + const int desk_index = + Shell::Get()->desks_controller()->GetDeskIndex(this) + 1; + base::UmaHistogramCounts1000( + base::StringPrintf("%s%i", kDeskLifetimeHistogramNamePrefix, desk_index), + (base::Time::Now() - creation_time_).InHours()); +} + void Desk::MoveWindowToDeskInternal(aura::Window* window, Desk* target_desk, aura::Window* target_root) { diff --git a/ash/wm/desks/desk.h b/ash/wm/desks/desk.h index b26882908a71b3..dcab2cf431ff9f 100644 --- a/ash/wm/desks/desk.h +++ b/ash/wm/desks/desk.h @@ -14,6 +14,7 @@ #include "base/macros.h" #include "base/observer_list.h" #include "base/strings/string16.h" +#include "base/time/time.h" #include "ui/aura/window_observer.h" namespace ash { @@ -54,6 +55,8 @@ class ASH_EXPORT Desk { const std::vector& windows() const { return windows_; } + const base::Time& creation_time() const { return creation_time_; } + const base::string16& name() const { return name_; } bool is_active() const { return is_active_; } @@ -66,6 +69,12 @@ class ASH_EXPORT Desk { bool is_name_set_by_user() const { return is_name_set_by_user_; } + // Sets the desk's |creation_time_| to |creation_time|. This will overwrite + // |creation_time_| so this should only be called when restoring desks. + void set_creation_time(base::Time creation_time) { + creation_time_ = creation_time; + } + void AddObserver(Observer* observer); void RemoveObserver(Observer* observer); @@ -130,6 +139,10 @@ class ASH_EXPORT Desk { // when desk is already removed from |desks_| in DesksController. void SetDeskBeingRemoved(); + // Records the lifetime of the desk based on its desk index. Should be called + // when this desk is removed by the user. + void RecordLifetimeHistogram(); + private: void MoveWindowToDeskInternal(aura::Window* window, Desk* target_desk, @@ -181,6 +194,9 @@ class ASH_EXPORT Desk { // True if the desk is being removed. bool is_desk_being_removed_ = false; + // The time this desk was created at. Used to record desk lifetimes. + base::Time creation_time_; + DISALLOW_COPY_AND_ASSIGN(Desk); }; diff --git a/ash/wm/desks/desks_controller.cc b/ash/wm/desks/desks_controller.cc index 8f6f22010ec50f..8b48c64c6a5ba1 100644 --- a/ash/wm/desks/desks_controller.cc +++ b/ash/wm/desks/desks_controller.cc @@ -372,6 +372,7 @@ void DesksController::NewDesk(DesksCreationRemovalSource source) { if (!is_first_ever_desk) { desks_restore_util::UpdatePrimaryUserDeskNamesPrefs(); + desks_restore_util::UpdatePrimaryUserDeskMetricsPrefs(); UMA_HISTOGRAM_ENUMERATION(kNewDeskHistogramName, source); ReportDesksCountHistogram(); } @@ -422,8 +423,10 @@ void DesksController::ReorderDesk(int old_index, int new_index) { // Meanwhile, only the primary user needs to update the active desk, which is // independent across profiles but only recoverable for the primary user. - // 1. Update desk name list in the user prefs to maintain the right order. + // 1. Update desk name and metrics lists in the user prefs to maintain the + // right order. desks_restore_util::UpdatePrimaryUserDeskNamesPrefs(); + desks_restore_util::UpdatePrimaryUserDeskMetricsPrefs(); // 2. For multi-profile switching, update all affected active desk index in // |user_to_active_desk_index_|. @@ -673,6 +676,14 @@ void DesksController::RestoreNameOfDeskAtIndex(base::string16 name, desks_[index]->SetName(std::move(name), /*set_by_user=*/true); } +void DesksController::RestoreMetricsOfDeskAtIndex(base::Time creation_time, + size_t index) { + DCHECK_LT(index, desks_.size()); + + const auto& target_desk = desks_[index]; + target_desk->set_creation_time(std::move(creation_time)); +} + void DesksController::OnRootWindowAdded(aura::Window* root_window) { for (auto& desk : desks_) desk->OnRootWindowAdded(root_window); @@ -886,6 +897,9 @@ void DesksController::RemoveDeskInternal(const Desk* desk, // Used by accessibility to indicate the desk that has been removed. const int removed_desk_number = std::distance(desks_.begin(), iter) + 1; + // Record |desk|'s lifetime before it's removed from |desks_|. + const_cast(desk)->RecordLifetimeHistogram(); + // Keep the removed desk alive until the end of this function. std::unique_ptr removed_desk = std::move(*iter); removed_desk->SetDeskBeingRemoved(); @@ -1017,6 +1031,7 @@ void DesksController::RemoveDeskInternal(const Desk* desk, active_desk_->name())); desks_restore_util::UpdatePrimaryUserDeskNamesPrefs(); + desks_restore_util::UpdatePrimaryUserDeskMetricsPrefs(); DCHECK_LE(available_container_ids_.size(), desks_util::GetMaxNumberOfDesks()); } diff --git a/ash/wm/desks/desks_controller.h b/ash/wm/desks/desks_controller.h index 798701974a5e71..57730550920fd7 100644 --- a/ash/wm/desks/desks_controller.h +++ b/ash/wm/desks/desks_controller.h @@ -190,11 +190,14 @@ class ASH_EXPORT DesksController : public DesksHelper, // it was never modified by users. void RevertDeskNameToDefault(Desk* desk); - // Restores the desk at |index| to the given |name|. This is only for user- - // modified desk names, and hence |name| should never be empty since users are - // not allowed to set empty names. + // Restores the desk at |index| to the given |name|. This is only for + // user-modified desk names, and hence |name| should never be empty since + // users are not allowed to set empty names. void RestoreNameOfDeskAtIndex(base::string16 name, size_t index); + // Restores the metrics of the desk at |index|. + void RestoreMetricsOfDeskAtIndex(base::Time creation_time, size_t index); + // Called explicitly by the RootWindowController when a root window has been // added or about to be removed in order to update all the available desks. void OnRootWindowAdded(aura::Window* root_window); diff --git a/ash/wm/desks/desks_restore_util.cc b/ash/wm/desks/desks_restore_util.cc index 2be77f2d756e7b..2f3e0f1f910548 100644 --- a/ash/wm/desks/desks_restore_util.cc +++ b/ash/wm/desks/desks_restore_util.cc @@ -25,6 +25,11 @@ namespace desks_restore_util { namespace { +// Keys for the dictionaries stored in |kDesksMetricsList|. The entry is an int +// which represents the number of minutes for +// base::Time::FromDeltaSinceWindowsEpoch(). +constexpr char kCreationTimeKey[] = "creation_time"; + // While restore is in progress, changes are being made to the desks and their // names. Those changes should not trigger an update to the prefs. bool g_pause_desks_prefs_updates = false; @@ -46,6 +51,7 @@ bool IsValidDeskIndex(int desk_index) { void RegisterProfilePrefs(PrefRegistrySimple* registry) { constexpr int kDefaultActiveDeskIndex = 0; registry->RegisterListPref(prefs::kDesksNamesList); + registry->RegisterListPref(prefs::kDesksMetricsList); if (features::IsBentoEnabled()) { registry->RegisterIntegerPref(prefs::kDesksActiveDesk, kDefaultActiveDeskIndex); @@ -61,11 +67,13 @@ void RestorePrimaryUserDesks() { return; } - const base::ListValue* desks_names_list = + const base::ListValue* desks_names = primary_user_prefs->GetList(prefs::kDesksNamesList); + const base::ListValue* desks_metrics = + primary_user_prefs->GetList(prefs::kDesksMetricsList); // First create the same number of desks. - const size_t restore_size = desks_names_list->GetSize(); + const size_t restore_size = desks_names->GetSize(); // If we don't have any restore data, or the list is corrupt for some reason, // abort. @@ -76,9 +84,12 @@ void RestorePrimaryUserDesks() { while (desks_controller->desks().size() < restore_size) desks_controller->NewDesk(DesksCreationRemovalSource::kDesksRestore); - size_t index = 0; - for (const auto& value : desks_names_list->GetList()) { - const std::string& desk_name = value.GetString(); + const auto& desks_names_list = desks_names->GetList(); + const auto& desks_metrics_list = desks_metrics->GetList(); + const size_t desks_metrics_list_size = desks_metrics->GetSize(); + const auto now = base::Time::Now(); + for (size_t index = 0; index < restore_size; ++index) { + const std::string& desk_name = desks_names_list[index].GetString(); // Empty desks names are just place holders for desks whose names haven't // been modified by the user. Those don't need to be restored; they already // have the correct default names based on their positions in the desks @@ -87,7 +98,20 @@ void RestorePrimaryUserDesks() { desks_controller->RestoreNameOfDeskAtIndex(base::UTF8ToUTF16(desk_name), index); } - ++index; + + if (index >= desks_metrics_list_size) + continue; + + // Only restore metrics if there is existing data. + const auto& desks_metrics_dict = desks_metrics_list[index]; + const auto& creation_time_entry = + desks_metrics_dict.FindIntPath(kCreationTimeKey); + if (creation_time_entry.has_value()) { + const auto creation_time = base::Time::FromDeltaSinceWindowsEpoch( + base::TimeDelta::FromMinutes(*creation_time_entry)); + if (!creation_time.is_null() && creation_time < now) + desks_controller->RestoreMetricsOfDeskAtIndex(creation_time, index); + } } // Restore an active desk for the primary user. @@ -114,20 +138,47 @@ void UpdatePrimaryUserDeskNamesPrefs() { return; } - ListPrefUpdate update(primary_user_prefs, prefs::kDesksNamesList); - base::ListValue* pref_data = update.Get(); - pref_data->Clear(); + ListPrefUpdate name_update(primary_user_prefs, prefs::kDesksNamesList); + base::ListValue* name_pref_data = name_update.Get(); + name_pref_data->Clear(); + const auto& desks = DesksController::Get()->desks(); for (const auto& desk : desks) { // Desks whose names were not changed by the user, are stored as empty // strings. They're just place holders to restore the correct desks count. // RestorePrimaryUserDesks() restores only non-empty desks names. - pref_data->Append(desk->is_name_set_by_user() - ? base::UTF16ToUTF8(desk->name()) - : std::string()); + name_pref_data->Append(desk->is_name_set_by_user() + ? base::UTF16ToUTF8(desk->name()) + : std::string()); + } + + DCHECK_EQ(name_pref_data->GetSize(), desks.size()); +} + +void UpdatePrimaryUserDeskMetricsPrefs() { + if (g_pause_desks_prefs_updates) + return; + + PrefService* primary_user_prefs = GetPrimaryUserPrefService(); + if (!primary_user_prefs) { + // Can be null in tests. + return; + } + + ListPrefUpdate metrics_update(primary_user_prefs, prefs::kDesksMetricsList); + base::ListValue* metrics_pref_data = metrics_update.Get(); + metrics_pref_data->Clear(); + + const auto& desks = DesksController::Get()->desks(); + for (const auto& desk : desks) { + base::DictionaryValue metrics_dict; + metrics_dict.SetInteger( + kCreationTimeKey, + desk->creation_time().ToDeltaSinceWindowsEpoch().InMinutes()); + metrics_pref_data->Append(std::move(metrics_dict)); } - DCHECK_EQ(pref_data->GetSize(), desks.size()); + DCHECK_EQ(metrics_pref_data->GetSize(), desks.size()); } void UpdatePrimaryUserActiveDeskPrefs(int active_desk_index) { diff --git a/ash/wm/desks/desks_restore_util.h b/ash/wm/desks/desks_restore_util.h index 968dc9217ef5c1..22f4fd50355932 100644 --- a/ash/wm/desks/desks_restore_util.h +++ b/ash/wm/desks/desks_restore_util.h @@ -24,6 +24,10 @@ void RestorePrimaryUserDesks(); // desks count or a desk name changes. ASH_EXPORT void UpdatePrimaryUserDeskNamesPrefs(); +// Called to update the desk metrics restore prefs for the primary user whenever +// desks count changes or desks reordered. +ASH_EXPORT void UpdatePrimaryUserDeskMetricsPrefs(); + // Called to update the active desk restore prefs for the primary user whenever // the primary user switches an active desk. void UpdatePrimaryUserActiveDeskPrefs(int active_desk_index); diff --git a/tools/metrics/histograms/histograms_xml/ash/histograms.xml b/tools/metrics/histograms/histograms_xml/ash/histograms.xml index 1ea8e2d00c0e8e..9629128a35d1e5 100644 --- a/tools/metrics/histograms/histograms_xml/ash/histograms.xml +++ b/tools/metrics/histograms/histograms_xml/ash/histograms.xml @@ -634,6 +634,26 @@ reviews. Googlers can read more about this at go/gwsq-gerrit. + + chinsenj@chromium.org + janetmac@chromium.org + + The lifetime of the desk at index {DeskIndex} (1-indexed). Emitted when a + desk is removed. This metric is persisted across sessions. + + + + + + + + + + + + + afakhry@chromium.org