Skip to content

Commit

Permalink
Upstream HostedAppButtonContainer container logic into BrowserNonClie…
Browse files Browse the repository at this point in the history 
…ntFrameView

Subclasses of BrowserNonClientFrameView that contain a HostedAppButtonContainer
child view have duplicate logic outside of creation, colors and layout.

This CL dedupes the logic involved in having a HostedAppButtonContainer
in preparation for adding one to BrowserNonClientFrameViewMac.

Bug: 895690
Change-Id: If2e5d58af58cd72ea9d0902d3a15f3375a9e1585
Reviewed-on: https://chromium-review.googlesource.com/c/1288092
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601427}
  • Loading branch information
@alancutter
alancutter authored and Commit Bot committed Oct 21, 2018
1 parent aa05092 commit 723b1ee
Show file tree
Hide file tree
Showing 10 changed files with 83 additions and 111 deletions.
27 changes: 27 additions & 0 deletions chrome/browser/ui/views/frame/browser_non_client_frame_view.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,20 @@
#include "chrome/browser/ui/layout_constants.h"
#include "chrome/browser/ui/view_ids.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/frame/hosted_app_button_container.h"
#include "chrome/browser/ui/views/tabs/tab_strip.h"
#include "chrome/common/chrome_features.h"
#include "chrome/grit/theme_resources.h"
#include "third_party/skia/include/core/SkColor.h"
#include "ui/base/hit_test.h"
#include "ui/base/theme_provider.h"
#include "ui/gfx/canvas.h"
#include "ui/gfx/color_palette.h"
#include "ui/gfx/image/image.h"
#include "ui/gfx/paint_vector_icon.h"
#include "ui/gfx/scoped_canvas.h"
#include "ui/views/background.h"
#include "ui/views/window/hit_test_utils.h"

#if defined(OS_WIN)
#include "chrome/browser/ui/views/frame/taskbar_decorator_win.h"
Expand Down Expand Up @@ -256,6 +259,22 @@ void BrowserNonClientFrameView::VisibilityChanged(views::View* starting_from,
OnProfileAvatarChanged(base::FilePath());
}

int BrowserNonClientFrameView::NonClientHitTest(const gfx::Point& point) {
if (hosted_app_button_container_) {
int hosted_app_component =
views::GetHitTestComponent(hosted_app_button_container_, point);
if (hosted_app_component != HTNOWHERE)
return hosted_app_component;
}

return HTNOWHERE;
}

void BrowserNonClientFrameView::ResetWindowControls() {
if (hosted_app_button_container_)
hosted_app_button_container_->UpdateContentSettingViewsVisibility();
}

void BrowserNonClientFrameView::OnSingleTabModeChanged() {
SchedulePaint();
}
Expand Down Expand Up @@ -326,6 +345,11 @@ gfx::ImageSkia BrowserNonClientFrameView::GetFrameOverlayImage(
: gfx::ImageSkia();
}

void BrowserNonClientFrameView::ChildPreferredSizeChanged(views::View* child) {
if (browser_view()->initialized() && child == hosted_app_button_container_)
Layout();
}

void BrowserNonClientFrameView::ActivationChanged(bool active) {
// On Windows, while deactivating the widget, this is called before the
// active HWND has actually been changed. Since we want the state to reflect
Expand All @@ -341,6 +365,9 @@ void BrowserNonClientFrameView::ActivationChanged(bool active) {

set_active_state_override(nullptr);

if (hosted_app_button_container_)
hosted_app_button_container_->SetPaintAsActive(active);

// Changing the activation state may change the visible frame color.
SchedulePaint();
}
Expand Down
22 changes: 22 additions & 0 deletions chrome/browser/ui/views/frame/browser_non_client_frame_view.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

class BrowserFrame;
class BrowserView;
class HostedAppButtonContainer;

// A specialization of the NonClientFrameView object that provides additional
// Browser-specific methods.
Expand Down Expand Up @@ -134,10 +135,16 @@ class BrowserNonClientFrameView : public views::NonClientFrameView,
// views::NonClientFrameView:
using views::NonClientFrameView::ShouldPaintAsActive;
void VisibilityChanged(views::View* starting_from, bool is_visible) override;
int NonClientHitTest(const gfx::Point& point) override;
void ResetWindowControls() override;

// TabStripObserver:
void OnSingleTabModeChanged() override;

HostedAppButtonContainer* hosted_app_button_container_for_testing() {
return hosted_app_button_container_;
}

protected:
// Whether the frame should be painted with theming.
// By default, tabbed browser windows are themed but popup and app windows are
Expand All @@ -157,6 +164,7 @@ class BrowserNonClientFrameView : public views::NonClientFrameView,
ActiveState active_state = kUseCurrent) const;

// views::NonClientFrameView:
void ChildPreferredSizeChanged(views::View* child) override;
void ActivationChanged(bool active) override;
bool DoesIntersectRect(const views::View* target,
const gfx::Rect& rect) const override;
Expand All @@ -169,6 +177,17 @@ class BrowserNonClientFrameView : public views::NonClientFrameView,
void OnProfileHighResAvatarLoaded(
const base::FilePath& profile_path) override;

void set_hosted_app_button_container(
HostedAppButtonContainer* hosted_app_button_container) {
hosted_app_button_container_ = hosted_app_button_container;
}
HostedAppButtonContainer* hosted_app_button_container() {
return hosted_app_button_container_;
}
const HostedAppButtonContainer* hosted_app_button_container() const {
return hosted_app_button_container_;
}

private:
void MaybeObserveTabstrip();

Expand All @@ -189,6 +208,9 @@ class BrowserNonClientFrameView : public views::NonClientFrameView,
// The BrowserView hosted within this View.
BrowserView* browser_view_;

// Menu button and page status icons. Only used by hosted app windows.
HostedAppButtonContainer* hosted_app_button_container_ = nullptr;

ScopedObserver<TabStrip, BrowserNonClientFrameView> tab_strip_observer_;

DISALLOW_COPY_AND_ASSIGN(BrowserNonClientFrameView);
Expand Down
30 changes: 10 additions & 20 deletions chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -343,11 +343,9 @@ void BrowserNonClientFrameViewAsh::GetWindowMask(const gfx::Size& size,
}

void BrowserNonClientFrameViewAsh::ResetWindowControls() {
BrowserNonClientFrameView::ResetWindowControls();
caption_button_container_->SetVisible(true);
caption_button_container_->ResetWindowControls();

if (hosted_app_button_container_)
hosted_app_button_container_->UpdateContentSettingViewsVisibility();
}

void BrowserNonClientFrameViewAsh::UpdateWindowIcon() {
Expand All @@ -369,9 +367,6 @@ void BrowserNonClientFrameViewAsh::ActivationChanged(bool active) {

const bool should_paint_as_active = ShouldPaintAsActive();
frame_header_->SetPaintAsActive(should_paint_as_active);

if (hosted_app_button_container_)
hosted_app_button_container_->SetPaintAsActive(should_paint_as_active);
}

///////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -401,8 +396,8 @@ void BrowserNonClientFrameViewAsh::Layout() {

if (profile_indicator_icon_)
LayoutProfileIndicator();
if (hosted_app_button_container_) {
hosted_app_button_container_->LayoutInContainer(
if (hosted_app_button_container()) {
hosted_app_button_container()->LayoutInContainer(
0, caption_button_container_->x(), 0, painted_height);
}

Expand Down Expand Up @@ -678,9 +673,9 @@ void BrowserNonClientFrameViewAsh::OnImmersiveRevealStarted() {
// temporarily children of the TopContainerView while they're all painting to
// their layers.
browser_view()->top_container()->AddChildViewAt(caption_button_container_, 0);
if (hosted_app_button_container_) {
if (hosted_app_button_container()) {
browser_view()->top_container()->AddChildViewAt(
hosted_app_button_container_, 0);
hosted_app_button_container(), 0);
}
if (back_button_)
browser_view()->top_container()->AddChildViewAt(back_button_, 0);
Expand All @@ -690,8 +685,8 @@ void BrowserNonClientFrameViewAsh::OnImmersiveRevealStarted() {

void BrowserNonClientFrameViewAsh::OnImmersiveRevealEnded() {
AddChildViewAt(caption_button_container_, 0);
if (hosted_app_button_container_)
AddChildViewAt(hosted_app_button_container_, 0);
if (hosted_app_button_container())
AddChildViewAt(hosted_app_button_container(), 0);
if (back_button_)
AddChildViewAt(back_button_, 0);
Layout();
Expand All @@ -701,11 +696,6 @@ void BrowserNonClientFrameViewAsh::OnImmersiveFullscreenExited() {
OnImmersiveRevealEnded();
}

HostedAppButtonContainer*
BrowserNonClientFrameViewAsh::GetHostedAppButtonContainerForTesting() const {
return hosted_app_button_container_;
}

// static
bool BrowserNonClientFrameViewAsh::UsePackagedAppHeaderStyle(
const Browser* browser) {
Expand Down Expand Up @@ -822,9 +812,9 @@ void BrowserNonClientFrameViewAsh::SetUpForHostedApp(
ash::FrameCaptionButton::GetInactiveButtonColorAlphaRatio();
SkColor inactive_color =
SkColorSetA(active_color, 255 * inactive_alpha_ratio);
hosted_app_button_container_ = new HostedAppButtonContainer(
frame(), browser_view(), active_color, inactive_color);
AddChildView(hosted_app_button_container_);
set_hosted_app_button_container(new HostedAppButtonContainer(
frame(), browser_view(), active_color, inactive_color));
AddChildView(hosted_app_button_container());
}

void BrowserNonClientFrameViewAsh::UpdateFrameColors() {
Expand Down
8 changes: 0 additions & 8 deletions chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ namespace {
class HostedAppNonClientFrameViewAshTest;
}

class HostedAppButtonContainer;
class ProfileIndicatorIcon;
class TabIconView;

Expand Down Expand Up @@ -132,8 +131,6 @@ class BrowserNonClientFrameViewAsh
void OnImmersiveRevealEnded() override;
void OnImmersiveFullscreenExited() override;

HostedAppButtonContainer* GetHostedAppButtonContainerForTesting() const;

// Returns true if the header should be painted so that it looks the same as
// the header used for packaged apps.
static bool UsePackagedAppHeaderStyle(const Browser* browser);
Expand Down Expand Up @@ -173,7 +170,6 @@ class BrowserNonClientFrameViewAsh
HeaderHeightForSnappedBrowserInSplitView);

friend class HostedAppNonClientFrameViewAshTest;
friend class ImmersiveModeControllerAshHostedAppBrowserTest;

// Returns whether the caption buttons should be visible. They are hidden, for
// example, in overview mode and tablet mode.
Expand Down Expand Up @@ -246,10 +242,6 @@ class BrowserNonClientFrameViewAsh
// A helper for controlling the window frame; only used in !Mash.
std::unique_ptr<ash::AshFrameCaptionController> caption_controller_;

// Container for extra frame buttons shown for hosted app windows.
// Owned by views hierarchy.
HostedAppButtonContainer* hosted_app_button_container_ = nullptr;

// Ash's mojom::SplitViewController.
ash::mojom::SplitViewControllerPtr split_view_controller_;

Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,7 @@ class HostedAppNonClientFrameViewAshTest
static_cast<ash::DefaultFrameHeader*>(frame_view->frame_header_.get());

hosted_app_button_container_ =
frame_view->GetHostedAppButtonContainerForTesting();
frame_view->hosted_app_button_container_for_testing();
DCHECK(hosted_app_button_container_);
DCHECK(hosted_app_button_container_->visible());

Expand Down
45 changes: 13 additions & 32 deletions chrome/browser/ui/views/frame/glass_browser_frame_view.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
#include "ui/strings/grit/ui_strings.h"
#include "ui/views/win/hwnd_util.h"
#include "ui/views/window/client_view.h"
#include "ui/views/window/hit_test_utils.h"

HICON GlassBrowserFrameView::throbber_icons_[
GlassBrowserFrameView::kThrobberIconCount];
Expand Down Expand Up @@ -119,9 +118,9 @@ GlassBrowserFrameView::GlassBrowserFrameView(BrowserFrame* frame,
// HostedAppButtonContainer::UpdateIconsColor() via a delegate interface.
SkColor active_color = GetTitlebarFeatureColor(kActive);
SkColor inactive_color = GetTitlebarFeatureColor(kInactive);
hosted_app_button_container_ = new HostedAppButtonContainer(
frame, browser_view, active_color, inactive_color);
AddChildView(hosted_app_button_container_);
set_hosted_app_button_container(new HostedAppButtonContainer(
frame, browser_view, active_color, inactive_color));
AddChildView(hosted_app_button_container());
}

minimize_button_ =
Expand Down Expand Up @@ -275,6 +274,10 @@ bool HitTestCaptionButton(Windows10CaptionButton* button,
} // namespace

int GlassBrowserFrameView::NonClientHitTest(const gfx::Point& point) {
int super_component = BrowserNonClientFrameView::NonClientHitTest(point);
if (super_component != HTNOWHERE)
return super_component;

// For app windows and popups without a custom titlebar we haven't customized
// the frame at all so Windows can figure it out.
if (!ShouldCustomDrawSystemTitlebar() &&
Expand All @@ -286,15 +289,6 @@ int GlassBrowserFrameView::NonClientHitTest(const gfx::Point& point) {
if (!bounds().Contains(point))
return HTNOWHERE;

if (hosted_app_button_container_) {
// TODO(alancutter): Assign hit test components to all children and refactor
// this entire function call to just be GetHitTestComponent(this, point).
int hosted_app_component =
views::GetHitTestComponent(hosted_app_button_container_, point);
if (hosted_app_component != HTNOWHERE)
return hosted_app_component;
}

int frame_component = frame()->client_view()->NonClientHitTest(point);

// See if we're in the sysmenu region. We still have to check the tabstrip
Expand Down Expand Up @@ -375,12 +369,11 @@ void GlassBrowserFrameView::UpdateWindowTitle() {
}

void GlassBrowserFrameView::ResetWindowControls() {
BrowserNonClientFrameView::ResetWindowControls();
minimize_button_->SetState(views::Button::STATE_NORMAL);
maximize_button_->SetState(views::Button::STATE_NORMAL);
restore_button_->SetState(views::Button::STATE_NORMAL);
close_button_->SetState(views::Button::STATE_NORMAL);
if (hosted_app_button_container_)
hosted_app_button_container_->UpdateContentSettingViewsVisibility();
}

void GlassBrowserFrameView::ButtonPressed(views::Button* sender,
Expand Down Expand Up @@ -418,11 +411,6 @@ const char* GlassBrowserFrameView::GetClassName() const {
return kClassName;
}

void GlassBrowserFrameView::ChildPreferredSizeChanged(views::View* child) {
if (browser_view()->initialized() && child == hosted_app_button_container_)
Layout();
}

void GlassBrowserFrameView::OnPaint(gfx::Canvas* canvas) {
TRACE_EVENT0("views.frame", "GlassBrowserFrameView::OnPaint");
if (ShouldCustomDrawSystemTitlebar())
Expand All @@ -443,13 +431,6 @@ void GlassBrowserFrameView::Layout() {
///////////////////////////////////////////////////////////////////////////////
// GlassBrowserFrameView, private:

void GlassBrowserFrameView::ActivationChanged(bool active) {
BrowserNonClientFrameView::ActivationChanged(active);

if (hosted_app_button_container_)
hosted_app_button_container_->SetPaintAsActive(active);
}

int GlassBrowserFrameView::FrameBorderThickness() const {
return (IsMaximized() || frame()->IsFullscreen())
? 0
Expand Down Expand Up @@ -514,13 +495,13 @@ int GlassBrowserFrameView::TopAreaHeight(bool restored) const {
int GlassBrowserFrameView::TitlebarMaximizedVisualHeight() const {
int maximized_height =
display::win::ScreenWin::GetSystemMetricsInDIP(SM_CYCAPTION);
if (hosted_app_button_container_) {
if (hosted_app_button_container()) {
// Adding 2px of vertical padding puts at least 1 px of space on the top and
// bottom of the element.
constexpr int kVerticalPadding = 2;
maximized_height =
std::max(maximized_height,
hosted_app_button_container_->GetPreferredSize().height() +
hosted_app_button_container()->GetPreferredSize().height() +
kVerticalPadding);
}
return maximized_height;
Expand Down Expand Up @@ -572,7 +553,7 @@ bool GlassBrowserFrameView::IsToolbarVisible() const {

bool GlassBrowserFrameView::ShowCustomIcon() const {
// Hosted app windows don't include the window icon as per UI mocks.
return !hosted_app_button_container_ && ShouldCustomDrawSystemTitlebar() &&
return !hosted_app_button_container() && ShouldCustomDrawSystemTitlebar() &&
browser_view()->ShouldShowWindowIcon();
}

Expand Down Expand Up @@ -706,8 +687,8 @@ void GlassBrowserFrameView::LayoutTitleBar() {
next_leading_x = window_icon_bounds.right() + kIconTitleSpacing;
}

if (hosted_app_button_container_) {
next_trailing_x = hosted_app_button_container_->LayoutInContainer(
if (hosted_app_button_container()) {
next_trailing_x = hosted_app_button_container()->LayoutInContainer(
next_leading_x, next_trailing_x, window_top, titlebar_visual_height);
}

Expand Down
Loading

0 comments on commit 723b1ee

Please sign in to comment.