Skip to content

Commit

Permalink
Simplified active web contents changes handling
Browse files Browse the repository at this point in the history
  • Loading branch information
simonhong committed Dec 12, 2024
1 parent b925802 commit f11acb4
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 61 deletions.
4 changes: 2 additions & 2 deletions browser/ui/views/frame/brave_browser_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -843,8 +843,8 @@ void BraveBrowserView::OnActiveTabChanged(content::WebContents* old_contents,
int reason) {
SplitView::AfterSetWebContents after_set_callback;
if (split_view_) {
after_set_callback = split_view_->WillSetActiveWebContentsToContentsWebView(
/*passkey*/ {}, new_contents, index);
after_set_callback = split_view_->WillChangeActiveWebContents(
/*passkey*/ {}, old_contents, new_contents);
}

BrowserView::OnActiveTabChanged(old_contents, new_contents, index, reason);
Expand Down
99 changes: 46 additions & 53 deletions browser/ui/views/split_view/split_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -95,54 +95,32 @@ SplitView::SplitView(Browser& browser,

SplitView::~SplitView() = default;

SplitView::AfterSetWebContents
SplitView::WillSetActiveWebContentsToContentsWebView(
SplitView::AfterSetWebContents SplitView::WillChangeActiveWebContents(
BrowserViewKey,
content::WebContents* new_contents,
int index) {
bool need_to_update_secondary_web_view = false;
// In order to minimize flickering during tab activation, we should update
// split view only when it's needed.
auto* browser_data =
SplitViewBrowserData::FromBrowser(base::to_address(browser_));
auto* tab_strip_model = browser_->tab_strip_model();
if (auto tile =
browser_data->GetTile(tab_strip_model->GetTabHandleAt(index))) {
auto* main_web_contents = tab_strip_model->GetWebContentsAt(
tab_strip_model->GetIndexOfTab(tile->first));
auto* secondary_web_contents = tab_strip_model->GetWebContentsAt(
tab_strip_model->GetIndexOfTab(tile->second));
if (main_web_contents != new_contents) {
std::swap(main_web_contents, secondary_web_contents);
}

need_to_update_secondary_web_view =
contents_web_view_->web_contents() != main_web_contents ||
secondary_contents_web_view_->web_contents() != secondary_web_contents;
} else {
// Old contents was in a split view. We should hide split view.
need_to_update_secondary_web_view =
secondary_contents_web_view_->web_contents();
content::WebContents* old_contents,
content::WebContents* new_contents) {
// Early return with null callback if this active state changes is not related
// with split view. |secondary_contents_container_| is not visible if previous
// active contents is not in tile.
if (!secondary_contents_container_->GetVisible() &&
!IsWebContentsTiled(new_contents)) {
return base::NullCallback();
}

if (need_to_update_secondary_web_view) {
// This helps reduce flickering when switching between tiled tabs.
contents_web_view_->SetFastResize(true);
secondary_contents_web_view_->SetFastResize(true);

if (!SplitViewBrowserData::FromBrowser(base::to_address(browser_))
->GetTile(browser_->tab_strip_model()->GetTabHandleAt(index))) {
// This will help reduce flickering when switching to non tiled tab by
// hiding secondary web view before detaching web contents.
UpdateSecondaryContentsWebViewVisibility();
}
// This helps reduce flickering when switching between tiled tabs.
contents_web_view_->SetFastResize(true);
secondary_contents_web_view_->SetFastResize(true);

secondary_contents_web_view_->SetWebContents(nullptr);
if (!IsWebContentsTiled(new_contents)) {
// This will help reduce flickering when switching to non tiled tab by
// hiding secondary web view before detaching web contents.
UpdateSecondaryContentsWebViewVisibility();
}

return base::BindOnce(&SplitView::DidSwapActiveWebContents,
weak_ptr_factory_.GetWeakPtr(),
need_to_update_secondary_web_view);
secondary_contents_web_view_->SetWebContents(nullptr);

return base::BindOnce(&SplitView::DidChangeActiveWebContents,
weak_ptr_factory_.GetWeakPtr());
}

void SplitView::WillUpdateDevToolsForActiveContents(BrowserViewKey) {
Expand All @@ -152,28 +130,28 @@ void SplitView::WillUpdateDevToolsForActiveContents(BrowserViewKey) {
}

void SplitView::DidUpdateDevToolsForActiveContents(BrowserViewKey) {
if (secondary_contents_web_view_->GetVisible()) {
if (secondary_contents_container_->GetVisible()) {
UpdateSecondaryDevtoolsLayoutAndVisibility();
}
}

void SplitView::DidSwapActiveWebContents(bool need_to_reset_fast_resize,
content::WebContents* old_contents,
content::WebContents* new_contents) {
void SplitView::DidChangeActiveWebContents(content::WebContents* old_contents,
content::WebContents* new_contents) {
UpdateSplitViewSizeDelta(old_contents, new_contents);

UpdateContentsWebViewVisual();

if (need_to_reset_fast_resize) {
// Revert back to default state.
contents_web_view_->SetFastResize(false);
secondary_contents_web_view_->SetFastResize(false);
InvalidateLayout();
}
// Revert back to default state.
contents_web_view_->SetFastResize(false);
secondary_contents_web_view_->SetFastResize(false);
InvalidateLayout();
}

void SplitView::GetAccessiblePanes(BrowserViewKey,
std::vector<views::View*>* panes) {
if (!secondary_contents_container_->GetVisible()) {
return;
}

if (secondary_contents_web_view_ &&
secondary_contents_web_view_->GetVisible()) {
panes->push_back(secondary_contents_web_view_);
Expand Down Expand Up @@ -217,6 +195,9 @@ void SplitView::AddedToWidget() {
secondary_location_bar_widget_->Init(
SplitViewLocationBar::GetWidgetInitParams(GetWidget()->GetNativeView(),
secondary_location_bar_.get()));

// Initialize secondary view state.
UpdateSecondaryContentsWebViewVisibility();
}

void SplitView::OnTileTabs(const TabTile& tile) {
Expand Down Expand Up @@ -258,6 +239,18 @@ bool SplitView::IsActiveWebContentsTiled(const TabTile& tile) const {
return tile.first == active_tab_handle || tile.second == active_tab_handle;
}

bool SplitView::IsWebContentsTiled(content::WebContents* contents) const {
const int tab_index =
browser_->tab_strip_model()->GetIndexOfWebContents(contents);
if (tab_index == TabStripModel::kNoTab) {
return false;
}
const auto tab_handle =
browser_->tab_strip_model()->GetTabHandleAt(tab_index);
return SplitViewBrowserData::FromBrowser(base::to_address(browser_))
->IsTabTiled(tab_handle);
}

void SplitView::UpdateSplitViewSizeDelta(content::WebContents* old_contents,
content::WebContents* new_contents) {
auto get_index_of = [this](content::WebContents* contents) {
Expand Down
12 changes: 6 additions & 6 deletions browser/ui/views/split_view/split_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ class SplitView : public views::View, public SplitViewBrowserDataObserver {
using AfterSetWebContents =
base::OnceCallback<void(content::WebContents* old_contents,
content::WebContents* new_contents)>;
[[nodiscard]] AfterSetWebContents WillSetActiveWebContentsToContentsWebView(
[[nodiscard]] AfterSetWebContents WillChangeActiveWebContents(
BrowserViewKey,
content::WebContents* new_contents,
int index);
content::WebContents* old_contents,
content::WebContents* new_contents);

// Called before/after BrowseView::UpdateDevToolsForContents().
void WillUpdateDevToolsForActiveContents(BrowserViewKey);
Expand Down Expand Up @@ -102,12 +102,12 @@ class SplitView : public views::View, public SplitViewBrowserDataObserver {
friend class SplitViewBrowserTest;
friend class SplitViewLocationBarBrowserTest;

void DidSwapActiveWebContents(bool need_to_reset_fast_resize,
content::WebContents* old_contents,
content::WebContents* new_contents);
void DidChangeActiveWebContents(content::WebContents* old_contents,
content::WebContents* new_contents);

tabs::TabHandle GetActiveTabHandle() const;
bool IsActiveWebContentsTiled(const TabTile& tile) const;
bool IsWebContentsTiled(content::WebContents* contents) const;
void UpdateSplitViewSizeDelta(content::WebContents* old_contents,
content::WebContents* new_contents);
void UpdateContentsWebViewVisual();
Expand Down
1 change: 1 addition & 0 deletions browser/ui/views/split_view/split_view_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ IN_PROC_BROWSER_TEST_F(SplitViewBrowserTest,

IN_PROC_BROWSER_TEST_F(SplitViewBrowserTest,
GetAccessiblePaneContainsSecondaryViews) {
secondary_contents_container().SetVisible(true);
secondary_contents_view().SetVisible(true);
secondary_dev_tools().SetVisible(true);
std::vector<views::View*> panes;
Expand Down

0 comments on commit f11acb4

Please sign in to comment.