Skip to content

Commit

Permalink
[ImmersiveFullscreenTabs] fix z-order of tab overlay widget
Browse files Browse the repository at this point in the history 
This CL makes the tab overlay widget a child of the overlay widget,
allowing for child widgets of the tab overlay widget to stack above the
top chrome content.

To accomplish this ImmersiveModeTitlebarObserver has been refactored to
forward frame KVO changes to the controller. This allows derived
controllers to respond to frame changes.

Also added a few tests for ImmersiveModeTabbedController.

Bug: 1414521
Change-Id: Icd37b210c3c5d4de970e8347d9b8243b085de0d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4312600
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Auto-Submit: Tom Burgin <bur@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1116494}
  • Loading branch information
Tom Burgin authored and Chromium LUCI CQ committed Mar 13, 2023
1 parent a4d15c8 commit ff78008
Show file tree
Hide file tree
Showing 7 changed files with 210 additions and 125 deletions.
18 changes: 10 additions & 8 deletions chrome/browser/ui/views/frame/browser_view.cc
View file
Expand Up @@ -3547,11 +3547,11 @@ views::View* BrowserView::CreateOverlayView() {
views::View* BrowserView::CreateMacOverlayView() {
DCHECK(UsesImmersiveFullscreenMode());

auto create_overlay_widget = [this]() -> views::Widget* {
auto create_overlay_widget = [this](views::Widget* parent) -> views::Widget* {
views::Widget::InitParams params;
params.type = views::Widget::InitParams::TYPE_POPUP;
params.child = true;
params.parent = GetWidget()->GetNativeView();
params.parent = parent->GetNativeView();
OverlayWidget* overlay_widget = new OverlayWidget(GetWidget());
overlay_widget->Init(std::move(params));
overlay_widget->SetNativeWindowProperty(kBrowserViewKey, this);
Expand All @@ -3569,7 +3569,7 @@ views::View* BrowserView::CreateMacOverlayView() {
};

// Create the toolbar overlay widget.
overlay_widget_ = create_overlay_widget();
overlay_widget_ = create_overlay_widget(GetWidget());

// Create a new TopContainerOverlayView. The tab strip, omnibox, bookmarks
// etc. will be contained within this view. Right clicking on the blank space
Expand All @@ -3587,17 +3587,19 @@ views::View* BrowserView::CreateMacOverlayView() {
overlay_widget_->GetRootView()->AddChildView(std::move(overlay_view));

if (base::FeatureList::IsEnabled(features::kImmersiveFullscreenTabs)) {
// Create the tab overlay widget.
tab_overlay_widget_ = create_overlay_widget();
// Create the tab overlay widget as a child of overlay_widget_.
tab_overlay_widget_ = create_overlay_widget(overlay_widget_);
std::unique_ptr<TabContainerOverlayView> tab_overlay_view =
std::make_unique<TabContainerOverlayView>(
weak_ptr_factory_.GetWeakPtr());
tab_overlay_view->set_context_menu_controller(frame());
tab_overlay_view_targeter_ =
std::make_unique<OverlayViewTargeterDelegate>();
tab_overlay_view->SetEventTargeter(std::make_unique<views::ViewTargeter>(
tab_overlay_view_targeter_.get()));
tab_overlay_view_ = tab_overlay_view.get();
tab_overlay_widget_->GetRootView()->AddChildView(
std::move(tab_overlay_view));

// TODO(https://crbug.com/1414521): Figure out how to handle multiple view
// targeters.
}

return overlay_view_;
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/ui/views/frame/browser_view.h
View file
Expand Up @@ -1048,6 +1048,10 @@ class BrowserView : public BrowserWindow,
// The hosting view of TabStripRegionView during immersive fullscreen.
raw_ptr<views::View, DanglingUntriaged> tab_overlay_view_ = nullptr;

// Targeter for the tab_overlay_view_. Ensures tab_overlay_view_ does not
// handle events, while allowing for child views to handle events.
std::unique_ptr<views::ViewTargeterDelegate> tab_overlay_view_targeter_;

#endif

// The Bookmark Bar View for this window. Lazily created. May be null for
Expand Down
13 changes: 10 additions & 3 deletions components/remote_cocoa/app_shim/immersive_mode_controller.h
View file
Expand Up @@ -10,6 +10,7 @@
#include "base/functional/callback.h"
#include "base/functional/callback_forward.h"
#include "base/mac/scoped_nsobject.h"
#import "components/remote_cocoa/app_shim/bridged_content_view.h"
#include "components/remote_cocoa/app_shim/remote_cocoa_app_shim_export.h"
#include "components/remote_cocoa/common/native_widget_ns_window.mojom-shared.h"

Expand Down Expand Up @@ -61,16 +62,20 @@ class REMOTE_COCOA_APP_SHIM_EXPORT ImmersiveModeController {
virtual void RevealUnlock();
int reveal_lock_count() { return reveal_lock_count_; }

// Called when the NSTitlebarContainerView frame changes.
virtual void OnTitlebarFrameDidChange(NSRect frame);

NSWindow* browser_window() { return browser_window_; }
NSWindow* overlay_window() { return overlay_window_; }
BridgedContentView* overlay_content_view() { return overlay_content_view_; }

// Caled when `immersive_mode_titlebar_view_controller_`'s view is moved to
// a different window.
void ImmersiveModeViewWillMoveToWindow(NSWindow* window);

// When true the titlebar is assumed to be fully visible. For testing only.
void SetTitlebarFullyVisibleForTesting(bool fully_visible) {
titlebar_fully_visible_for_testing_ = fully_visible;
titlebar_fully_visible_ = fully_visible;
}

private:
Expand All @@ -87,6 +92,7 @@ class REMOTE_COCOA_APP_SHIM_EXPORT ImmersiveModeController {

NSWindow* const browser_window_;
NSWindow* const overlay_window_;
BridgedContentView* overlay_content_view_;

// A controller for top chrome.
base::scoped_nsobject<ImmersiveModeTitlebarViewController>
Expand Down Expand Up @@ -124,7 +130,8 @@ class REMOTE_COCOA_APP_SHIM_EXPORT ImmersiveModeController {
mojom::ToolbarVisibilityStyle last_used_style_ =
mojom::ToolbarVisibilityStyle::kAutohide;

bool titlebar_fully_visible_for_testing_ = false;
bool titlebar_fully_visible_ = false;
bool titlebar_frame_change_barrier_ = false;

base::WeakPtrFactory<ImmersiveModeController> weak_ptr_factory_;
};
Expand All @@ -148,7 +155,7 @@ REMOTE_COCOA_APP_SHIM_EXPORT @interface ImmersiveModeTitlebarObserver : NSObject
- (instancetype)initWithController:
(base::WeakPtr<remote_cocoa::ImmersiveModeController>)
controller
overlayView:(NSView*)overlay_view;
titlebarContainerView:(NSView*)titlebarContainerView;

@end

Expand Down
166 changes: 80 additions & 86 deletions components/remote_cocoa/app_shim/immersive_mode_controller.mm
View file
Expand Up @@ -8,7 +8,6 @@
#include "base/check.h"
#include "base/mac/foundation_util.h"
#include "base/mac/scoped_block.h"
#import "components/remote_cocoa/app_shim/bridged_content_view.h"
#import "components/remote_cocoa/app_shim/immersive_mode_delegate_mac.h"
#import "components/remote_cocoa/app_shim/native_widget_mac_nswindow.h"
#import "components/remote_cocoa/app_shim/native_widget_ns_window_bridge.h"
Expand Down Expand Up @@ -41,9 +40,7 @@ void PropagateFrameSizeToViewsSubviews(NSView* view) {

@interface ImmersiveModeTitlebarObserver () {
base::WeakPtr<remote_cocoa::ImmersiveModeController> _controller;
NSView* _overlay_view;
BOOL _barrier;
BOOL _titlebarFullyVisible;
NSView* _titlebarContainerView;
}
@end

Expand All @@ -52,18 +49,22 @@ @implementation ImmersiveModeTitlebarObserver
- (instancetype)initWithController:
(base::WeakPtr<remote_cocoa::ImmersiveModeController>)
controller
overlayView:(NSView*)overlay_view {
titlebarContainerView:(NSView*)titlebarContainerView {
self = [super init];
if (self) {
_controller = std::move(controller);
_overlay_view = overlay_view;
_titlebarContainerView = titlebarContainerView;
[_titlebarContainerView addObserver:self
forKeyPath:@"frame"
options:NSKeyValueObservingOptionInitial |
NSKeyValueObservingOptionNew
context:NULL];
}
return self;
}

- (void)dealloc {
NSView* view = GetNSTitlebarContainerViewFromWindow(_overlay_view.window);
[view removeObserver:self forKeyPath:@"frame"];
[_titlebarContainerView removeObserver:self forKeyPath:@"frame"];
[super dealloc];
}

Expand All @@ -76,59 +77,7 @@ - (void)observeValueForKeyPath:(NSString*)keyPath
}

NSRect frame = [change[@"new"] rectValue];
_titlebarFullyVisible = frame.origin.y == 0;

// Find the overlay view's point on screen (bottom left).
NSPoint point_in_window = [_overlay_view convertPoint:NSZeroPoint toView:nil];
NSPoint point_on_screen =
[_overlay_view.window convertPointToScreen:point_in_window];

BOOL overlay_view_is_clipped = NO;
// This branch is only useful on macOS 11 and greater. macOS 10.15 and
// earlier move the window instead of clipping the view within the window.
// This allows the overlay window to appropriately track the overlay view.
if (@available(macOS 11.0, *)) {
// If the overlay view is clipped move the overlay window off screen. A
// clipped overlay view indicates the titlebar is hidden or is in transition
// AND the browser content view takes up the whole window ("Always Show
// Toolbar in Full Screen" is disabled). When we are in this state we don't
// want the overlay window on screen, otherwise it may mask input to the
// browser view.
// In all other cases will not enter this branch and the overlay
// window will be placed at the same coordinates as the overlay view.
if (_overlay_view.visibleRect.size.height !=
_overlay_view.frame.size.height) {
point_on_screen.y = -_overlay_view.frame.size.height;
overlay_view_is_clipped = YES;
}
}

if (!overlay_view_is_clipped) {
// If there are sub-windows and the titlebar is fully visible (a y origin of
// 0), pin the titlebar. This will prevent the titlebar from autohiding and
// causing the sub-windows from moving up when the mouse leaves top chrome.
if (!_barrier && _titlebarFullyVisible &&
_controller->titlebar_lock_count() > 0) {
// Add a barrier to prevent re-entry, which is a byproduct of
// TitlebarLock() and TitlebarUnlock().
base::AutoReset<BOOL> set_barrier(&_barrier, YES);
// This lock / unlock scheme is to force the titlebar to be pinned in
// place, which can only be done when the titlebar is fully visible.
// Existing sub-windows hold a lock, however since the titlebar isn't
// fully revealed until this point the existing locks don't actually pin
// the titlebar. The existing locks are still important for knowing when
// to unpin the titlebar. When all outstanding locks are released the
// titlebar be unpinned.
_controller->TitlebarLock();
_controller->TitlebarUnlock();
}
}

[_controller->overlay_window() setFrameOrigin:point_on_screen];
}

- (BOOL)titlebarFullyVisible {
return _titlebarFullyVisible;
_controller->OnTitlebarFrameDidChange(frame);
}

@end
Expand Down Expand Up @@ -336,23 +285,16 @@ bool IsNSToolbarFullScreenWindow(NSWindow* window) {

// Remove the content view from the overlay view widget's NSWindow. This
// view will be re-parented into the AppKit created NSWindow.
BridgedContentView* overlay_content_view =
base::mac::ObjCCastStrict<BridgedContentView>(
overlay_window_.contentView);
[overlay_content_view retain];
[overlay_content_view removeFromSuperview];

// Create the titlebar observer. Observing can only start once the view has
// been fully re-parented into the AppKit fullscreen window.
immersive_mode_titlebar_observer_.reset([[ImmersiveModeTitlebarObserver alloc]
initWithController:weak_ptr_factory_.GetWeakPtr()
overlayView:overlay_content_view]);
overlay_content_view_ = base::mac::ObjCCastStrict<BridgedContentView>(
overlay_window_.contentView);
[overlay_content_view_ retain];
[overlay_content_view_ removeFromSuperview];

// The original content view (top chrome) has been moved to the AppKit
// created NSWindow. Create a new content view but reuse the original bridge
// so that mouse drags are handled.
overlay_window_.contentView =
[[[BridgedContentView alloc] initWithBridge:overlay_content_view.bridge
[[[BridgedContentView alloc] initWithBridge:overlay_content_view_.bridge
bounds:gfx::Rect()] autorelease];

// The overlay window will become a child of NSToolbarFullScreenWindow and sit
Expand All @@ -365,8 +307,8 @@ bool IsNSToolbarFullScreenWindow(NSWindow* window) {
// Add the overlay view to the accessory view controller getting ready to
// hand everything over to AppKit.
[immersive_mode_titlebar_view_controller_.get().view
addSubview:overlay_content_view];
[overlay_content_view release];
addSubview:overlay_content_view_];
[overlay_content_view_ release];
immersive_mode_titlebar_view_controller_.get().layoutAttribute =
NSLayoutAttributeBottom;

Expand All @@ -389,10 +331,8 @@ bool IsNSToolbarFullScreenWindow(NSWindow* window) {

// Rollback the view shuffling from enablement.
[thin_titlebar_view_controller_ removeFromParentViewController];
NSView* overlay_content_view =
immersive_mode_titlebar_view_controller_.get().view.subviews.firstObject;
[overlay_content_view removeFromSuperview];
overlay_window_.contentView = overlay_content_view;
[overlay_content_view_ removeFromSuperview];
overlay_window_.contentView = overlay_content_view_;
[immersive_mode_titlebar_view_controller_ removeFromParentViewController];
[immersive_mode_titlebar_view_controller_.get().view release];
immersive_mode_titlebar_view_controller_.reset();
Expand Down Expand Up @@ -556,8 +496,7 @@ bool IsNSToolbarFullScreenWindow(NSWindow* window) {

void ImmersiveModeController::TitlebarLock() {
titlebar_lock_count_++;
if (titlebar_fully_visible_for_testing_ ||
[immersive_mode_titlebar_observer_ titlebarFullyVisible]) {
if (titlebar_fully_visible_) {
SetTitlebarPinned(true);
}
}
Expand Down Expand Up @@ -613,12 +552,67 @@ bool IsNSToolbarFullScreenWindow(NSWindow* window) {

NSView* view = GetNSTitlebarContainerViewFromWindow(window);
DCHECK(view);
[view addObserver:immersive_mode_titlebar_observer_
forKeyPath:@"frame"
options:NSKeyValueObservingOptionInitial |
NSKeyValueObservingOptionNew
context:NULL];
// Create the titlebar observer. Observing can only start once the view has
// been fully re-parented into the AppKit fullscreen window.
immersive_mode_titlebar_observer_.reset(
[[ImmersiveModeTitlebarObserver alloc]
initWithController:weak_ptr_factory_.GetWeakPtr()
titlebarContainerView:view]);
}
}

void ImmersiveModeController::OnTitlebarFrameDidChange(NSRect frame) {
titlebar_fully_visible_ = frame.origin.y == 0;

// Find the overlay view's point on screen (bottom left).
NSPoint point_in_window = [overlay_content_view_ convertPoint:NSZeroPoint
toView:nil];
NSPoint point_on_screen =
[overlay_content_view_.window convertPointToScreen:point_in_window];

BOOL overlay_view_is_clipped = NO;
// This branch is only useful on macOS 11 and greater. macOS 10.15 and
// earlier move the window instead of clipping the view within the window.
// This allows the overlay window to appropriately track the overlay view.
if (@available(macOS 11.0, *)) {
// If the overlay view is clipped move the overlay window off screen. A
// clipped overlay view indicates the titlebar is hidden or is in
// transition AND the browser content view takes up the whole window
// ("Always Show Toolbar in Full Screen" is disabled). When we are in this
// state we don't want the overlay window on screen, otherwise it may mask
// input to the browser view. In all other cases will not enter this
// branch and the overlay window will be placed at the same coordinates as
// the overlay view.
if (overlay_content_view_.visibleRect.size.height !=
overlay_content_view_.frame.size.height) {
point_on_screen.y = -overlay_content_view_.frame.size.height;
overlay_view_is_clipped = YES;
}
}

if (!overlay_view_is_clipped) {
// If there are sub-windows and the titlebar is fully visible (a y origin
// of 0), pin the titlebar. This will prevent the titlebar from autohiding
// and causing the sub-windows from moving up when the mouse leaves top
// chrome.
if (!titlebar_frame_change_barrier_ && titlebar_fully_visible_ &&
titlebar_lock_count() > 0) {
// Add a barrier to prevent re-entry, which is a byproduct of
// TitlebarLock() and TitlebarUnlock().
base::AutoReset<bool> set_barrier(&titlebar_frame_change_barrier_, YES);
// This lock / unlock scheme is to force the titlebar to be pinned in
// place, which can only be done when the titlebar is fully visible.
// Existing sub-windows hold a lock, however since the titlebar isn't
// fully revealed until this point the existing locks don't actually pin
// the titlebar. The existing locks are still important for knowing when
// to unpin the titlebar. When all outstanding locks are released the
// titlebar be unpinned.
TitlebarLock();
TitlebarUnlock();
}
}

[overlay_window_ setFrameOrigin:point_on_screen];
}

} // namespace remote_cocoa
3 changes: 3 additions & 0 deletions components/remote_cocoa/app_shim/immersive_mode_tabbed_controller.h
View file
Expand Up @@ -8,6 +8,7 @@
#include "components/remote_cocoa/app_shim/immersive_mode_controller.h"

#include "base/mac/scoped_nsobject.h"
#import "components/remote_cocoa/app_shim/bridged_content_view.h"

@class TabTitlebarViewController;

Expand All @@ -33,12 +34,14 @@ class REMOTE_COCOA_APP_SHIM_EXPORT ImmersiveModeTabbedController
void RevealUnlock() override;
void TitlebarLock() override;
void TitlebarUnlock() override;
void OnTitlebarFrameDidChange(NSRect frame) override;

private:
void TitlebarReveal();
void TitlebarHide();

NSWindow* const tab_window_;
BridgedContentView* tab_content_view_;
base::scoped_nsobject<TabTitlebarViewController>
tab_titlebar_view_controller_;
};
Expand Down

0 comments on commit ff78008

Please sign in to comment.