Commit 0f83ff4c authored by Wei Li's avatar Wei Li Committed by Commit Bot

[HaTS] Refactor bubble showing procedure

Simplify the bubble showing procedure through browser view by 1)
move profile checking logic into Hats service; 2) move anchor view
handling logic into Hats bubble class.

BUG=979530

Change-Id: I6962f3904d575671f592ca0926edd214fe748c2b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1742511Reviewed-by: default avatarScott Violet <sky@chromium.org>
Reviewed-by: default avatarAllen Bauer <kylixrd@chromium.org>
Commit-Queue: Wei Li <weili@chromium.org>
Cr-Commit-Position: refs/heads/master@{#685400}
parent 984239a8
......@@ -459,9 +459,10 @@ class BrowserWindow : public ui::BaseWindow {
signin_metrics::AccessPoint access_point,
bool is_source_keyboard) = 0;
// Shows User Happiness Tracking Survey's invitation bubble anchored to the
// app menu button that will lead to a survey identified by |site_id|.
virtual void ShowHatsBubbleFromAppMenuButton(const std::string& site_id) = 0;
// Shows User Happiness Tracking Survey's invitation bubble when possible
// (such as having the proper anchor view).
// |site_id| is the site identification of the survey the bubble leads to.
virtual void ShowHatsBubble(const std::string& site_id) = 0;
// Executes |command| registered by |extension|.
virtual void ExecuteExtensionCommand(const extensions::Extension* extension,
......
......@@ -13,6 +13,7 @@
#include "base/version.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profiles_state.h"
#include "chrome/browser/ui/browser_finder.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/common/chrome_features.h"
......@@ -84,8 +85,10 @@ void HatsService::RegisterProfilePrefs(PrefRegistrySimple* registry) {
void HatsService::LaunchSatisfactionSurvey() {
if (ShouldShowSurvey(kHatsSurveyTriggerSatisfaction)) {
Browser* browser = chrome::FindLastActive();
if (browser && browser->is_type_tabbed()) {
browser->window()->ShowHatsBubbleFromAppMenuButton(en_site_id_);
// Never show HaTS bubble in Incognito mode.
if (browser && browser->is_type_tabbed() &&
profiles::IsRegularOrGuestSession(browser)) {
browser->window()->ShowHatsBubble(en_site_id_);
DictionaryPrefUpdate update(profile_->GetPrefs(),
prefs::kHatsSurveyMetadata);
......
......@@ -3013,19 +3013,8 @@ void BrowserView::ShowAvatarBubbleFromAvatarButton(
ProfileMetrics::LogProfileOpenMethod(ProfileMetrics::ICON_AVATAR_BUBBLE);
}
void BrowserView::ShowHatsBubbleFromAppMenuButton(const std::string& site_id) {
// Never show any HaTS bubble in Incognito.
if (!IsRegularOrGuestSession())
return;
AppMenuButton* app_menu_button =
toolbar_button_provider()->GetAppMenuButton();
// Do not show HaTS bubble if there is no avatar menu button to anchor to.
if (!app_menu_button)
return;
HatsBubbleView::Show(browser(), app_menu_button, site_id);
void BrowserView::ShowHatsBubble(const std::string& site_id) {
HatsBubbleView::Show(browser(), site_id);
}
void BrowserView::ExecuteExtensionCommand(
......
......@@ -425,7 +425,7 @@ class BrowserView : public BrowserWindow,
const signin::ManageAccountsParams& manage_accounts_params,
signin_metrics::AccessPoint access_point,
bool is_source_keyboard) override;
void ShowHatsBubbleFromAppMenuButton(const std::string& site_id) override;
void ShowHatsBubble(const std::string& site_id) override;
void ExecuteExtensionCommand(const extensions::Extension* extension,
const extensions::Command& command) override;
ExclusiveAccessContext* GetExclusiveAccessContext() override;
......
......@@ -18,7 +18,7 @@ class HatsBubbleTest : public DialogBrowserTest {
void ShowUi(const std::string& name) override {
ASSERT_TRUE(browser()->is_type_tabbed());
BrowserView::GetBrowserViewForBrowser(InProcessBrowserTest::browser())
->ShowHatsBubbleFromAppMenuButton(std::string());
->ShowHatsBubble(std::string());
}
private:
......
......@@ -10,6 +10,8 @@
#include "chrome/browser/ui/browser_dialogs.h"
#include "chrome/browser/ui/views/chrome_layout_provider.h"
#include "chrome/browser/ui/views/frame/app_menu_button.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/frame/toolbar_button_provider.h"
#include "chrome/browser/ui/views/hats/hats_web_dialog.h"
#include "chrome/grit/chromium_strings.h"
#include "chrome/grit/generated_resources.h"
......@@ -36,8 +38,14 @@ views::BubbleDialogDelegateView* HatsBubbleView::GetHatsBubble() {
// static
void HatsBubbleView::Show(Browser* browser,
AppMenuButton* anchor_button,
const std::string& site_id) {
AppMenuButton* anchor_button = BrowserView::GetBrowserViewForBrowser(browser)
->toolbar_button_provider()
->GetAppMenuButton();
// Do not show HaTS bubble if there is no avatar menu button to anchor to.
if (!anchor_button)
return;
base::RecordAction(base::UserMetricsAction("HatsBubble.Show"));
DCHECK(anchor_button->GetWidget());
......
......@@ -22,11 +22,9 @@ class HatsBubbleView : public views::BubbleDialogDelegateView {
public:
// Returns a pointer to the Hats Bubble being shown. For testing only.
static views::BubbleDialogDelegateView* GetHatsBubble();
// Creates and shows the bubble anchored to the |anchor_button| and leads to
// a survey identified by |site_id|.
static void Show(Browser* browser,
AppMenuButton* anchor_button,
const std::string& site_id);
// Creates and shows the bubble that leads to a survey identified by
// |site_id|.
static void Show(Browser* browser, const std::string& site_id);
protected:
// views::WidgetDelegate:
......
......@@ -175,7 +175,7 @@ class TestBrowserWindow : public BrowserWindow {
#if defined(OS_CHROMEOS) || defined(OS_MACOSX) || defined(OS_WIN) || \
defined(OS_LINUX)
void ShowHatsBubbleFromAppMenuButton(const std::string& site_id) override {}
void ShowHatsBubble(const std::string& site_id) override {}
#endif
void ExecuteExtensionCommand(const extensions::Extension* extension,
......
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