Skip to content

Commit

Permalink
Fix a regression caused by my change to make the first run bubble win…
Browse files Browse the repository at this point in the history 
…dow display as an inactive window.

The expectation is that we should dismiss the bubble on activities like mouse down, touch down, on the anchor
window as well.

The FirstRunBubbleCloser class now observes the native view instead of the root view as the latter does not work
if we have mouse and touch events on the RWHVA.

BUG=555416
TEST=Covered by tests FirstRunBubbleTest.CloseBubbleOnMouseDownEvent and FirstRunBubbleTest.CloseBubbleOnTouchDownEvent

Review URL: https://codereview.chromium.org/1443253004

Cr-Commit-Position: refs/heads/master@{#359902}
  • Loading branch information
ananta authored and Commit bot committed Nov 16, 2015
1 parent 7e9135e commit 50a8b1f
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 65 deletions.
43 changes: 31 additions & 12 deletions chrome/browser/ui/views/first_run_bubble.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "chrome/browser/ui/chrome_pages.h"
#include "chrome/grit/generated_resources.h"
#include "components/search_engines/util.h"
#include "ui/aura/window.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/resource/resource_bundle.h"
#include "ui/views/controls/label.h"
Expand Down Expand Up @@ -103,30 +104,48 @@ FirstRunBubble::FirstRunBubbleCloser::FirstRunBubbleCloser(
views::View* anchor_view)
: bubble_(bubble),
anchor_widget_(anchor_view->GetWidget()) {
AddKeyboardEventObserver();
AddEventObservers();
}

FirstRunBubble::FirstRunBubbleCloser::~FirstRunBubbleCloser() {
if (anchor_widget_)
RemoveKeyboardEventObserver();
RemoveEventObservers();
}

void FirstRunBubble::FirstRunBubbleCloser::OnKeyEvent(ui::KeyEvent* event) {
if (!anchor_widget_)
return;
CloseBubble();
}

RemoveKeyboardEventObserver();
DCHECK(bubble_);
bubble_->GetWidget()->Close();
bubble_ = nullptr;
void FirstRunBubble::FirstRunBubbleCloser::OnMouseEvent(
ui::MouseEvent* event) {
if (event->type() == ui::ET_MOUSE_PRESSED)
CloseBubble();
}

void FirstRunBubble::FirstRunBubbleCloser::OnGestureEvent(
ui::GestureEvent* event) {
if (event->type() == ui::ET_GESTURE_TAP ||
event->type() == ui::ET_GESTURE_TAP_DOWN) {
CloseBubble();
}
}

void FirstRunBubble::FirstRunBubbleCloser::AddKeyboardEventObserver() {
anchor_widget_->GetRootView()->AddPreTargetHandler(this);
void FirstRunBubble::FirstRunBubbleCloser::AddEventObservers() {
anchor_widget_->GetNativeView()->AddPreTargetHandler(this);
}

void FirstRunBubble::FirstRunBubbleCloser::RemoveKeyboardEventObserver() {
void FirstRunBubble::FirstRunBubbleCloser::RemoveEventObservers() {
DCHECK(anchor_widget_);
anchor_widget_->GetRootView()->RemovePreTargetHandler(this);
anchor_widget_->GetNativeView()->RemovePreTargetHandler(this);
anchor_widget_ = nullptr;
}

void FirstRunBubble::FirstRunBubbleCloser::CloseBubble() {
if (!anchor_widget_)
return;

RemoveEventObservers();
DCHECK(bubble_);
bubble_->GetWidget()->Close();
bubble_ = nullptr;
}
14 changes: 10 additions & 4 deletions chrome/browser/ui/views/first_run_bubble.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#define CHROME_BROWSER_UI_VIEWS_FIRST_RUN_BUBBLE_H_

#include "base/macros.h"
#include "ui/events/event.h"
#include "ui/views/bubble/bubble_delegate.h"
#include "ui/views/controls/link_listener.h"

Expand All @@ -25,19 +26,24 @@ class FirstRunBubble : public views::BubbleDelegateView,
FirstRunBubble(Browser* browser, views::View* anchor_view);
~FirstRunBubble() override;

// This class observes keyboard events targeted towards the target view
// dismisses the first run bubble accordingly.
// This class observes keyboard events, mouse clicks and touch down events
// targeted towards the anchor widget and dismisses the first run bubble
// accordingly.
class FirstRunBubbleCloser : public ui::EventHandler {
public:
FirstRunBubbleCloser(FirstRunBubble* bubble, views::View* anchor_view);
~FirstRunBubbleCloser() override;

// ui::EventHandler overrides.
void OnKeyEvent(ui::KeyEvent* event) override;
void OnMouseEvent(ui::MouseEvent* event) override;
void OnGestureEvent(ui::GestureEvent* event) override;

private:
void AddKeyboardEventObserver();
void RemoveKeyboardEventObserver();
void AddEventObservers();
void RemoveEventObservers();

void CloseBubble();

// The bubble instance.
FirstRunBubble* bubble_;
Expand Down
120 changes: 71 additions & 49 deletions chrome/browser/ui/views/first_run_bubble_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,44 @@
#include "ui/aura/window_tree_host.h"
#include "ui/events/event.h"
#include "ui/events/event_processor.h"
#include "ui/events/event_utils.h"
#include "ui/views/test/views_test_base.h"
#include "ui/views/view.h"
#include "ui/views/widget/widget.h"

// Provides functionality to observe the widget passed in the constructor for
// the widget closing event.
class WidgetClosingObserver : public views::WidgetObserver {
public:
explicit WidgetClosingObserver(views::Widget* widget)
: widget_(widget),
widget_destroyed_(false) {
widget_->AddObserver(this);
}

~WidgetClosingObserver() override {
if (widget_)
widget_->RemoveObserver(this);
}

void OnWidgetClosing(views::Widget* widget) override {
DCHECK(widget == widget_);
widget_->RemoveObserver(this);
widget_destroyed_ = true;
widget_ = nullptr;
}

bool widget_destroyed() const {
return widget_destroyed_;
}

private:
views::Widget* widget_;
bool widget_destroyed_;

DISALLOW_COPY_AND_ASSIGN(WidgetClosingObserver);
};

class FirstRunBubbleTest : public views::ViewsTestBase,
views::WidgetObserver {
public:
Expand All @@ -27,6 +61,8 @@ class FirstRunBubbleTest : public views::ViewsTestBase,
void SetUp() override;
void TearDown() override;

void CreateAndCloseBubbleOnEventTest(ui::Event* event);

protected:
TestingProfile* profile() { return profile_.get(); }

Expand Down Expand Up @@ -56,57 +92,35 @@ void FirstRunBubbleTest::TearDown() {
TestingBrowserProcess::DeleteInstance();
}

// Provides functionality to observe the widget passed in the constructor for
// the widget closing event.
class WidgetClosingObserver : public views::WidgetObserver {
public:
WidgetClosingObserver(views::Widget* widget)
: widget_(widget),
widget_destroyed_(false) {
widget_->AddObserver(this);
}

~WidgetClosingObserver() override {
if (widget_)
widget_->RemoveObserver(this);
}

void OnWidgetClosing(views::Widget* widget) override {
DCHECK(widget == widget_);
widget_->RemoveObserver(this);
widget_destroyed_ = true;
widget_ = nullptr;
}

bool widget_destroyed() const {
return widget_destroyed_;
}

private:
views::Widget* widget_;
bool widget_destroyed_;

DISALLOW_COPY_AND_ASSIGN(WidgetClosingObserver);
};

TEST_F(FirstRunBubbleTest, CreateAndClose) {
void FirstRunBubbleTest::CreateAndCloseBubbleOnEventTest(ui::Event* event) {
// Create the anchor and parent widgets.
views::Widget::InitParams params =
CreateParams(views::Widget::InitParams::TYPE_WINDOW);
params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
scoped_ptr<views::Widget> anchor_widget(new views::Widget);
anchor_widget->Init(params);
anchor_widget->SetBounds(gfx::Rect(10, 10, 500, 500));
anchor_widget->Show();

FirstRunBubble* delegate =
FirstRunBubble::ShowBubble(NULL, anchor_widget->GetContentsView());
EXPECT_TRUE(delegate != NULL);
delegate->GetWidget()->CloseNow();

anchor_widget->GetFocusManager()->SetFocusedView(
anchor_widget->GetContentsView());

scoped_ptr<WidgetClosingObserver> widget_observer(
new WidgetClosingObserver(delegate->GetWidget()));

ui::EventDispatchDetails details =
anchor_widget->GetNativeWindow()->GetHost()->event_processor()->
OnEventFromSource(event);
EXPECT_FALSE(details.dispatcher_destroyed);

EXPECT_TRUE(widget_observer->widget_destroyed());
}

// Tests that the first run bubble is closed when keyboard events are
// dispatched to the anchor widget.
TEST_F(FirstRunBubbleTest, CloseBubbleOnKeyEvent) {
TEST_F(FirstRunBubbleTest, CreateAndClose) {
// Create the anchor and parent widgets.
views::Widget::InitParams params =
CreateParams(views::Widget::InitParams::TYPE_WINDOW);
Expand All @@ -118,19 +132,27 @@ TEST_F(FirstRunBubbleTest, CloseBubbleOnKeyEvent) {
FirstRunBubble* delegate =
FirstRunBubble::ShowBubble(NULL, anchor_widget->GetContentsView());
EXPECT_TRUE(delegate != NULL);
delegate->GetWidget()->CloseNow();
}

anchor_widget->GetFocusManager()->SetFocusedView(
anchor_widget->GetContentsView());

scoped_ptr<WidgetClosingObserver> widget_observer(
new WidgetClosingObserver(delegate->GetWidget()));

// Tests that the first run bubble is closed when keyboard events are
// dispatched to the anchor widget.
TEST_F(FirstRunBubbleTest, CloseBubbleOnKeyEvent) {
ui::KeyEvent key_event(ui::ET_KEY_PRESSED, ui::VKEY_ESCAPE, ui::EF_NONE);
CreateAndCloseBubbleOnEventTest(&key_event);
}

ui::EventDispatchDetails details =
anchor_widget->GetNativeWindow()->GetHost()->event_processor()->
OnEventFromSource(&key_event);
EXPECT_FALSE(details.dispatcher_destroyed);
TEST_F(FirstRunBubbleTest, CloseBubbleOnMouseDownEvent) {
gfx::Point pt(110, 210);
ui::MouseEvent mouse_down(
ui::ET_MOUSE_PRESSED, pt, pt, ui::EventTimeForNow(),
ui::EF_LEFT_MOUSE_BUTTON, ui::EF_LEFT_MOUSE_BUTTON);
CreateAndCloseBubbleOnEventTest(&mouse_down);
}

EXPECT_TRUE(widget_observer->widget_destroyed());
TEST_F(FirstRunBubbleTest, CloseBubbleOnTouchDownEvent) {
ui::TouchEvent touch_down(
ui::ET_TOUCH_PRESSED, gfx::Point(10, 10), 0, ui::EventTimeForNow());
CreateAndCloseBubbleOnEventTest(&touch_down);
}

0 comments on commit 50a8b1f

Please sign in to comment.