Commit 00a0e731 authored by Vasilii Sukhanov's avatar Vasilii Sukhanov Committed by Commit Bot

Don't update the profile chooser if it's to be closed.

Unnecessary update skews the metrics. Typical flow before the CL is
- The profile chooser shown.
- User clicks something.
- The profile chooser is being hidden.
- Some state changes and updates still existing bubble.

Bug: 901449
Change-Id: Iee9b5c7757aad2f0d9c7149a1e26b33e6e954716
Reviewed-on: https://chromium-review.googlesource.com/c/1320172Reviewed-by: default avatarBret Sepulveda <bsep@chromium.org>
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606412}
parent 880a5ff8
......@@ -382,12 +382,7 @@ ProfileChooserView::ProfileChooserView(views::Button* anchor_button,
chrome::RecordDialogCreation(chrome::DialogIdentifier::PROFILE_CHOOSER);
}
ProfileChooserView::~ProfileChooserView() {
ProfileOAuth2TokenService* oauth2_token_service =
ProfileOAuth2TokenServiceFactory::GetForProfile(browser_->profile());
if (oauth2_token_service)
oauth2_token_service->RemoveObserver(this);
}
ProfileChooserView::~ProfileChooserView() = default;
void ProfileChooserView::ResetView() {
open_other_profile_indexes_map_.clear();
......@@ -579,6 +574,16 @@ void ProfileChooserView::WindowClosing() {
profile_bubble_ = NULL;
}
void ProfileChooserView::OnWidgetClosing(views::Widget* widget) {
// Unsubscribe from everything early so that the updates do not reach the
// bubble and change its state.
avatar_menu_.reset();
ProfileOAuth2TokenService* oauth2_token_service =
ProfileOAuth2TokenServiceFactory::GetForProfile(browser_->profile());
if (oauth2_token_service)
oauth2_token_service->RemoveObserver(this);
}
bool ProfileChooserView::AcceleratorPressed(
const ui::Accelerator& accelerator) {
if (accelerator.key_code() != ui::VKEY_DOWN &&
......
......@@ -88,6 +88,7 @@ class ProfileChooserView : public content::WebContentsDelegate,
void Init() override;
void OnNativeThemeChanged(const ui::NativeTheme* native_theme) override;
void WindowClosing() override;
void OnWidgetClosing(views::Widget* widget) override;
bool AcceleratorPressed(const ui::Accelerator& accelerator) override;
views::View* GetInitiallyFocusedView() override;
int GetDialogButtons() const override;
......
......@@ -13,6 +13,7 @@
#include "base/run_loop.h"
#include "base/strings/utf_string_conversions.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/metrics/user_action_tester.h"
#include "base/threading/thread_restrictions.h"
#include "build/build_config.h"
#include "chrome/browser/browser_process.h"
......@@ -221,7 +222,7 @@ class ProfileChooserViewExtensionsTest
return ProfileChooserView::profile_bubble_;
}
views::View* signin_current_profile_button() {
views::LabelButton* signin_current_profile_button() {
return ProfileChooserView::profile_bubble_->signin_current_profile_button_;
}
......@@ -248,6 +249,17 @@ IN_PROC_BROWSER_TEST_F(ProfileChooserViewExtensionsTest,
EXPECT_TRUE(signin_current_profile_button()->HasFocus());
}
IN_PROC_BROWSER_TEST_F(ProfileChooserViewExtensionsTest, ClickSigninButton) {
ASSERT_NO_FATAL_FAILURE(OpenProfileChooserView(browser()));
views::ButtonListener* bubble = current_profile_bubble();
const ui::MouseEvent event(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
ui::EventTimeForNow(), 0, 0);
base::UserActionTester tester;
bubble->ButtonPressed(signin_current_profile_button(), event);
EXPECT_EQ(1, tester.GetActionCount("Signin_Signin_FromAvatarBubbleSignin"));
}
// Make sure nothing bad happens when the browser theme changes while the
// ProfileChooserView is visible. Regression test for crbug.com/737470
IN_PROC_BROWSER_TEST_F(ProfileChooserViewExtensionsTest, ThemeChanged) {
......
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