Skip to content

Commit

Permalink
Mac immersive: ensure infobar is always visible
Browse files Browse the repository at this point in the history
The infobar lives in a sort of netherworld where
it lives outside of the topchrome, but is UI, not
content. Because of this, when immersive fullscreen
is enabled, revealing the menubar overlaps some or
all of the infobar.

Since the infobar can convey crucial information
(for example, tab screen share), we can't move it
into the autohiding topchrome. Instead, this change
treats it as a third "layer" which lives in the
content view, but slides on menubar reveal to always
be visible below the topchrome.

Bug: 1473068
Change-Id: I444900d60fc70262aac3cce1e96345c5ef6a0cb2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4920111
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Commit-Queue: Leonard Grey <lgrey@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Keren Zhu <kerenzhu@chromium.org>
Reviewed-by: Elly FJ <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1216149}
  • Loading branch information
speednoisemovement authored and Chromium LUCI CQ committed Oct 27, 2023
1 parent b8d45e8 commit e36b92f
Show file tree
Hide file tree
Showing 20 changed files with 360 additions and 197 deletions.
11 changes: 11 additions & 0 deletions chrome/browser/ui/views/frame/browser_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,17 @@ class BrowserViewLayoutDelegateImpl : public BrowserViewLayoutDelegate {
return true;
}

int GetExtraInfobarOffset() const override {
#if BUILDFLAG(IS_MAC)
if (browser_view_->UsesImmersiveFullscreenMode() &&
browser_view_->immersive_mode_controller()->IsEnabled()) {
return browser_view_->immersive_mode_controller()
->GetExtraInfobarOffset();
}
#endif
return 0;
}

private:
raw_ptr<BrowserView> browser_view_;
};
Expand Down
10 changes: 7 additions & 3 deletions chrome/browser/ui/views/frame/browser_view_layout.cc
Original file line number Diff line number Diff line change
Expand Up @@ -652,12 +652,16 @@ int BrowserViewLayout::LayoutInfoBar(int top) {
top = (browser_view_ ? browser_view_->y() : 0) +
immersive_mode_controller_->GetMinimumContentOffset();
}

// The content usually starts at the bottom of the infobar. When there is an
// extra infobar offset the infobar is shifted down while the content stays.
int infobar_top = top;
int content_top = infobar_top + infobar_container_->height();
infobar_top += delegate_->GetExtraInfobarOffset();
SetViewVisibility(infobar_container_, IsInfobarVisible());
infobar_container_->SetBounds(
vertical_layout_rect_.x(), top, vertical_layout_rect_.width(),
vertical_layout_rect_.x(), infobar_top, vertical_layout_rect_.width(),
infobar_container_->GetPreferredSize().height());
return top + infobar_container_->height();
return content_top;
}

void BrowserViewLayout::LayoutContentsContainerView(int top, int bottom) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class BrowserViewLayoutDelegate {
virtual void UpdateWindowControlsOverlay(
const gfx::Rect& available_titlebar_area) const = 0;
virtual bool ShouldLayoutTabStrip() const = 0;
virtual int GetExtraInfobarOffset() const = 0;
};

#endif // CHROME_BROWSER_UI_VIEWS_FRAME_BROWSER_VIEW_LAYOUT_DELEGATE_H_
2 changes: 2 additions & 0 deletions chrome/browser/ui/views/frame/browser_view_layout_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ class MockBrowserViewLayoutDelegate : public BrowserViewLayoutDelegate {
bool IsWindowControlsOverlayEnabled() const override { return false; }
void UpdateWindowControlsOverlay(const gfx::Rect& rect) const override {}
bool ShouldLayoutTabStrip() const override { return true; }
int GetExtraInfobarOffset() const override { return 0; }

private:
bool should_draw_tab_strip_ = true;
Expand Down Expand Up @@ -151,6 +152,7 @@ class MockImmersiveModeController : public ImmersiveModeController {
bool ShouldStayImmersiveAfterExitingFullscreen() override { return true; }
void OnWidgetActivationChanged(views::Widget* widget, bool active) override {}
int GetMinimumContentOffset() const override { return 0; }
int GetExtraInfobarOffset() const override { return 0; }
};

} // anonymous namespace
Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/ui/views/frame/immersive_mode_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,11 @@ class ImmersiveModeController {
// find results from hiding under the top chrome when the find bar is in use.
virtual int GetMinimumContentOffset() const = 0;

// Returns an offset to add to the vertical origin of the infobar while
// laying out the browser view. Used on Mac to ensure the infobar stays
// visible when revealing topchrome.
virtual int GetExtraInfobarOffset() const = 0;

virtual void AddObserver(Observer* observer);
virtual void RemoveObserver(Observer* observer);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,10 @@ int ImmersiveModeControllerChromeos::GetMinimumContentOffset() const {
return 0;
}

int ImmersiveModeControllerChromeos::GetExtraInfobarOffset() const {
return 0;
}

void ImmersiveModeControllerChromeos::LayoutBrowserRootView() {
views::Widget* widget = browser_view_->frame();
// Update the window caption buttons.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ class ImmersiveModeControllerChromeos
bool ShouldStayImmersiveAfterExitingFullscreen() override;
void OnWidgetActivationChanged(views::Widget* widget, bool active) override;
int GetMinimumContentOffset() const override;
int GetExtraInfobarOffset() const override;

private:
// Updates the browser root view's layout including window caption controls.
Expand Down
162 changes: 160 additions & 2 deletions chrome/browser/ui/views/frame/immersive_mode_controller_mac.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,168 @@

#include <memory>

class BrowserView;
class ImmersiveModeController;
#include "base/memory/raw_ptr.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/frame/immersive_mode_controller.h"
#include "components/remote_cocoa/common/native_widget_ns_window.mojom.h"
#include "ui/views/cocoa/immersive_mode_reveal_client.h"
#include "ui/views/focus/focus_manager.h"
#include "ui/views/view_observer.h"
#include "ui/views/widget/widget.h"
#include "ui/views/widget/widget_observer.h"

std::unique_ptr<ImmersiveModeController> CreateImmersiveModeControllerMac(
const BrowserView* browser_view);

class ImmersiveModeControllerMac;

// This class notifies the browser view to refresh layout whenever the overlay
// widget moves. This is necessary for positioning web dialogs.
class ImmersiveModeOverlayWidgetObserver : public views::WidgetObserver {
public:
explicit ImmersiveModeOverlayWidgetObserver(
ImmersiveModeControllerMac* controller);

ImmersiveModeOverlayWidgetObserver(
const ImmersiveModeOverlayWidgetObserver&) = delete;
ImmersiveModeOverlayWidgetObserver& operator=(
const ImmersiveModeOverlayWidgetObserver&) = delete;
~ImmersiveModeOverlayWidgetObserver() override;

// views::WidgetObserver:
void OnWidgetBoundsChanged(views::Widget* widget,
const gfx::Rect& new_bounds) override;

private:
raw_ptr<ImmersiveModeControllerMac> controller_;
};

class ImmersiveModeControllerMac : public ImmersiveModeController,
public views::FocusChangeListener,
public views::ViewObserver,
public views::WidgetObserver,
public views::FocusTraversable,
public views::ImmersiveModeRevealClient {
public:
class RevealedLock : public ImmersiveRevealedLock {
public:
explicit RevealedLock(base::WeakPtr<ImmersiveModeControllerMac> controller);

RevealedLock(const RevealedLock&) = delete;
RevealedLock& operator=(const RevealedLock&) = delete;

~RevealedLock() override;

private:
base::WeakPtr<ImmersiveModeControllerMac> controller_;
};

// If `separate_tab_strip` is true, the tab strip is split out into its own
// widget separate from the overlay view so that it can live in the title bar.
explicit ImmersiveModeControllerMac(bool separate_tab_strip);

ImmersiveModeControllerMac(const ImmersiveModeControllerMac&) = delete;
ImmersiveModeControllerMac& operator=(const ImmersiveModeControllerMac&) =
delete;

~ImmersiveModeControllerMac() override;

// ImmersiveModeController overrides:
void Init(BrowserView* browser_view) override;
void SetEnabled(bool enabled) override;
bool IsEnabled() const override;
bool ShouldHideTopViews() const override;
bool IsRevealed() const override;
int GetTopContainerVerticalOffset(
const gfx::Size& top_container_size) const override;
std::unique_ptr<ImmersiveRevealedLock> GetRevealedLock(
AnimateReveal animate_reveal) override;
void OnFindBarVisibleBoundsChanged(
const gfx::Rect& new_visible_bounds_in_screen) override;
bool ShouldStayImmersiveAfterExitingFullscreen() override;
void OnWidgetActivationChanged(views::Widget* widget, bool active) override;
int GetMinimumContentOffset() const override;
int GetExtraInfobarOffset() const override;

// Set the widget id of the tab hosting widget. Set before calling SetEnabled.
void SetTabNativeWidgetID(uint64_t widget_id);

// views::FocusChangeListener implementation.
void OnWillChangeFocus(views::View* focused_before,
views::View* focused_now) override;
void OnDidChangeFocus(views::View* focused_before,
views::View* focused_now) override;

// views::ViewObserver implementation
void OnViewBoundsChanged(views::View* observed_view) override;

// views::WidgetObserver implementation
void OnWidgetDestroying(views::Widget* widget) override;

// views::Traversable:
views::FocusSearch* GetFocusSearch() override;
views::FocusTraversable* GetFocusTraversableParent() override;
views::View* GetFocusTraversableParentView() override;

// views::ImmersiveModeRevealClient:
void OnImmersiveModeToolbarRevealChanged(bool is_revealed) override;
void OnImmersiveModeMenuBarRevealChanged(float reveal_amount) override;
void OnAutohidingMenuBarHeightChanged(int menu_bar_height) override;

BrowserView* browser_view() { return browser_view_; }

private:
friend class RevealedLock;

void LockDestroyed();

// Move children from `from_widget` to `to_widget`. Certain child widgets will
// be held back from the move, see `ShouldMoveChild` for details.
void MoveChildren(views::Widget* from_widget, views::Widget* to_widget);

// Returns true if the child should be moved.
bool ShouldMoveChild(views::Widget* child);

raw_ptr<BrowserView> browser_view_ = nullptr; // weak
std::unique_ptr<ImmersiveRevealedLock> focus_lock_;
bool enabled_ = false;
base::ScopedObservation<views::View, views::ViewObserver>
top_container_observation_{this};
base::ScopedObservation<views::Widget, views::WidgetObserver>
browser_frame_observation_{this};
ImmersiveModeOverlayWidgetObserver overlay_widget_observer_{this};
base::ScopedObservation<views::Widget, views::WidgetObserver>
overlay_widget_observation_{&overlay_widget_observer_};
remote_cocoa::mojom::NativeWidgetNSWindow* GetNSWindowMojo();
std::unique_ptr<views::FocusSearch> focus_search_;

// Used to hold the widget id for the tab hosting widget. This will be passed
// to the remote_cocoa immersive mode controller where the tab strip will be
// placed in the titlebar.
uint64_t tab_native_widget_id_ = 0;

// Whether the tab strip should be a separate widget.
bool separate_tab_strip_ = false;
// Height of the tab widget, used when resizing. Only non-zero if
// `separate_tab_strip_` is true.
int tab_widget_height_ = 0;
// Total height of the overlay (including the separate tab strip if relevant).
int overlay_height_ = 0;
// Whether the find bar is currently visible.
bool find_bar_visible_ = false;
// Whether the toolbar is currently visible.
bool is_revealed_ = false;
// The proportion of the menubar/topchrome that has been revealed as a result
// of the user mousing to the top of the screen.
float reveal_amount_ = 0;
// The height of the menubar, if the menubar should be accounted for when
// compensating for reveal animations, otherwise 0. Situations where it is
// not accounted for include screens with notches (where there is always
// space reserved for it) and the "Always Show Menu Bar" system setting.
int menu_bar_height_ = 0;

base::WeakPtrFactory<ImmersiveModeControllerMac> weak_ptr_factory_;
};

#endif // CHROME_BROWSER_UI_VIEWS_FRAME_IMMERSIVE_MODE_CONTROLLER_MAC_H_

0 comments on commit e36b92f

Please sign in to comment.