Commit 4e890408 authored by Mike Wasserman's avatar Mike Wasserman Committed by Commit Bot

Fix ContentSettingBubbleContentsBrowserTest to work on Chrome OS.

Add plumbing to check if a content setting bubble is showing.
Use that to test for bubble presence, not GetAllWidgets.
See http://crrev.com/c/922268 for GetAllWidgets changes.

Bug: 811940
Change-Id: I5b8e36c5dcddb8dfd407ad0f889ad74fb9804273
Reviewed-on: https://chromium-review.googlesource.com/919922Reviewed-by: default avatarElly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537212}
parent d6f6d84c
...@@ -37,6 +37,9 @@ class ContentSettingDecoration : public ImageDecoration { ...@@ -37,6 +37,9 @@ class ContentSettingDecoration : public ImageDecoration {
// Returns true if the decoration's visible state changed. // Returns true if the decoration's visible state changed.
bool UpdateFromWebContents(content::WebContents* web_contents); bool UpdateFromWebContents(content::WebContents* web_contents);
// Returns if the content setting bubble is showing for this decoration.
bool IsShowingBubble() const;
// Overridden from |LocationBarDecoration| // Overridden from |LocationBarDecoration|
AcceptsPress AcceptsMousePress() override; AcceptsPress AcceptsMousePress() override;
bool OnMousePressed(NSRect frame, NSPoint location) override; bool OnMousePressed(NSRect frame, NSPoint location) override;
......
...@@ -228,6 +228,10 @@ bool ContentSettingDecoration::UpdateFromWebContents( ...@@ -228,6 +228,10 @@ bool ContentSettingDecoration::UpdateFromWebContents(
return decoration_changed; return decoration_changed;
} }
bool ContentSettingDecoration::IsShowingBubble() const {
return bubbleWindow_ && [bubbleWindow_ isVisible];
}
CGFloat ContentSettingDecoration::MeasureTextWidth() { CGFloat ContentSettingDecoration::MeasureTextWidth() {
return [animated_text_ size].width; return [animated_text_ size].width;
} }
......
...@@ -88,6 +88,7 @@ class LocationBarViewMac : public LocationBar, ...@@ -88,6 +88,7 @@ class LocationBarViewMac : public LocationBar,
// Overridden from LocationBarTesting: // Overridden from LocationBarTesting:
bool GetBookmarkStarVisibility() override; bool GetBookmarkStarVisibility() override;
bool TestContentSettingImagePressed(size_t index) override; bool TestContentSettingImagePressed(size_t index) override;
bool IsContentSettingBubbleShowing(size_t index) override;
// Set/Get the editable state of the field. // Set/Get the editable state of the field.
void SetEditable(bool editable); void SetEditable(bool editable);
......
...@@ -266,6 +266,11 @@ bool LocationBarViewMac::TestContentSettingImagePressed(size_t index) { ...@@ -266,6 +266,11 @@ bool LocationBarViewMac::TestContentSettingImagePressed(size_t index) {
return true; return true;
} }
bool LocationBarViewMac::IsContentSettingBubbleShowing(size_t index) {
return index < content_setting_decorations_.size() &&
content_setting_decorations_[index]->IsShowingBubble();
}
void LocationBarViewMac::SetEditable(bool editable) { void LocationBarViewMac::SetEditable(bool editable) {
[field_ setEditable:editable ? YES : NO]; [field_ setEditable:editable ? YES : NO];
UpdateBookmarkStarVisibility(); UpdateBookmarkStarVisibility();
......
...@@ -105,6 +105,9 @@ class LocationBarTesting { ...@@ -105,6 +105,9 @@ class LocationBarTesting {
// Returns false if there is none. // Returns false if there is none.
virtual bool TestContentSettingImagePressed(size_t index) = 0; virtual bool TestContentSettingImagePressed(size_t index) = 0;
// Returns if the content setting image at |index| is displaying a bubble.
virtual bool IsContentSettingBubbleShowing(size_t index) = 0;
protected: protected:
virtual ~LocationBarTesting() {} virtual ~LocationBarTesting() {}
}; };
......
...@@ -4,8 +4,6 @@ ...@@ -4,8 +4,6 @@
#include "chrome/browser/ui/views/content_setting_bubble_contents.h" #include "chrome/browser/ui/views/content_setting_bubble_contents.h"
#include <algorithm>
#include "base/run_loop.h" #include "base/run_loop.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/permissions/permission_request_manager_test_api.h" #include "chrome/browser/permissions/permission_request_manager_test_api.h"
...@@ -16,13 +14,13 @@ ...@@ -16,13 +14,13 @@
#include "chrome/test/base/in_process_browser_test.h" #include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h" #include "chrome/test/base/ui_test_utils.h"
#include "content/public/test/browser_test_utils.h" #include "content/public/test/browser_test_utils.h"
#include "ui/views/test/widget_test.h"
#include "ui/views/widget/widget.h"
class ContentSettingBubbleContentsBrowserTest : public InProcessBrowserTest { class ContentSettingBubbleContentsBrowserTest : public InProcessBrowserTest {
protected: public:
using Widgets = views::Widget::Widgets; ContentSettingBubbleContentsBrowserTest() = default;
~ContentSettingBubbleContentsBrowserTest() override {}
protected:
GURL GetTestPageUrl(const std::string& name) { GURL GetTestPageUrl(const std::string& name) {
return ui_test_utils::GetTestUrl( return ui_test_utils::GetTestUrl(
base::FilePath().AppendASCII("content_setting_bubble"), base::FilePath().AppendASCII("content_setting_bubble"),
...@@ -37,26 +35,13 @@ class ContentSettingBubbleContentsBrowserTest : public InProcessBrowserTest { ...@@ -37,26 +35,13 @@ class ContentSettingBubbleContentsBrowserTest : public InProcessBrowserTest {
return content::ExecuteScript(GetWebContents(), script); return content::ExecuteScript(GetWebContents(), script);
} }
Widgets GetWidgetsNotIn(Widgets widgets) { private:
Widgets new_widgets = views::test::WidgetTest::GetAllWidgets(); DISALLOW_COPY_AND_ASSIGN(ContentSettingBubbleContentsBrowserTest);
Widgets result;
std::set_difference(new_widgets.begin(), new_widgets.end(), widgets.begin(),
widgets.end(), std::inserter(result, result.begin()));
return result;
}
}; };
#if defined(OS_CHROMEOS)
// https://crbug.com/811940
#define MAYBE_HidesAtWebContentsClose DISABLED_HidesAtWebContentsClose
#else
#define MAYBE_HidesAtWebContentsClose HidesAtWebContentsClose
#endif
IN_PROC_BROWSER_TEST_F(ContentSettingBubbleContentsBrowserTest, IN_PROC_BROWSER_TEST_F(ContentSettingBubbleContentsBrowserTest,
MAYBE_HidesAtWebContentsClose) { HidesAtWebContentsClose) {
// Create a second tab, so that closing the test tab doesn't close the entire // Create a second tab, so closing the test tab doesn't close the browser.
// browser.
chrome::NewTab(browser()); chrome::NewTab(browser());
// Navigate to the test page, and have it request and be denied geolocation // Navigate to the test page, and have it request and be denied geolocation
...@@ -66,19 +51,18 @@ IN_PROC_BROWSER_TEST_F(ContentSettingBubbleContentsBrowserTest, ...@@ -66,19 +51,18 @@ IN_PROC_BROWSER_TEST_F(ContentSettingBubbleContentsBrowserTest,
->set_auto_response_for_test(PermissionRequestManager::DISMISS); ->set_auto_response_for_test(PermissionRequestManager::DISMISS);
ExecuteScript("geolocate();"); ExecuteScript("geolocate();");
// Press the geolocation icon in the location bar, and make sure a new widget // Press the geolocation icon and make sure its content setting bubble shows.
// (the content setting bubble) appears.
LocationBarTesting* bar = LocationBarTesting* bar =
browser()->window()->GetLocationBar()->GetLocationBarForTesting(); browser()->window()->GetLocationBar()->GetLocationBarForTesting();
Widgets before = GetWidgetsNotIn({});
EXPECT_TRUE(bar->TestContentSettingImagePressed( EXPECT_TRUE(bar->TestContentSettingImagePressed(
static_cast<size_t>(ContentSettingImageModel::ImageType::GEOLOCATION))); static_cast<size_t>(ContentSettingImageModel::ImageType::GEOLOCATION)));
EXPECT_EQ(1u, GetWidgetsNotIn(before).size()); EXPECT_TRUE(bar->IsContentSettingBubbleShowing(
static_cast<size_t>(ContentSettingImageModel::ImageType::GEOLOCATION)));
// Now close the tab, and make sure the content setting bubble's widget is // Close the tab, and make sure the bubble is gone. Note that window closure
// gone. Note that window closure in Aura is asynchronous, so it's necessary // in Aura is asynchronous, so it's necessary to spin the run loop here.
// to spin the runloop here.
chrome::CloseTab(browser()); chrome::CloseTab(browser());
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_EQ(0u, GetWidgetsNotIn(before).size()); EXPECT_FALSE(bar->IsContentSettingBubbleShowing(
static_cast<size_t>(ContentSettingImageModel::ImageType::GEOLOCATION)));
} }
...@@ -66,11 +66,6 @@ class ContentSettingImageView : public IconLabelBubbleView { ...@@ -66,11 +66,6 @@ class ContentSettingImageView : public IconLabelBubbleView {
void disable_animation() { can_animate_ = false; } void disable_animation() { can_animate_ = false; }
private:
// The total animation time, including open and close as well as an
// intervening "stay open" period.
static const int kAnimationDurationMS;
// IconLabelBubbleView: // IconLabelBubbleView:
const char* GetClassName() const override; const char* GetClassName() const override;
void OnBoundsChanged(const gfx::Rect& previous_bounds) override; void OnBoundsChanged(const gfx::Rect& previous_bounds) override;
...@@ -85,6 +80,11 @@ class ContentSettingImageView : public IconLabelBubbleView { ...@@ -85,6 +80,11 @@ class ContentSettingImageView : public IconLabelBubbleView {
bool ShowBubble(const ui::Event& event) override; bool ShowBubble(const ui::Event& event) override;
bool IsBubbleShowing() const override; bool IsBubbleShowing() const override;
private:
// The total animation time, including open and close as well as an
// intervening "stay open" period.
static const int kAnimationDurationMS;
// gfx::AnimationDelegate: // gfx::AnimationDelegate:
void AnimationEnded(const gfx::Animation* animation) override; void AnimationEnded(const gfx::Animation* animation) override;
void AnimationProgressed(const gfx::Animation* animation) override; void AnimationProgressed(const gfx::Animation* animation) override;
......
...@@ -1019,6 +1019,11 @@ bool LocationBarView::TestContentSettingImagePressed(size_t index) { ...@@ -1019,6 +1019,11 @@ bool LocationBarView::TestContentSettingImagePressed(size_t index) {
return true; return true;
} }
bool LocationBarView::IsContentSettingBubbleShowing(size_t index) {
return index < content_setting_views_.size() &&
content_setting_views_[index]->IsBubbleShowing();
}
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
// LocationBarView, private views::View implementation: // LocationBarView, private views::View implementation:
......
...@@ -331,6 +331,7 @@ class LocationBarView : public LocationBar, ...@@ -331,6 +331,7 @@ class LocationBarView : public LocationBar,
// LocationBarTesting: // LocationBarTesting:
bool GetBookmarkStarVisibility() override; bool GetBookmarkStarVisibility() override;
bool TestContentSettingImagePressed(size_t index) override; bool TestContentSettingImagePressed(size_t index) override;
bool IsContentSettingBubbleShowing(size_t index) override;
// views::View: // views::View:
const char* GetClassName() const override; const char* GetClassName() const override;
......
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