-
Notifications
You must be signed in to change notification settings - Fork 6.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[bookmarks] Add BookmarkStatsTabHelper for bookmark metrics - Part 1
This adds a new struct BookmarkLaunchAction that captures the location and time that a user opens their bookmarks. Currently this struct is created only for bookmark launches originating from the toolbar, follow up CLs will add this to other launch locations. The BookmarkStatsTabHelper has been introduced to help report CUJ metrics following an initial launch action by the user. The launch data is cleared after each navigation to ensure only the initial navigation is considered when reporting metrics. The first few UMA histograms for the helper will be added in Part 2. Bug: 1449016 Change-Id: I55f3c06f13bda9b232447a5c02c311b066b8dac0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4566539 Reviewed-by: Dana Fried <dfried@chromium.org> Commit-Queue: Thomas Lukaszewicz <tluk@chromium.org> Cr-Commit-Position: refs/heads/main@{#1152094}
- Loading branch information
Thomas Lukaszewicz
authored and
Chromium LUCI CQ
committed
Jun 1, 2023
1 parent
83ce983
commit 84bb564
Showing
10 changed files
with
269 additions
and
22 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| // Copyright 2023 The Chromium Authors | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| #include "chrome/browser/ui/bookmarks/bookmark_stats_tab_helper.h" | ||
|
|
||
| #include "content/public/browser/navigation_handle.h" | ||
|
|
||
| BookmarkStatsTabHelper::~BookmarkStatsTabHelper() = default; | ||
|
|
||
| bool BookmarkStatsTabHelper::SetLaunchAction( | ||
| const BookmarkLaunchAction& launch_action, | ||
| WindowOpenDisposition tab_disposition) { | ||
| if (web_contents()->GetController().GetPendingEntry()) { | ||
| launch_action_ = launch_action; | ||
| tab_disposition_ = tab_disposition; | ||
| // The launch data should remain valid through to the end of the current | ||
| // navigation and the begging of the next navigation. | ||
| should_reset_launch_data_ = false; | ||
| return true; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| void BookmarkStatsTabHelper::DidStartNavigation( | ||
| content::NavigationHandle* navigation_handle) { | ||
| // Reset the launch action data at the beginning of the next navigation. | ||
| if (navigation_handle->IsInPrimaryMainFrame() && should_reset_launch_data_) { | ||
| tab_disposition_.reset(); | ||
| launch_action_.reset(); | ||
| should_reset_launch_data_ = false; | ||
| } | ||
| } | ||
|
|
||
| void BookmarkStatsTabHelper::DidFinishNavigation( | ||
| content::NavigationHandle* navigation_handle) { | ||
| if (navigation_handle->IsInPrimaryMainFrame()) { | ||
| should_reset_launch_data_ = true; | ||
| } | ||
| } | ||
|
|
||
| BookmarkStatsTabHelper::BookmarkStatsTabHelper( | ||
| content::WebContents* web_contents) | ||
| : content::WebContentsObserver(web_contents), | ||
| content::WebContentsUserData<BookmarkStatsTabHelper>(*web_contents) {} | ||
|
|
||
| WEB_CONTENTS_USER_DATA_KEY_IMPL(BookmarkStatsTabHelper); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| // Copyright 2023 The Chromium Authors | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| #ifndef CHROME_BROWSER_UI_BOOKMARKS_BOOKMARK_STATS_TAB_HELPER_H_ | ||
| #define CHROME_BROWSER_UI_BOOKMARKS_BOOKMARK_STATS_TAB_HELPER_H_ | ||
|
|
||
| #include "chrome/browser/ui/bookmarks/bookmark_stats.h" | ||
| #include "content/public/browser/web_contents_observer.h" | ||
| #include "content/public/browser/web_contents_user_data.h" | ||
| #include "ui/base/window_open_disposition.h" | ||
|
|
||
| // A tab helper responsible for reporting metrics on various bookmarks user | ||
| // journeys. | ||
| class BookmarkStatsTabHelper | ||
| : public content::WebContentsObserver, | ||
| public content::WebContentsUserData<BookmarkStatsTabHelper> { | ||
| public: | ||
| BookmarkStatsTabHelper(); | ||
| BookmarkStatsTabHelper(const BookmarkStatsTabHelper&) = delete; | ||
| BookmarkStatsTabHelper& operator=(const BookmarkStatsTabHelper&) = delete; | ||
| ~BookmarkStatsTabHelper() override; | ||
|
|
||
| // The launch action should be set immediately after a given tab has been | ||
| // navigated following a bookmark launch action. The launch action will only | ||
| // be set if there is a pending navigation for the current tab. | ||
| // `tab_disposition` is the disposition requested for by this tab for the | ||
| // navigation. | ||
| bool SetLaunchAction(const BookmarkLaunchAction& launch_action, | ||
| WindowOpenDisposition tab_disposition); | ||
|
|
||
| // content::WebContentsObserver: | ||
| void DidStartNavigation( | ||
| content::NavigationHandle* navigation_handle) override; | ||
| void DidFinishNavigation( | ||
| content::NavigationHandle* navigation_handle) override; | ||
|
|
||
| const absl::optional<BookmarkLaunchAction>& launch_action_for_testing() | ||
| const { | ||
| return launch_action_; | ||
| } | ||
| const absl::optional<WindowOpenDisposition>& tab_disposition_for_testing() | ||
| const { | ||
| return tab_disposition_; | ||
| } | ||
|
|
||
| private: | ||
| friend class content::WebContentsUserData<BookmarkStatsTabHelper>; | ||
|
|
||
| explicit BookmarkStatsTabHelper(content::WebContents* web_contents); | ||
|
|
||
| // The launch action and requested tab disposition for the current navigation. | ||
| // These will be reset after the following navigation begins. | ||
| absl::optional<BookmarkLaunchAction> launch_action_; | ||
| absl::optional<WindowOpenDisposition> tab_disposition_; | ||
|
|
||
| // Tracks whether launch data should be reset as a new primary frame | ||
| // navigation begins. | ||
| bool should_reset_launch_data_ = false; | ||
|
|
||
| WEB_CONTENTS_USER_DATA_KEY_DECL(); | ||
| }; | ||
|
|
||
| #endif // CHROME_BROWSER_UI_BOOKMARKS_BOOKMARK_STATS_TAB_HELPER_H_ |
96 changes: 96 additions & 0 deletions
96
chrome/browser/ui/bookmarks/bookmark_stats_tab_helper_unittest.cc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| // Copyright 2023 The Chromium Authors | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| #include "chrome/browser/ui/bookmarks/bookmark_stats_tab_helper.h" | ||
|
|
||
| #include <memory> | ||
|
|
||
| #include "chrome/test/base/testing_profile.h" | ||
| #include "content/public/browser/browser_context.h" | ||
| #include "content/public/browser/navigation_entry.h" | ||
| #include "content/public/test/browser_task_environment.h" | ||
| #include "content/public/test/test_renderer_host.h" | ||
| #include "content/public/test/web_contents_tester.h" | ||
| #include "testing/gtest/include/gtest/gtest.h" | ||
| #include "url/gurl.h" | ||
|
|
||
| namespace { | ||
|
|
||
| constexpr char kTestUrl1[] = "https://www.test-url-1.com/"; | ||
| constexpr char kTestUrl2[] = "https://www.test-url-2.com/"; | ||
|
|
||
| } // namespace | ||
|
|
||
| namespace test { | ||
|
|
||
| class BookmarkStatsTabHelperTest : public ::testing::Test { | ||
| public: | ||
| // ::testing::Test: | ||
| void SetUp() override { | ||
| web_contents_ = | ||
| content::WebContentsTester::CreateTestWebContents(&profile_, nullptr); | ||
| Test::SetUp(); | ||
| } | ||
|
|
||
| protected: | ||
| void LoadUrl(const char* url) { | ||
| web_contents()->GetController().LoadURL(GURL(url), content::Referrer(), | ||
| ui::PAGE_TRANSITION_AUTO_TOPLEVEL, | ||
| std::string()); | ||
| } | ||
|
|
||
| void CommitPendingNavigation() { | ||
| content::WebContentsTester::For(web_contents())->CommitPendingNavigation(); | ||
| } | ||
|
|
||
| BookmarkStatsTabHelper* GetHelper() { | ||
| BookmarkStatsTabHelper::CreateForWebContents(web_contents_.get()); | ||
| return BookmarkStatsTabHelper::FromWebContents(web_contents_.get()); | ||
| } | ||
|
|
||
| content::WebContents* web_contents() { return web_contents_.get(); } | ||
|
|
||
| private: | ||
| content::BrowserTaskEnvironment task_environment_; | ||
| content::RenderViewHostTestEnabler rvh_enabler_; | ||
| TestingProfile profile_; | ||
| std::unique_ptr<content::WebContents> web_contents_; | ||
| }; | ||
|
|
||
| TEST_F(BookmarkStatsTabHelperTest, LaunchActionAddedWithPendingEntry) { | ||
| // The launch action should start unset. | ||
| BookmarkStatsTabHelper* helper = GetHelper(); | ||
| EXPECT_FALSE(helper->launch_action_for_testing().has_value()); | ||
| EXPECT_FALSE(helper->tab_disposition_for_testing().has_value()); | ||
|
|
||
| // Attempting to set the launch action without a pending navigation having | ||
| // been set should result in a no-op. | ||
| helper->SetLaunchAction( | ||
| {BookmarkLaunchLocation::kAttachedBar, base::TimeTicks::Now()}, | ||
| WindowOpenDisposition::CURRENT_TAB); | ||
| EXPECT_FALSE(helper->launch_action_for_testing().has_value()); | ||
| EXPECT_FALSE(helper->tab_disposition_for_testing().has_value()); | ||
|
|
||
| // Begin a navigation and attempt to set the launch action. This should now | ||
| // be reflected in the helper. | ||
| LoadUrl(kTestUrl1); | ||
| helper->SetLaunchAction( | ||
| {BookmarkLaunchLocation::kAttachedBar, base::TimeTicks::Now()}, | ||
| WindowOpenDisposition::CURRENT_TAB); | ||
| EXPECT_TRUE(helper->launch_action_for_testing().has_value()); | ||
| EXPECT_TRUE(helper->tab_disposition_for_testing().has_value()); | ||
|
|
||
| // After the navigation completes the launch action data should remain. | ||
| CommitPendingNavigation(); | ||
| EXPECT_TRUE(helper->launch_action_for_testing().has_value()); | ||
| EXPECT_TRUE(helper->tab_disposition_for_testing().has_value()); | ||
|
|
||
| // A following navigation should clear the launch action data. | ||
| LoadUrl(kTestUrl2); | ||
| CommitPendingNavigation(); | ||
| EXPECT_FALSE(helper->launch_action_for_testing().has_value()); | ||
| EXPECT_FALSE(helper->tab_disposition_for_testing().has_value()); | ||
| } | ||
|
|
||
| } // namespace test |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.