diff options
Diffstat (limited to 'chromium/components/bookmarks/browser')
10 files changed, 79 insertions, 70 deletions
diff --git a/chromium/components/bookmarks/browser/bookmark_load_details.cc b/chromium/components/bookmarks/browser/bookmark_load_details.cc index f1b9c528deb..d1bed0cbd91 100644 --- a/chromium/components/bookmarks/browser/bookmark_load_details.cc +++ b/chromium/components/bookmarks/browser/bookmark_load_details.cc @@ -13,7 +13,8 @@ namespace bookmarks { BookmarkLoadDetails::BookmarkLoadDetails(BookmarkClient* client) : load_managed_node_callback_(client->GetLoadManagedNodeCallback()), - index_(std::make_unique<TitledUrlIndex>()) { + index_(std::make_unique<TitledUrlIndex>()), + load_start_(base::TimeTicks::Now()) { // WARNING: do NOT add |client| as a member. Much of this code runs on another // thread, and |client_| is not thread safe, and/or may be destroyed before // this. diff --git a/chromium/components/bookmarks/browser/bookmark_load_details.h b/chromium/components/bookmarks/browser/bookmark_load_details.h index 0c1ff5e5db9..830826eaa7c 100644 --- a/chromium/components/bookmarks/browser/bookmark_load_details.h +++ b/chromium/components/bookmarks/browser/bookmark_load_details.h @@ -13,6 +13,10 @@ #include "components/bookmarks/browser/bookmark_client.h" #include "components/bookmarks/browser/bookmark_node.h" +namespace base { +class TimeTicks; +} + namespace bookmarks { class BookmarkPermanentNode; @@ -91,6 +95,8 @@ class BookmarkLoadDetails { void CreateUrlIndex(); UrlIndex* url_index() { return url_index_.get(); } + base::TimeTicks load_start() { return load_start_; } + private: std::unique_ptr<BookmarkNode> root_node_; raw_ptr<BookmarkNode> root_node_ptr_; @@ -108,6 +114,7 @@ class BookmarkLoadDetails { scoped_refptr<UrlIndex> url_index_; // A string blob represetning the sync metadata stored in the json file. std::string sync_metadata_str_; + base::TimeTicks load_start_; }; } // namespace bookmarks diff --git a/chromium/components/bookmarks/browser/bookmark_model.cc b/chromium/components/bookmarks/browser/bookmark_model.cc index 8dc727bab13..66b8d1b80f6 100644 --- a/chromium/components/bookmarks/browser/bookmark_model.cc +++ b/chromium/components/bookmarks/browser/bookmark_model.cc @@ -34,6 +34,7 @@ #include "components/bookmarks/browser/url_and_title.h" #include "components/bookmarks/browser/url_index.h" #include "components/bookmarks/common/bookmark_constants.h" +#include "components/bookmarks/common/bookmark_metrics.h" #include "components/favicon_base/favicon_types.h" #include "components/strings/grit/components_strings.h" #include "ui/base/l10n/l10n_util.h" @@ -823,6 +824,10 @@ void BookmarkModel::DoneLoading(std::unique_ptr<BookmarkLoadDetails> details) { base::Unretained(store_.get())) : base::DoNothing()); + const base::TimeDelta load_duration = + base::TimeTicks::Now() - details->load_start(); + metrics::RecordTimeToLoadAtStartup(load_duration); + // Notify our direct observers. for (BookmarkModelObserver& observer : observers_) observer.BookmarkModelLoaded(this, details->ids_reassigned()); diff --git a/chromium/components/bookmarks/browser/bookmark_storage.cc b/chromium/components/bookmarks/browser/bookmark_storage.cc index 0ff4aeeb2fa..915b9678460 100644 --- a/chromium/components/bookmarks/browser/bookmark_storage.cc +++ b/chromium/components/bookmarks/browser/bookmark_storage.cc @@ -23,6 +23,7 @@ #include "components/bookmarks/browser/bookmark_model.h" #include "components/bookmarks/browser/bookmark_node.h" #include "components/bookmarks/common/bookmark_constants.h" +#include "components/bookmarks/common/bookmark_metrics.h" namespace bookmarks { @@ -50,7 +51,8 @@ BookmarkStorage::BookmarkStorage(BookmarkModel* model, writer_(profile_path.Append(kBookmarksFileName), backend_task_runner_, kSaveDelay, - "BookmarkStorage") {} + "BookmarkStorage"), + last_scheduled_save_(base::TimeTicks::Now()) {} BookmarkStorage::~BookmarkStorage() { if (writer_.HasPendingWrite()) @@ -67,6 +69,11 @@ void BookmarkStorage::ScheduleSave() { } writer_.ScheduleWriteWithBackgroundDataSerializer(this); + + const base::TimeDelta schedule_delta = + base::TimeTicks::Now() - last_scheduled_save_; + metrics::RecordTimeSinceLastScheduledSave(schedule_delta); + last_scheduled_save_ = base::TimeTicks::Now(); } void BookmarkStorage::BookmarkModelDeleted() { diff --git a/chromium/components/bookmarks/browser/bookmark_storage.h b/chromium/components/bookmarks/browser/bookmark_storage.h index 6bd25c8eae1..e85825859cd 100644 --- a/chromium/components/bookmarks/browser/bookmark_storage.h +++ b/chromium/components/bookmarks/browser/bookmark_storage.h @@ -89,6 +89,9 @@ class BookmarkStorage // The state of the backup file creation which is created lazily just before // the first scheduled save. bool backup_triggered_ = false; + + // Used to track the frequency of saves starting from the first save. + base::TimeTicks last_scheduled_save_; }; } // namespace bookmarks diff --git a/chromium/components/bookmarks/browser/bookmark_storage_unittest.cc b/chromium/components/bookmarks/browser/bookmark_storage_unittest.cc index e03c2e9016b..c20ec55aa08 100644 --- a/chromium/components/bookmarks/browser/bookmark_storage_unittest.cc +++ b/chromium/components/bookmarks/browser/bookmark_storage_unittest.cc @@ -9,6 +9,7 @@ #include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/files/scoped_temp_dir.h" +#include "base/test/metrics/histogram_tester.h" #include "base/test/task_environment.h" #include "base/time/time.h" #include "components/bookmarks/browser/bookmark_model.h" @@ -31,6 +32,7 @@ std::unique_ptr<BookmarkModel> CreateModelWithOneBookmark() { } // namespace TEST(BookmarkStorageTest, ShouldSaveFileToDiskAfterDelay) { + base::HistogramTester histogram_tester; std::unique_ptr<BookmarkModel> model = CreateModelWithOneBookmark(); base::ScopedTempDir temp_dir; ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); @@ -56,6 +58,8 @@ TEST(BookmarkStorageTest, ShouldSaveFileToDiskAfterDelay) { task_environment.FastForwardBy(base::Milliseconds(20)); EXPECT_FALSE(storage.HasScheduledSaveForTesting()); EXPECT_TRUE(base::PathExists(temp_dir.GetPath().Append(kBookmarksFileName))); + histogram_tester.ExpectTotalCount( + "Bookmarks.Storage.TimeSinceLastScheduledSave", 1); } TEST(BookmarkStorageTest, ShouldSaveFileDespiteShutdownWhileScheduled) { @@ -115,4 +119,36 @@ TEST(BookmarkStorageTest, ShouldGenerateBackupFileUponFirstSave) { EXPECT_FALSE(base::PathExists(backup_file_path)); } +TEST(BookmarkStorageTest, RecordTimeSinceLastScheduledSave) { + base::HistogramTester histogram_tester; + std::unique_ptr<BookmarkModel> model = CreateModelWithOneBookmark(); + base::ScopedTempDir temp_dir; + ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); + + base::test::TaskEnvironment task_environment{ + base::test::TaskEnvironment::TimeSource::MOCK_TIME}; + BookmarkStorage storage(model.get(), temp_dir.GetPath()); + + ASSERT_FALSE(storage.HasScheduledSaveForTesting()); + ASSERT_FALSE(base::PathExists(temp_dir.GetPath().Append(kBookmarksFileName))); + + storage.ScheduleSave(); + + base::TimeDelta delay_ms = base::Milliseconds(10); + // Advance clock until immediately before saving takes place. + task_environment.FastForwardBy(delay_ms); + storage.ScheduleSave(); + EXPECT_TRUE(storage.HasScheduledSaveForTesting()); + EXPECT_FALSE(base::PathExists(temp_dir.GetPath().Append(kBookmarksFileName))); + + // Advance clock past the saving moment. + task_environment.FastForwardBy(BookmarkStorage::kSaveDelay + delay_ms); + EXPECT_FALSE(storage.HasScheduledSaveForTesting()); + EXPECT_TRUE(base::PathExists(temp_dir.GetPath().Append(kBookmarksFileName))); + histogram_tester.ExpectTotalCount( + "Bookmarks.Storage.TimeSinceLastScheduledSave", 2); + histogram_tester.ExpectTimeBucketCount( + "Bookmarks.Storage.TimeSinceLastScheduledSave", delay_ms, 1); +} + } // namespace bookmarks diff --git a/chromium/components/bookmarks/browser/bookmark_utils.cc b/chromium/components/bookmarks/browser/bookmark_utils.cc index 651dc39ca8e..3e9c55300d1 100644 --- a/chromium/components/bookmarks/browser/bookmark_utils.cc +++ b/chromium/components/bookmarks/browser/bookmark_utils.cc @@ -429,7 +429,7 @@ bool DoesBookmarkContainWords(const std::u16string& title, DoesBookmarkTextContainWords(base::UTF8ToUTF16(url.spec()), words) || DoesBookmarkTextContainWords( url_formatter::FormatUrl(url, url_formatter::kFormatUrlOmitNothing, - net::UnescapeRule::NORMAL, nullptr, + base::UnescapeRule::NORMAL, nullptr, nullptr, nullptr), words); } @@ -522,8 +522,8 @@ std::u16string CleanUpUrlForMatching( return base::i18n::ToLower(url_formatter::FormatUrlWithAdjustments( GURL(TruncateUrl(gurl.spec())), url_formatter::kFormatUrlOmitUsernamePassword, - net::UnescapeRule::SPACES | net::UnescapeRule::PATH_SEPARATORS | - net::UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS, + base::UnescapeRule::SPACES | base::UnescapeRule::PATH_SEPARATORS | + base::UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS, nullptr, nullptr, adjustments ? adjustments : &tmp_adjustments)); } diff --git a/chromium/components/bookmarks/browser/model_loader.cc b/chromium/components/bookmarks/browser/model_loader.cc index ade292fcb20..eb8a7492f4e 100644 --- a/chromium/components/bookmarks/browser/model_loader.cc +++ b/chromium/components/bookmarks/browser/model_loader.cc @@ -10,6 +10,7 @@ #include "base/json/json_file_value_serializer.h" #include "base/json/json_reader.h" #include "base/metrics/histogram_functions.h" +#include "base/numerics/clamped_math.h" #include "base/synchronization/waitable_event.h" #include "base/task/task_traits.h" #include "base/task/thread_pool.h" @@ -17,6 +18,8 @@ #include "components/bookmarks/browser/bookmark_load_details.h" #include "components/bookmarks/browser/titled_url_index.h" #include "components/bookmarks/browser/url_index.h" +#include "components/bookmarks/common/bookmark_metrics.h" +#include "components/bookmarks/common/url_load_stats.h" namespace bookmarks { @@ -79,52 +82,13 @@ void LoadBookmarks(const base::FilePath& path, details->CreateUrlIndex(); - UrlIndex::Stats stats = details->url_index()->ComputeStats(); + UrlLoadStats stats = details->url_index()->ComputeStats(); + metrics::RecordUrlLoadStatsOnProfileLoad(stats); - DCHECK_LE(stats.duplicate_url_bookmark_count, stats.total_url_bookmark_count); - DCHECK_LE(stats.duplicate_url_and_title_bookmark_count, - stats.duplicate_url_bookmark_count); - DCHECK_LE(stats.duplicate_url_and_title_and_parent_bookmark_count, - stats.duplicate_url_and_title_bookmark_count); - - base::UmaHistogramCounts100000( - "Bookmarks.Count.OnProfileLoad", - base::saturated_cast<int>(stats.total_url_bookmark_count)); - - if (stats.duplicate_url_bookmark_count != 0) { - base::UmaHistogramCounts100000( - "Bookmarks.Count.OnProfileLoad.DuplicateUrl2", - base::saturated_cast<int>(stats.duplicate_url_bookmark_count)); - } - - if (stats.duplicate_url_and_title_bookmark_count != 0) { - base::UmaHistogramCounts100000( - "Bookmarks.Count.OnProfileLoad.DuplicateUrlAndTitle", - base::saturated_cast<int>( - stats.duplicate_url_and_title_bookmark_count)); + int64_t file_size_bytes; + if (bookmark_file_exists && base::GetFileSize(path, &file_size_bytes)) { + metrics::RecordFileSizeAtStartup(file_size_bytes); } - - if (stats.duplicate_url_and_title_and_parent_bookmark_count != 0) { - base::UmaHistogramCounts100000( - "Bookmarks.Count.OnProfileLoad.DuplicateUrlAndTitleAndParent", - base::saturated_cast<int>( - stats.duplicate_url_and_title_and_parent_bookmark_count)); - } - - // Log derived metrics for convenience. - base::UmaHistogramCounts100000( - "Bookmarks.Count.OnProfileLoad.UniqueUrl", - base::saturated_cast<int>(stats.total_url_bookmark_count - - stats.duplicate_url_bookmark_count)); - base::UmaHistogramCounts100000( - "Bookmarks.Count.OnProfileLoad.UniqueUrlAndTitle", - base::saturated_cast<int>(stats.total_url_bookmark_count - - stats.duplicate_url_and_title_bookmark_count)); - base::UmaHistogramCounts100000( - "Bookmarks.Count.OnProfileLoad.UniqueUrlAndTitleAndParent", - base::saturated_cast<int>( - stats.total_url_bookmark_count - - stats.duplicate_url_and_title_and_parent_bookmark_count)); } } // namespace diff --git a/chromium/components/bookmarks/browser/url_index.cc b/chromium/components/bookmarks/browser/url_index.cc index 10b11c7dcfa..033f5a5e317 100644 --- a/chromium/components/bookmarks/browser/url_index.cc +++ b/chromium/components/bookmarks/browser/url_index.cc @@ -9,6 +9,7 @@ #include "base/containers/adapters.h" #include "base/guid.h" #include "components/bookmarks/browser/url_and_title.h" +#include "components/bookmarks/common/url_load_stats.h" namespace bookmarks { @@ -23,7 +24,7 @@ namespace { // calling site. void AddStatsForBookmarksWithSameUrl( std::vector<const BookmarkNode*>* bookmarks_with_same_url, - UrlIndex::Stats* stats) { + UrlLoadStats* stats) { if (bookmarks_with_same_url->size() <= 1) return; @@ -126,9 +127,9 @@ bool UrlIndex::HasBookmarks() const { return !nodes_ordered_by_url_set_.empty(); } -UrlIndex::Stats UrlIndex::ComputeStats() const { +UrlLoadStats UrlIndex::ComputeStats() const { base::AutoLock url_lock(url_lock_); - UrlIndex::Stats stats; + UrlLoadStats stats; stats.total_url_bookmark_count = nodes_ordered_by_url_set_.size(); if (stats.total_url_bookmark_count <= 1) diff --git a/chromium/components/bookmarks/browser/url_index.h b/chromium/components/bookmarks/browser/url_index.h index 5f1c0dca6ef..e31877053c2 100644 --- a/chromium/components/bookmarks/browser/url_index.h +++ b/chromium/components/bookmarks/browser/url_index.h @@ -19,6 +19,7 @@ namespace bookmarks { class BookmarkNode; +struct UrlLoadStats; struct UrlAndTitle; // UrlIndex maintains the bookmark nodes of type url. The nodes are ordered by @@ -67,24 +68,8 @@ class UrlIndex : public HistoryBookmarkModel { // Returns true if there is at least one bookmark. bool HasBookmarks() const; - // Returns some stats about number of URL bookmarks stored, for UMA purposes. - struct Stats { - // Number of bookmark in the index excluding folders. - size_t total_url_bookmark_count = 0; - // Number of bookmarks (excluding folders) with a URL that is used by at - // least one other bookmark, excluding one bookmark per unique URL (i.e. all - // except one are considered duplicates). - size_t duplicate_url_bookmark_count = 0; - // Number of bookmarks (excluding folders) with the pair <URL, title> that - // is used by at least one other bookmark, excluding one bookmark per unique - // URL (i.e. all except one are considered duplicates). - size_t duplicate_url_and_title_bookmark_count = 0; - // Number of bookmarks (excluding folders) with the triple <URL, title, - // parent> that is used by at least one other bookmark, excluding one - // bookmark per unique URL (i.e. all except one are considered duplicates). - size_t duplicate_url_and_title_and_parent_bookmark_count = 0; - }; - Stats ComputeStats() const; + // Compute stats from the load. + UrlLoadStats ComputeStats() const; // HistoryBookmarkModel: bool IsBookmarked(const GURL& url) override; |