Skip to content

Commit

Permalink
[M106 Merge][side search] Add feature engagement for autotriggering
Browse files Browse the repository at this point in the history
This CL integrates the autotriggering side search side panel feature
with the feature egagement backend. This is only done for the unified
side panel as experimentation will happen after this feature has
fully rolled out.

ShouldAutomaticallyTriggerAfterNavigation() has been added to the
unified side panel controller to determine whether or not we should
attempt to automatically trigger the side panel based on the
following conditions:

  1. The associated tab is the currently active browser tab

  2. The side panel is not currently open to side search

  3. The navigation is renderer initiated (i.e. not an omnibox
     or bookmark navigation)

  4. We have navigated back to the same SRP within the tab 2
     number of times (default, configurable by finch param)

  5. The currently committed navigation is the first navigation
     away from the SRP

The automatic triggering behavior is also configurable via the
feature engagement backend. This can be used to control the
window in which auto-triggering is reattempted and how frequently
this occurs.

int returned_to_previous_srp_count() has also replaced
bool returned_to_previous_srp() in the tab helper to allow for
count based (rather than boolean based) tracking for the number
of times users return to the same SRP.

To test enable IPH_SideSearchAutoTriggering as the
in-product-help-demo-mode-choice in chrome://flags and use the
following on the command line (ignore the newlines):

--enable-features=
SideSearch,
SideSearchDSESupport,
UnifiedSidePanel,
SideSearchAutoTriggering:SideSearchAutoTriggeringReturnCount/1

Follow up will add additional metrics and tests.

(cherry picked from commit 35268ca)

Bug: 1354016
Change-Id: Id6890e0dbe045a55137d07f311d498da62839454
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3835658
Commit-Queue: Thomas Lukaszewicz <tluk@chromium.org>
Reviewed-by: Yuheng Huang <yuhengh@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1037325}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3850822
Cr-Commit-Position: refs/branch-heads/5249@{#49}
Cr-Branched-From: 4f7bea5-refs/heads/main@{#1036826}
  • Loading branch information
Thomas Lukaszewicz authored and Chromium LUCI CQ committed Aug 23, 2022
1 parent 1055f36 commit c951047
Show file tree
Hide file tree
Showing 16 changed files with 295 additions and 29 deletions.
2 changes: 2 additions & 0 deletions chrome/browser/ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -4921,6 +4921,8 @@ static_library("ui") {
"views/side_search/side_search_browser_controller.h",
"views/side_search/side_search_icon_view.cc",
"views/side_search/side_search_icon_view.h",
"views/side_search/side_search_views_utils.cc",
"views/side_search/side_search_views_utils.h",
"views/side_search/unified_side_search_controller.cc",
"views/side_search/unified_side_search_controller.h",
"views/side_search/unified_side_search_helper.cc",
Expand Down
13 changes: 10 additions & 3 deletions chrome/browser/ui/side_search/side_search_tab_contents_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,15 @@ void SideSearchTabContentsHelper::DidFinishNavigation(
auto* config = GetConfig();

if (config->ShouldNavigateInSidePanel(url)) {
returned_to_previous_srp_ = navigation_handle->GetPageTransition() &
ui::PAGE_TRANSITION_FORWARD_BACK;
// Keep track of how many times a user returned to the `last_search_url_`
// via back-navigation. Reset the count if navigating to a new SRP or
// forward through history to an existing SRP.
if (navigation_handle->GetNavigationEntryOffset() < 0 &&
url == last_search_url_) {
++returned_to_previous_srp_count_;
} else {
returned_to_previous_srp_count_ = 0;
}

// Capture the URL here in case the side contents is closed before the
// navigation completes.
Expand Down Expand Up @@ -260,7 +267,7 @@ void SideSearchTabContentsHelper::ClearHelperState() {
toggled_open_ = false;
simple_loader_.reset();
last_search_url_.reset();
returned_to_previous_srp_ = false;
returned_to_previous_srp_count_ = 0;
toggled_open_ = false;

// Notify the side panel after resetting the above state but before clearing
Expand Down
14 changes: 8 additions & 6 deletions chrome/browser/ui/side_search/side_search_tab_contents_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,9 @@ class SideSearchTabContentsHelper
return side_panel_initiated_redirect_info_;
}

bool returned_to_previous_srp() const { return returned_to_previous_srp_; }
int returned_to_previous_srp_count() const {
return returned_to_previous_srp_count_;
}

bool toggled_open() const { return toggled_open_; }
void set_toggled_open(bool toggled_open) { toggled_open_ = toggled_open; }
Expand Down Expand Up @@ -164,11 +166,11 @@ class SideSearchTabContentsHelper
// The last Google search URL encountered by this tab contents.
absl::optional<GURL> last_search_url_;

// Whether the last search url was the result of the user navigating back
// to the previously visisted search url. Used to detect cases where the
// side search panel would be of use to the user and thus could benefit
// of IPH promo.
bool returned_to_previous_srp_ = false;
// Counts the number of times the user has returned to the `last_search_url_`
// via back navigation. This is used to detect cases where the side search
// panel would be of use to the user and is used to show an IPH promo and
// automatically trigger the side panel.
int returned_to_previous_srp_count_ = 0;

// A flag to track whether the current tab has its side panel toggled open.
// Only used with the kSideSearchStatePerTab flag.
Expand Down
11 changes: 11 additions & 0 deletions chrome/browser/ui/ui_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,17 @@ const base::FeatureParam<int> kSideSearchPageActionLabelAnimationMaxCount{
const base::Feature kClobberAllSideSearchSidePanels{
"ClobberAllSideSearchSidePanels", base::FEATURE_ENABLED_BY_DEFAULT};

// Feature that controls whether or not feature engagement configurations can be
// used to control automatic triggering for side search.
const base::Feature kSideSearchAutoTriggering{"SideSearchAutoTriggering",
base::FEATURE_ENABLED_BY_DEFAULT};

// Feature param that determines how many times a user has to return to a given
// SRP before we automatically trigger the side search side panel for that SRP
// on a subsequent navigation.
const base::FeatureParam<int> kSideSearchAutoTriggeringReturnCount{
&kSideSearchAutoTriggering, "SideSearchAutoTriggeringReturnCount", 2};

// Adds improved support for handling multiple contextual and global RHS browser
// side panels. Designed specifically to handle the interim state before the v2
// side panel project launches.
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/ui/ui_features.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ extern const base::Feature kSideSearchFeedback;
extern const base::Feature kSideSearchDSESupport;
extern const base::Feature kClobberAllSideSearchSidePanels;

extern const base::Feature kSideSearchAutoTriggering;
extern const base::FeatureParam<int> kSideSearchAutoTriggeringReturnCount;

extern const base::Feature kSideSearchPageActionLabelAnimation;

enum class kSideSearchLabelAnimationTypeOption {
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/ui/views/side_panel/side_panel_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ class SidePanelUtil {
kComboboxSelected = 4,
kTabChanged = 5,
kSidePanelEntryDeregistered = 6,
kMaxValue = kSidePanelEntryDeregistered,
kIPHSideSearchAutoTrigger = 7,
kMaxValue = kIPHSideSearchAutoTrigger,
};

static void PopulateGlobalEntries(Browser* browser,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ void SideSearchBrowserController::UpdateSidePanel() {
// Once the anchor element is visible, maybe show promo for the toolbar
// button.
if (toolbar_button_ && can_show_side_panel_for_page &&
tab_contents_helper->returned_to_previous_srp()) {
tab_contents_helper->returned_to_previous_srp_count() > 0) {
browser_view_->MaybeShowFeaturePromo(
feature_engagement::kIPHSideSearchFeature);
}
Expand Down
17 changes: 15 additions & 2 deletions chrome/browser/ui/views/side_search/side_search_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,21 @@ void SideSearchBrowserTest::AppendTab(Browser* browser, const GURL& url) {
}

void SideSearchBrowserTest::NavigateActiveTab(Browser* browser,
const GURL& url) {
ASSERT_TRUE(ui_test_utils::NavigateToURL(browser, url));
const GURL& url,
bool is_renderer_initiated) {
if (is_renderer_initiated) {
// Navigate from a link and wait for loading to finish.
content::TestNavigationObserver observer(
browser->tab_strip_model()->GetActiveWebContents());
NavigateParams params(browser, url,
ui::PageTransition::PAGE_TRANSITION_LINK);
params.initiator_origin = url::Origin();
params.is_renderer_initiated = true;
ui_test_utils::NavigateToURL(&params);
observer.WaitForNavigationFinished();
} else {
ASSERT_TRUE(ui_test_utils::NavigateToURL(browser, url));
}
}

content::WebContents* SideSearchBrowserTest::GetActiveSidePanelWebContents(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ class SideSearchBrowserTest : public InProcessBrowserTest {
void AppendTab(Browser* browser, const GURL& url);

// Navigates the browser's currently active tab to `url`.
void NavigateActiveTab(Browser* browser, const GURL& url);
void NavigateActiveTab(Browser* browser,
const GURL& url,
bool is_renderer_initiated = false);

// Gets the browser's currently active tab contents.
content::WebContents* GetActiveSidePanelWebContents(Browser* browser);
Expand Down
18 changes: 3 additions & 15 deletions chrome/browser/ui/views/side_search/side_search_icon_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/side_search/default_search_icon_source.h"
#include "chrome/browser/ui/views/side_search/side_search_browser_controller.h"
#include "chrome/browser/ui/views/side_search/side_search_views_utils.h"
#include "chrome/browser/ui/views/side_search/unified_side_search_controller.h"
#include "chrome/grit/generated_resources.h"
#include "components/feature_engagement/public/event_constants.h"
Expand All @@ -26,19 +27,6 @@
#include "ui/gfx/paint_vector_icon.h"
#include "ui/views/view_class_properties.h"

namespace {

bool IsSideSearchToggleOpen(BrowserView* browser_view) {
if (base::FeatureList::IsEnabled(features::kUnifiedSidePanel)) {
auto* coordinator = browser_view->side_panel_coordinator();
return coordinator->IsSidePanelShowing() &&
coordinator->GetCurrentEntryId() == SidePanelEntry::Id::kSideSearch;
}
return browser_view->side_search_controller()->GetSidePanelToggledOpen();
}

} // namespace

SideSearchIconView::SideSearchIconView(
CommandUpdater* command_updater,
IconLabelBubbleView::Delegate* icon_label_bubble_delegate,
Expand Down Expand Up @@ -107,15 +95,15 @@ void SideSearchIconView::UpdateImpl() {
const bool was_visible = GetVisible();
const bool should_show =
tab_contents_helper->CanShowSidePanelForCommittedNavigation() &&
!IsSideSearchToggleOpen(browser_view);
!side_search::IsSideSearchToggleOpen(browser_view);
SetVisible(should_show);

if (should_show && !was_visible) {
if (ShouldShowPageActionLabel()) {
SetPageActionLabelShown();
should_extend_label_shown_duration_ = true;
AnimateIn(absl::nullopt);
} else if (tab_contents_helper->returned_to_previous_srp()) {
} else if (tab_contents_helper->returned_to_previous_srp_count() > 0) {
// If we are not animating-in the label text make a request to show the
// IPH if we detect the user may be engaging in a pogo-sticking journey.
browser_view->MaybeShowFeaturePromo(
Expand Down
24 changes: 24 additions & 0 deletions chrome/browser/ui/views/side_search/side_search_views_utils.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2022 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 "chrome/browser/ui/views/side_search/side_search_views_utils.h"

#include "chrome/browser/ui/ui_features.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/side_panel/side_panel_coordinator.h"
#include "chrome/browser/ui/views/side_panel/side_panel_entry.h"
#include "chrome/browser/ui/views/side_search/side_search_browser_controller.h"

namespace side_search {

bool IsSideSearchToggleOpen(BrowserView* browser_view) {
if (base::FeatureList::IsEnabled(features::kUnifiedSidePanel)) {
auto* coordinator = browser_view->side_panel_coordinator();
return coordinator->IsSidePanelShowing() &&
coordinator->GetCurrentEntryId() == SidePanelEntry::Id::kSideSearch;
}
return browser_view->side_search_controller()->GetSidePanelToggledOpen();
}

} // namespace side_search
18 changes: 18 additions & 0 deletions chrome/browser/ui/views/side_search/side_search_views_utils.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright 2022 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.

#ifndef CHROME_BROWSER_UI_VIEWS_SIDE_SEARCH_SIDE_SEARCH_VIEWS_UTILS_H_
#define CHROME_BROWSER_UI_VIEWS_SIDE_SEARCH_SIDE_SEARCH_VIEWS_UTILS_H_

class BrowserView;

namespace side_search {

// Returns true if the side panel is open to the side search feature. This is
// used by both the independent and unified side panel implementations.
bool IsSideSearchToggleOpen(BrowserView* browser_view);

} // namespace side_search

#endif // CHROME_BROWSER_UI_VIEWS_SIDE_SEARCH_SIDE_SEARCH_VIEWS_UTILS_H_
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,19 @@

#include "base/bind.h"
#include "chrome/app/vector_icons/vector_icons.h"
#include "chrome/browser/feature_engagement/tracker_factory.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser_finder.h"
#include "chrome/browser/ui/color/chrome_color_id.h"
#include "chrome/browser/ui/side_search/side_search_utils.h"
#include "chrome/browser/ui/ui_features.h"
#include "chrome/browser/ui/views/chrome_layout_provider.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/side_panel/side_panel.h"
#include "chrome/browser/ui/views/side_search/side_search_views_utils.h"
#include "components/feature_engagement/public/event_constants.h"
#include "components/feature_engagement/public/feature_constants.h"
#include "components/feature_engagement/public/tracker.h"
#include "components/url_formatter/elide_url.h"
#include "components/vector_icons/vector_icons.h"
#include "content/public/browser/navigation_handle.h"
Expand Down Expand Up @@ -105,6 +111,28 @@ void UnifiedSideSearchController::DidFinishNavigation(
}

UpdateSidePanel();

if (ShouldAutomaticallyTriggerAfterNavigation(navigation_handle)) {
auto* tracker =
feature_engagement::TrackerFactory::GetForBrowserContext(GetProfile());
auto* browser_view = GetBrowserView();

if (!browser_view || !tracker ||
!tracker->ShouldTriggerHelpUI(
feature_engagement::kIPHSideSearchAutoTriggeringFeature)) {
return;
}

browser_view->side_panel_coordinator()->Show(
SidePanelEntry::Id::kSideSearch,
SidePanelUtil::SidePanelOpenTrigger::kIPHSideSearchAutoTrigger);

// Note that `Dismiss()` in this case does not dismiss the UI. It's telling
// the FE backend that the promo is done so that other promos can run. The
// side panel showing should not block other promos from displaying.
tracker->Dismissed(feature_engagement::kIPHSideSearchAutoTriggeringFeature);
tracker->NotifyEvent(feature_engagement::events::kSideSearchAutoTriggered);
}
}

void UnifiedSideSearchController::OnEntryShown(SidePanelEntry* entry) {
Expand Down Expand Up @@ -194,6 +222,10 @@ BrowserView* UnifiedSideSearchController::GetBrowserView() const {
return browser ? BrowserView::GetBrowserViewForBrowser(browser) : nullptr;
}

Profile* UnifiedSideSearchController::GetProfile() {
return Profile::FromBrowserContext(web_contents()->GetBrowserContext());
}

void UnifiedSideSearchController::UpdateSidePanel() {
auto* tab_contents_helper =
SideSearchTabContentsHelper::FromWebContents(web_contents());
Expand Down Expand Up @@ -241,4 +273,40 @@ void UnifiedSideSearchController::UpdateSidePanelRegistry(bool is_available) {
}
}

bool UnifiedSideSearchController::ShouldAutomaticallyTriggerAfterNavigation(
content::NavigationHandle* navigation_handle) {
// Only trigger the panel automatically if the current tab is the browser's
// active tab (it may not necessarily be the active tab if navigation commit
// happens after the user switches tabs).
auto* browser_view = GetBrowserView();
if (!browser_view || browser_view->GetActiveWebContents() != web_contents())
return false;

// If the side search side panel is already open we do not need to
// automatically retrigger the panel.
if (side_search::IsSideSearchToggleOpen(browser_view))
return false;

auto* tab_contents_helper =
SideSearchTabContentsHelper::FromWebContents(web_contents());
if (!tab_contents_helper)
return false;

const GURL& previously_committed_url =
navigation_handle->GetPreviousPrimaryMainFrameURL();
const bool is_renderer_initiated = navigation_handle->IsRendererInitiated();
const int auto_triggering_return_count =
features::kSideSearchAutoTriggeringReturnCount.Get();

// Trigger the side panel only if we've returned to the same SRP n times and
// this is the first navigation after navigating away from the Google SRP. We
// also check to ensure the navigation is renderer initiated to avoid showing
// the side panel if the user navigates the tab via the omnibox / bookmarks
// etc.
return is_renderer_initiated &&
tab_contents_helper->returned_to_previous_srp_count() ==
auto_triggering_return_count &&
previously_committed_url == tab_contents_helper->last_search_url();
}

WEB_CONTENTS_USER_DATA_KEY_IMPL(UnifiedSideSearchController);
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "ui/views/view.h"

class BrowserView;
class Profile;

// Responsible for managing the WebContents hosted in the browser's side panel
// for Side Search. Created immediately following the creation of the tab's
Expand Down Expand Up @@ -64,6 +65,7 @@ class UnifiedSideSearchController

private:
BrowserView* GetBrowserView() const;
Profile* GetProfile();

// Create a WebView to host the side search WebContents.
std::unique_ptr<views::View> GetSideSearchView();
Expand All @@ -79,6 +81,11 @@ class UnifiedSideSearchController

void UpdateSidePanelRegistry(bool is_available);

// True if the side panel should be automatically triggered after a navigation
// defined by `navigation_handle`.
bool ShouldAutomaticallyTriggerAfterNavigation(
content::NavigationHandle* navigation_handle);

// A handler to handle unhandled keyboard messages coming back from the
// renderer process.
views::UnhandledKeyboardEventHandler unhandled_keyboard_event_handler_;
Expand Down

0 comments on commit c951047

Please sign in to comment.