Commit ac782973 authored by Elly Fong-Jones's avatar Elly Fong-Jones Committed by Commit Bot

polychrome: fix BrowserActionsBarBrowserTest tests

The process of fixing these tests uncovered a real bug in the Cocoa
BrowserActionsContainerView: it was always resizable even when highlighting.
This change:

1) Fixes that bug;
2) Adds an abstract CanBeResized test helper;
3) Implements that test helper for Cocoa and Views windows;
4) Marks the two drag & drop tests as Views-window-only, since the mechanics of
   drag & drop aren't exactly compatible between the two platforms.

Bug: 817408
Change-Id: I3a1316df05ff5aa1b3ae29c815fec6bac8062ad1
Reviewed-on: https://chromium-review.googlesource.com/963289
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: default avatarRobert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543398}
parent 3ebe17ec
...@@ -215,6 +215,7 @@ class BrowserActionTestUtilCocoa : public BrowserActionTestUtil { ...@@ -215,6 +215,7 @@ class BrowserActionTestUtilCocoa : public BrowserActionTestUtil {
std::unique_ptr<BrowserActionTestUtil> CreateOverflowBar() override; std::unique_ptr<BrowserActionTestUtil> CreateOverflowBar() override;
gfx::Size GetMinPopupSize() override; gfx::Size GetMinPopupSize() override;
gfx::Size GetMaxPopupSize() override; gfx::Size GetMaxPopupSize() override;
bool CanBeResized() override;
private: private:
friend class BrowserActionTestUtil; friend class BrowserActionTestUtil;
...@@ -334,6 +335,12 @@ gfx::Size BrowserActionTestUtilCocoa::GetMaxPopupSize() { ...@@ -334,6 +335,12 @@ gfx::Size BrowserActionTestUtilCocoa::GetMaxPopupSize() {
return GetExtensionPopupTestManager()->GetMaxPopupSize(); return GetExtensionPopupTestManager()->GetMaxPopupSize();
} }
bool BrowserActionTestUtilCocoa::CanBeResized() {
BrowserActionsContainerView* containerView =
[GetController(browser_, test_helper_.get()) containerView];
return [containerView canBeResized];
}
BrowserActionTestUtilCocoa::BrowserActionTestUtilCocoa( BrowserActionTestUtilCocoa::BrowserActionTestUtilCocoa(
Browser* browser, Browser* browser,
BrowserActionTestUtilCocoa* main_bar) BrowserActionTestUtilCocoa* main_bar)
......
...@@ -106,6 +106,9 @@ enum BrowserActionsContainerKeyAction { ...@@ -106,6 +106,9 @@ enum BrowserActionsContainerKeyAction {
// Stops any animation in progress. // Stops any animation in progress.
- (void)stopAnimation; - (void)stopAnimation;
// Returns true if this view is currently resizable.
- (BOOL)canBeResized;
@property(nonatomic) CGFloat minWidth; @property(nonatomic) CGFloat minWidth;
@property(nonatomic) CGFloat maxWidth; @property(nonatomic) CGFloat maxWidth;
@property(nonatomic) BOOL grippyPinned; @property(nonatomic) BOOL grippyPinned;
......
...@@ -140,7 +140,7 @@ const CGFloat kGrippyWidth = 3.0; ...@@ -140,7 +140,7 @@ const CGFloat kGrippyWidth = 3.0;
if (highlight || highlight_) { if (highlight || highlight_) {
highlight_ = std::move(highlight); highlight_ = std::move(highlight);
// We don't allow resizing when the container is highlighting. // We don't allow resizing when the container is highlighting.
resizable_ = highlight.get() == nullptr; resizable_ = highlight_.get() == nullptr;
[self setNeedsDisplay:YES]; [self setNeedsDisplay:YES];
} }
} }
...@@ -302,6 +302,10 @@ const CGFloat kGrippyWidth = 3.0; ...@@ -302,6 +302,10 @@ const CGFloat kGrippyWidth = 3.0;
[resizeAnimation_ stopAnimation]; [resizeAnimation_ stopAnimation];
} }
- (BOOL)canBeResized {
return resizable_;
}
#pragma mark - #pragma mark -
#pragma mark Private Methods #pragma mark Private Methods
......
...@@ -104,6 +104,9 @@ class BrowserActionTestUtil { ...@@ -104,6 +104,9 @@ class BrowserActionTestUtil {
// Returns the maximum allowed size of an extension popup. // Returns the maximum allowed size of an extension popup.
virtual gfx::Size GetMaxPopupSize() = 0; virtual gfx::Size GetMaxPopupSize() = 0;
// Returns whether the browser action container can currently be resized.
virtual bool CanBeResized() = 0;
protected: protected:
BrowserActionTestUtil() {} BrowserActionTestUtil() {}
......
...@@ -198,6 +198,19 @@ gfx::Size BrowserActionTestUtilViews::GetMaxPopupSize() { ...@@ -198,6 +198,19 @@ gfx::Size BrowserActionTestUtilViews::GetMaxPopupSize() {
return gfx::Size(ExtensionPopup::kMaxWidth, ExtensionPopup::kMaxHeight); return gfx::Size(ExtensionPopup::kMaxWidth, ExtensionPopup::kMaxHeight);
} }
bool BrowserActionTestUtilViews::CanBeResized() {
BrowserActionsContainer* container =
BrowserView::GetBrowserViewForBrowser(browser_)
->toolbar()
->browser_actions();
// The container can only be resized if we can start a drag for the view.
DCHECK_LE(1u, container->num_toolbar_actions());
ToolbarActionView* action_view = container->GetToolbarActionViewAt(0);
gfx::Point point(action_view->x(), action_view->y());
return container->CanStartDragForView(action_view, point, point);
}
BrowserActionTestUtilViews::BrowserActionTestUtilViews( BrowserActionTestUtilViews::BrowserActionTestUtilViews(
Browser* browser, Browser* browser,
BrowserActionTestUtilViews* main_bar) BrowserActionTestUtilViews* main_bar)
......
...@@ -32,6 +32,7 @@ class BrowserActionTestUtilViews : public BrowserActionTestUtil { ...@@ -32,6 +32,7 @@ class BrowserActionTestUtilViews : public BrowserActionTestUtil {
std::unique_ptr<BrowserActionTestUtil> CreateOverflowBar() override; std::unique_ptr<BrowserActionTestUtil> CreateOverflowBar() override;
gfx::Size GetMinPopupSize() override; gfx::Size GetMinPopupSize() override;
gfx::Size GetMaxPopupSize() override; gfx::Size GetMaxPopupSize() override;
bool CanBeResized() override;
private: private:
friend class BrowserActionTestUtil; friend class BrowserActionTestUtil;
......
...@@ -18,6 +18,7 @@ ...@@ -18,6 +18,7 @@
#include "chrome/browser/ui/views/frame/browser_view.h" #include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/toolbar/toolbar_action_view.h" #include "chrome/browser/ui/views/toolbar/toolbar_action_view.h"
#include "chrome/browser/ui/views/toolbar/toolbar_view.h" #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
#include "chrome/test/views/scoped_macviews_browser_mode.h"
#include "extensions/browser/extension_prefs.h" #include "extensions/browser/extension_prefs.h"
#include "extensions/browser/extension_registry.h" #include "extensions/browser/extension_registry.h"
#include "extensions/common/extension.h" #include "extensions/common/extension.h"
...@@ -27,6 +28,8 @@ ...@@ -27,6 +28,8 @@
#include "ui/views/test/test_views.h" #include "ui/views/test/test_views.h"
#include "ui/views/view.h" #include "ui/views/view.h"
namespace {
// TODO(devlin): Continue moving any tests that should be platform independent // TODO(devlin): Continue moving any tests that should be platform independent
// from this file to the crossplatform tests in // from this file to the crossplatform tests in
// chrome/browser/ui/toolbar/browser_actions_bar_browsertest.cc. // chrome/browser/ui/toolbar/browser_actions_bar_browsertest.cc.
...@@ -34,7 +37,16 @@ ...@@ -34,7 +37,16 @@
// Test that dragging browser actions works, and that dragging a browser action // Test that dragging browser actions works, and that dragging a browser action
// from the overflow menu results in it "popping" out (growing the container // from the overflow menu results in it "popping" out (growing the container
// size by 1), rather than just reordering the extensions. // size by 1), rather than just reordering the extensions.
IN_PROC_BROWSER_TEST_F(BrowserActionsBarBrowserTest, DragBrowserActions) {
// The two drag & drop tests are currently restricted to Views browsers in the
// absence of a good way to abstract drag & drop actions.
class BrowserActionsBarViewsBrowserTest : public BrowserActionsBarBrowserTest {
private:
test::ScopedMacViewsBrowserMode views_mode_{true};
};
} // namespace
IN_PROC_BROWSER_TEST_F(BrowserActionsBarViewsBrowserTest, DragBrowserActions) {
LoadExtensions(); LoadExtensions();
// Sanity check: All extensions showing; order is A B C. // Sanity check: All extensions showing; order is A B C.
...@@ -147,7 +159,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionsBarBrowserTest, DragBrowserActions) { ...@@ -147,7 +159,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionsBarBrowserTest, DragBrowserActions) {
// Test that changes performed in one container affect containers in other // Test that changes performed in one container affect containers in other
// windows so that it is consistent. // windows so that it is consistent.
IN_PROC_BROWSER_TEST_F(BrowserActionsBarBrowserTest, MultipleWindows) { IN_PROC_BROWSER_TEST_F(BrowserActionsBarViewsBrowserTest, MultipleWindows) {
LoadExtensions(); LoadExtensions();
BrowserActionsContainer* first = BrowserActionsContainer* first =
BrowserView::GetBrowserViewForBrowser(browser())->toolbar()-> BrowserView::GetBrowserViewForBrowser(browser())->toolbar()->
...@@ -214,16 +226,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionsBarBrowserTest, HighlightMode) { ...@@ -214,16 +226,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionsBarBrowserTest, HighlightMode) {
EXPECT_EQ(3, browser_actions_bar()->VisibleBrowserActions()); EXPECT_EQ(3, browser_actions_bar()->VisibleBrowserActions());
EXPECT_EQ(3, browser_actions_bar()->NumberOfBrowserActions()); EXPECT_EQ(3, browser_actions_bar()->NumberOfBrowserActions());
EXPECT_TRUE(browser_actions_bar()->CanBeResized());
BrowserActionsContainer* container =
BrowserView::GetBrowserViewForBrowser(browser())
->toolbar()->browser_actions();
// Currently, dragging should be enabled.
ToolbarActionView* action_view = container->GetToolbarActionViewAt(0);
ASSERT_TRUE(action_view);
gfx::Point point(action_view->x(), action_view->y());
EXPECT_TRUE(container->CanStartDragForView(action_view, point, point));
std::vector<std::string> action_ids; std::vector<std::string> action_ids;
action_ids.push_back(extension_a()->id()); action_ids.push_back(extension_a()->id());
...@@ -236,15 +239,13 @@ IN_PROC_BROWSER_TEST_F(BrowserActionsBarBrowserTest, HighlightMode) { ...@@ -236,15 +239,13 @@ IN_PROC_BROWSER_TEST_F(BrowserActionsBarBrowserTest, HighlightMode) {
EXPECT_EQ(2, browser_actions_bar()->NumberOfBrowserActions()); EXPECT_EQ(2, browser_actions_bar()->NumberOfBrowserActions());
// We shouldn't be able to drag in highlight mode. // We shouldn't be able to drag in highlight mode.
action_view = container->GetToolbarActionViewAt(0); EXPECT_FALSE(browser_actions_bar()->CanBeResized());
EXPECT_FALSE(container->CanStartDragForView(action_view, point, point));
// We should go back to normal after leaving highlight mode. // We should go back to normal after leaving highlight mode.
toolbar_model()->StopHighlighting(); toolbar_model()->StopHighlighting();
EXPECT_EQ(3, browser_actions_bar()->VisibleBrowserActions()); EXPECT_EQ(3, browser_actions_bar()->VisibleBrowserActions());
EXPECT_EQ(3, browser_actions_bar()->NumberOfBrowserActions()); EXPECT_EQ(3, browser_actions_bar()->NumberOfBrowserActions());
action_view = container->GetToolbarActionViewAt(0); EXPECT_TRUE(browser_actions_bar()->CanBeResized());
EXPECT_TRUE(container->CanStartDragForView(action_view, point, point));
} }
// Test the behavior of the overflow container for Extension Actions. // Test the behavior of the overflow container for Extension Actions.
......
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