Commit 3ab83c88 authored by Robbie Gibson's avatar Robbie Gibson Committed by Chromium LUCI CQ

[iOS][ThumbStrip] Set BVC position on new tabs if using smooth scrolling

Now, instead of having the BVC manually set the contentInset, it
instead alerts the fullscreen controller that the thumb strip has
appeared. The fullscreen controller can then use this to correctly set
the insets itself, even when new tabs are added.

Also, the BVC now keeps track of whether the thumb strip has caused
the view to be translated, so it can re-apply this translation when
adding a new web state.

Fixed: 1155160
Change-Id: I827d9edf058667a535275367605518512ebca7e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2599866Reviewed-by: default avatarMark Cogan <marq@chromium.org>
Reviewed-by: default avatarChris Lu <thegreenfrog@chromium.org>
Commit-Queue: Robbie Gibson <rkgibson@google.com>
Cr-Commit-Position: refs/heads/master@{#841461}
parent b5ab8f49
...@@ -606,6 +606,11 @@ NSString* const kBrowserViewControllerSnackbarCategory = ...@@ -606,6 +606,11 @@ NSString* const kBrowserViewControllerSnackbarCategory =
@property(nonatomic, strong) @property(nonatomic, strong)
BrowserViewHiderCoordinator* browserViewHiderCoordinator; BrowserViewHiderCoordinator* browserViewHiderCoordinator;
// Whether the view has been translated for thumb strip usage when smooth
// scrolling has been enabled. This allows the correct setup to be done when
// displaying a new web state.
@property(nonatomic, assign) BOOL viewTranslatedForSmoothScrolling;
// BVC initialization // BVC initialization
// ------------------ // ------------------
// If the BVC is initialized with a valid browser state & tab model immediately, // If the BVC is initialized with a valid browser state & tab model immediately,
...@@ -2408,7 +2413,15 @@ NSString* const kBrowserViewControllerSnackbarCategory = ...@@ -2408,7 +2413,15 @@ NSString* const kBrowserViewControllerSnackbarCategory =
// Make new content visible, resizing it first as the orientation may // Make new content visible, resizing it first as the orientation may
// have changed from the last time it was displayed. // have changed from the last time it was displayed.
CGRect webStateViewFrame = self.contentArea.bounds; CGRect webStateViewFrame = self.contentArea.bounds;
if (!fullscreen::features::ShouldUseSmoothScrolling()) { if (fullscreen::features::ShouldUseSmoothScrolling()) {
// If the view was translated for the thumb strip, make sure to re-apply
// that translation here.
if (self.viewTranslatedForSmoothScrolling) {
CGFloat toolbarHeight = [self expandedTopToolbarHeight];
webStateViewFrame = UIEdgeInsetsInsetRect(
webStateViewFrame, UIEdgeInsetsMake(toolbarHeight, 0, 0, 0));
}
} else {
// If the Smooth Scrolling is on, the WebState view is not // If the Smooth Scrolling is on, the WebState view is not
// resized, and should always match the bounds of the content area. When // resized, and should always match the bounds of the content area. When
// the provider is not initialized, viewport insets resize the webview, so // the provider is not initialized, viewport insets resize the webview, so
...@@ -2908,25 +2921,32 @@ NSString* const kBrowserViewControllerSnackbarCategory = ...@@ -2908,25 +2921,32 @@ NSString* const kBrowserViewControllerSnackbarCategory =
// frame must be moved down and the content inset is decreased. To prevent // frame must be moved down and the content inset is decreased. To prevent
// the actual web content from jumping, the content offset must be moved up // the actual web content from jumping, the content offset must be moved up
// by a corresponding amount. // by a corresponding amount.
if (self.currentWebState && ![self isNTPActiveForCurrentWebState] && if (fullscreen::features::ShouldUseSmoothScrolling()) {
fullscreen::features::ShouldUseSmoothScrolling()) { self.viewTranslatedForSmoothScrolling = YES;
CGFloat toolbarHeight = [self expandedTopToolbarHeight]; CGFloat toolbarHeight = [self expandedTopToolbarHeight];
CGRect webStateViewFrame = UIEdgeInsetsInsetRect( if (self.currentWebState) {
[self viewForWebState:self.currentWebState].frame, CGRect webStateViewFrame = UIEdgeInsetsInsetRect(
UIEdgeInsetsMake(toolbarHeight, 0, 0, 0)); [self viewForWebState:self.currentWebState].frame,
[self viewForWebState:self.currentWebState].frame = webStateViewFrame; UIEdgeInsetsMake(toolbarHeight, 0, 0, 0));
[self viewForWebState:self.currentWebState].frame = webStateViewFrame;
CRWWebViewScrollViewProxy* scrollProxy = }
self.currentWebState->GetWebViewProxy().scrollViewProxy;
CGPoint scrollOffset = scrollProxy.contentOffset; // Translate all web states' offset so web states from other tabs are also
scrollOffset.y += toolbarHeight; // updated.
scrollProxy.contentOffset = scrollOffset; if (self.browser) {
WebStateList* webStateList = self.browser->GetWebStateList();
// TODO(crbug.com/1155536): Inform FullscreenController about these for (int index = 0; index < webStateList->count(); ++index) {
// changes and allow it to calculate the correct overall contentInset. web::WebState* webState = webStateList->GetWebStateAt(index);
UIEdgeInsets contentInset = scrollProxy.contentInset; CRWWebViewScrollViewProxy* scrollProxy =
contentInset.top -= toolbarHeight; webState->GetWebViewProxy().scrollViewProxy;
scrollProxy.contentInset = contentInset; CGPoint scrollOffset = scrollProxy.contentOffset;
scrollOffset.y += toolbarHeight;
scrollProxy.contentOffset = scrollOffset;
}
}
// This alerts the fullscreen controller to use the correct new content
// insets.
self.fullscreenController->FreezeToolbarHeight(true);
} }
} }
} }
...@@ -2973,25 +2993,31 @@ NSString* const kBrowserViewControllerSnackbarCategory = ...@@ -2973,25 +2993,31 @@ NSString* const kBrowserViewControllerSnackbarCategory =
[self installFakeStatusBar]; [self installFakeStatusBar];
[self setupStatusBarLayout]; [self setupStatusBarLayout];
// See the comments in |-willAnimateViewReveal:| for the explantation of why // See the comments in |-willAnimateViewReveal:| for the explanation of why
// this is necessary. // this is necessary.
if (self.currentWebState && ![self isNTPActiveForCurrentWebState] && if (fullscreen::features::ShouldUseSmoothScrolling()) {
fullscreen::features::ShouldUseSmoothScrolling()) { self.viewTranslatedForSmoothScrolling = NO;
self.fullscreenController->FreezeToolbarHeight(false);
CGFloat toolbarHeight = [self expandedTopToolbarHeight]; CGFloat toolbarHeight = [self expandedTopToolbarHeight];
CGRect webStateViewFrame = UIEdgeInsetsInsetRect( if (self.currentWebState) {
[self viewForWebState:self.currentWebState].frame, CGRect webStateViewFrame = UIEdgeInsetsInsetRect(
UIEdgeInsetsMake(-toolbarHeight, 0, 0, 0)); [self viewForWebState:self.currentWebState].frame,
[self viewForWebState:self.currentWebState].frame = webStateViewFrame; UIEdgeInsetsMake(-toolbarHeight, 0, 0, 0));
[self viewForWebState:self.currentWebState].frame = webStateViewFrame;
CRWWebViewScrollViewProxy* scrollProxy = }
self.currentWebState->GetWebViewProxy().scrollViewProxy;
UIEdgeInsets contentInset = scrollProxy.contentInset; if (self.browser) {
contentInset.top += toolbarHeight; WebStateList* webStateList = self.browser->GetWebStateList();
scrollProxy.contentInset = contentInset; for (int index = 0; index < webStateList->count(); ++index) {
web::WebState* webState = webStateList->GetWebStateAt(index);
CGPoint scrollOffset = scrollProxy.contentOffset; CRWWebViewScrollViewProxy* scrollProxy =
scrollOffset.y -= toolbarHeight; webState->GetWebViewProxy().scrollViewProxy;
scrollProxy.contentOffset = scrollOffset;
CGPoint scrollOffset = scrollProxy.contentOffset;
scrollOffset.y -= toolbarHeight;
scrollProxy.contentOffset = scrollOffset;
}
}
} }
} }
} }
......
...@@ -87,6 +87,8 @@ class FullscreenController : public base::SupportsUserData::Data { ...@@ -87,6 +87,8 @@ class FullscreenController : public base::SupportsUserData::Data {
// 1.0. // 1.0.
virtual void ExitFullscreen() = 0; virtual void ExitFullscreen() = 0;
virtual void FreezeToolbarHeight(bool freeze_toolbar_height) = 0;
// Force horizontal content resize, when content isn't tracking resize by // Force horizontal content resize, when content isn't tracking resize by
// itself. // itself.
virtual void ResizeHorizontalViewport() = 0; virtual void ResizeHorizontalViewport() = 0;
......
...@@ -48,6 +48,7 @@ class FullscreenControllerImpl : public FullscreenController { ...@@ -48,6 +48,7 @@ class FullscreenControllerImpl : public FullscreenController {
void EnterFullscreen() override; void EnterFullscreen() override;
void ExitFullscreen() override; void ExitFullscreen() override;
void ResizeHorizontalViewport() override; void ResizeHorizontalViewport() override;
void FreezeToolbarHeight(bool freeze_toolbar_height) override;
private: private:
......
...@@ -177,3 +177,7 @@ void FullscreenControllerImpl::ResizeHorizontalViewport() { ...@@ -177,3 +177,7 @@ void FullscreenControllerImpl::ResizeHorizontalViewport() {
// two relayouts. // two relayouts.
mediator_.ResizeHorizontalInsets(); mediator_.ResizeHorizontalInsets();
} }
void FullscreenControllerImpl::FreezeToolbarHeight(bool freeze_toolbar_height) {
model_.SetFreezeToolbarHeight(freeze_toolbar_height);
}
...@@ -46,7 +46,7 @@ class FullscreenModel : public ChromeBroadcastObserverInterface { ...@@ -46,7 +46,7 @@ class FullscreenModel : public ChromeBroadcastObserverInterface {
// Returns the difference between the max and min toolbar heights. // Returns the difference between the max and min toolbar heights.
CGFloat toolbar_height_delta() const { CGFloat toolbar_height_delta() const {
return expanded_toolbar_height_ - collapsed_toolbar_height_; return GetExpandedToolbarHeight() - GetCollapsedToolbarHeight();
} }
// Returns whether the page content is tall enough for the toolbar to be // Returns whether the page content is tall enough for the toolbar to be
...@@ -57,7 +57,7 @@ class FullscreenModel : public ChromeBroadcastObserverInterface { ...@@ -57,7 +57,7 @@ class FullscreenModel : public ChromeBroadcastObserverInterface {
// Whether the view is scrolled all the way to the top. // Whether the view is scrolled all the way to the top.
bool is_scrolled_to_top() const { bool is_scrolled_to_top() const {
return y_content_offset_ <= -expanded_toolbar_height_; return y_content_offset_ <= -GetExpandedToolbarHeight();
} }
// Whether the view is scrolled all the way to the bottom. // Whether the view is scrolled all the way to the bottom.
...@@ -79,8 +79,8 @@ class FullscreenModel : public ChromeBroadcastObserverInterface { ...@@ -79,8 +79,8 @@ class FullscreenModel : public ChromeBroadcastObserverInterface {
// Returns the toolbar insets at |progress|. // Returns the toolbar insets at |progress|.
UIEdgeInsets GetToolbarInsetsAtProgress(CGFloat progress) const { UIEdgeInsets GetToolbarInsetsAtProgress(CGFloat progress) const {
return UIEdgeInsetsMake( return UIEdgeInsetsMake(
collapsed_toolbar_height_ + GetCollapsedToolbarHeight() + progress * (GetExpandedToolbarHeight() -
progress * (expanded_toolbar_height_ - collapsed_toolbar_height_), GetCollapsedToolbarHeight()),
0, progress * bottom_toolbar_height_, 0); 0, progress * bottom_toolbar_height_, 0);
} }
...@@ -157,6 +157,9 @@ class FullscreenModel : public ChromeBroadcastObserverInterface { ...@@ -157,6 +157,9 @@ class FullscreenModel : public ChromeBroadcastObserverInterface {
void SetWebViewSafeAreaInsets(UIEdgeInsets safe_area_insets); void SetWebViewSafeAreaInsets(UIEdgeInsets safe_area_insets);
UIEdgeInsets GetWebViewSafeAreaInsets() const; UIEdgeInsets GetWebViewSafeAreaInsets() const;
void SetFreezeToolbarHeight(bool freeze_toolbar_height);
bool GetFreezeToolbarHeight() const;
private: private:
// Returns how a scroll to the current |y_content_offset_| from |from_offset| // Returns how a scroll to the current |y_content_offset_| from |from_offset|
// should be handled. // should be handled.
...@@ -234,6 +237,7 @@ class FullscreenModel : public ChromeBroadcastObserverInterface { ...@@ -234,6 +237,7 @@ class FullscreenModel : public ChromeBroadcastObserverInterface {
UIEdgeInsets safe_area_insets_ = UIEdgeInsetsZero; UIEdgeInsets safe_area_insets_ = UIEdgeInsetsZero;
// The number of FullscreenModelObserver callbacks currently being executed. // The number of FullscreenModelObserver callbacks currently being executed.
size_t observer_callback_count_ = 0; size_t observer_callback_count_ = 0;
bool freeze_toolbar_height_ = false;
DISALLOW_COPY_AND_ASSIGN(FullscreenModel); DISALLOW_COPY_AND_ASSIGN(FullscreenModel);
}; };
......
...@@ -79,7 +79,7 @@ void FullscreenModel::AnimationEndedWithProgress(CGFloat progress) { ...@@ -79,7 +79,7 @@ void FullscreenModel::AnimationEndedWithProgress(CGFloat progress) {
} }
void FullscreenModel::SetCollapsedToolbarHeight(CGFloat height) { void FullscreenModel::SetCollapsedToolbarHeight(CGFloat height) {
if (AreCGFloatsEqual(collapsed_toolbar_height_, height)) if (AreCGFloatsEqual(GetCollapsedToolbarHeight(), height))
return; return;
DCHECK_GE(height, 0.0); DCHECK_GE(height, 0.0);
collapsed_toolbar_height_ = height; collapsed_toolbar_height_ = height;
...@@ -91,11 +91,11 @@ void FullscreenModel::SetCollapsedToolbarHeight(CGFloat height) { ...@@ -91,11 +91,11 @@ void FullscreenModel::SetCollapsedToolbarHeight(CGFloat height) {
} }
CGFloat FullscreenModel::GetCollapsedToolbarHeight() const { CGFloat FullscreenModel::GetCollapsedToolbarHeight() const {
return collapsed_toolbar_height_; return GetFreezeToolbarHeight() ? 0 : collapsed_toolbar_height_;
} }
void FullscreenModel::SetExpandedToolbarHeight(CGFloat height) { void FullscreenModel::SetExpandedToolbarHeight(CGFloat height) {
if (AreCGFloatsEqual(expanded_toolbar_height_, height)) if (AreCGFloatsEqual(GetExpandedToolbarHeight(), height))
return; return;
DCHECK_GE(height, 0.0); DCHECK_GE(height, 0.0);
expanded_toolbar_height_ = height; expanded_toolbar_height_ = height;
...@@ -107,7 +107,7 @@ void FullscreenModel::SetExpandedToolbarHeight(CGFloat height) { ...@@ -107,7 +107,7 @@ void FullscreenModel::SetExpandedToolbarHeight(CGFloat height) {
} }
CGFloat FullscreenModel::GetExpandedToolbarHeight() const { CGFloat FullscreenModel::GetExpandedToolbarHeight() const {
return expanded_toolbar_height_; return GetFreezeToolbarHeight() ? 0 : expanded_toolbar_height_;
} }
void FullscreenModel::SetBottomToolbarHeight(CGFloat height) { void FullscreenModel::SetBottomToolbarHeight(CGFloat height) {
...@@ -247,6 +247,22 @@ UIEdgeInsets FullscreenModel::GetWebViewSafeAreaInsets() const { ...@@ -247,6 +247,22 @@ UIEdgeInsets FullscreenModel::GetWebViewSafeAreaInsets() const {
return safe_area_insets_; return safe_area_insets_;
} }
void FullscreenModel::SetFreezeToolbarHeight(bool freeze_toolbar_height) {
if (freeze_toolbar_height_ == freeze_toolbar_height) {
return;
}
freeze_toolbar_height_ = freeze_toolbar_height;
base_offset_ = NAN;
ScopedIncrementer toolbar_height_incrementer(&observer_callback_count_);
for (auto& observer : observers_) {
observer.FullscreenModelToolbarHeightsUpdated(this);
}
}
bool FullscreenModel::GetFreezeToolbarHeight() const {
return freeze_toolbar_height_;
}
FullscreenModel::ScrollAction FullscreenModel::ActionForScrollFromOffset( FullscreenModel::ScrollAction FullscreenModel::ActionForScrollFromOffset(
CGFloat from_offset) const { CGFloat from_offset) const {
// Update the base offset but don't recalculate progress if: // Update the base offset but don't recalculate progress if:
...@@ -309,7 +325,7 @@ void FullscreenModel::UpdateDisabledCounterForContentHeight() { ...@@ -309,7 +325,7 @@ void FullscreenModel::UpdateDisabledCounterForContentHeight() {
// When Smooth Scrolling is disabled, the scroll view can sometimes be // When Smooth Scrolling is disabled, the scroll view can sometimes be
// resized to account for the viewport insets after the page has been // resized to account for the viewport insets after the page has been
// rendered, so account for the maximum toolbar insets in the threshold. // rendered, so account for the maximum toolbar insets in the threshold.
disabling_threshold += expanded_toolbar_height_ + bottom_toolbar_height_; disabling_threshold += GetExpandedToolbarHeight() + bottom_toolbar_height_;
} else { } else {
// After reloads, pages whose viewports fit the screen are sometimes resized // After reloads, pages whose viewports fit the screen are sometimes resized
// to account for the safe area insets. Adding these to the threshold helps // to account for the safe area insets. Adding these to the threshold helps
......
...@@ -314,3 +314,16 @@ TEST_F(FullscreenModelTest, ScrolledToTopAndBottom) { ...@@ -314,3 +314,16 @@ TEST_F(FullscreenModelTest, ScrolledToTopAndBottom) {
EXPECT_FALSE(model().is_scrolled_to_top()); EXPECT_FALSE(model().is_scrolled_to_top());
EXPECT_TRUE(model().is_scrolled_to_bottom()); EXPECT_TRUE(model().is_scrolled_to_bottom());
} }
// Tests that when the toolbar height is frozen, setting the height doesn't
// change the returned height until the toolbar is unfrozen.
TEST_F(FullscreenModelTest, FreezeToolbarHeight) {
model().SetFreezeToolbarHeight(true);
EXPECT_EQ(model().GetExpandedToolbarHeight(), 0);
model().SetExpandedToolbarHeight(100);
EXPECT_EQ(model().GetExpandedToolbarHeight(), 0);
model().SetFreezeToolbarHeight(false);
EXPECT_EQ(model().GetExpandedToolbarHeight(), 100);
}
...@@ -41,6 +41,7 @@ class TestFullscreenController : public FullscreenController { ...@@ -41,6 +41,7 @@ class TestFullscreenController : public FullscreenController {
void EnterFullscreen() override; void EnterFullscreen() override;
void ExitFullscreen() override; void ExitFullscreen() override;
void ResizeHorizontalViewport() override; void ResizeHorizontalViewport() override;
void FreezeToolbarHeight(bool freeze_toolbar_height) override;
// Calls FullscreenViewportInsetRangeChanged() on observers. // Calls FullscreenViewportInsetRangeChanged() on observers.
void OnFullscreenViewportInsetRangeChanged(UIEdgeInsets min_viewport_insets, void OnFullscreenViewportInsetRangeChanged(UIEdgeInsets min_viewport_insets,
......
...@@ -126,3 +126,9 @@ void TestFullscreenController::OnFullscreenWillAnimate( ...@@ -126,3 +126,9 @@ void TestFullscreenController::OnFullscreenWillAnimate(
void TestFullscreenController::ResizeHorizontalViewport() { void TestFullscreenController::ResizeHorizontalViewport() {
// NOOP in tests. // NOOP in tests.
} }
void TestFullscreenController::FreezeToolbarHeight(bool freeze_toolbar_height) {
if (model_) {
model_->SetFreezeToolbarHeight(freeze_toolbar_height);
}
}
...@@ -51,6 +51,7 @@ source_set("eg2_tests") { ...@@ -51,6 +51,7 @@ source_set("eg2_tests") {
"//base", "//base",
"//base/test:test_support", "//base/test:test_support",
"//ios/chrome/browser/ui:feature_flags", "//ios/chrome/browser/ui:feature_flags",
"//ios/chrome/browser/ui/fullscreen:feature_flags",
"//ios/chrome/test:eg_test_support+eg2", "//ios/chrome/test:eg_test_support+eg2",
"//ios/chrome/test/earl_grey:eg_test_support+eg2", "//ios/chrome/test/earl_grey:eg_test_support+eg2",
"//ios/testing/earl_grey:eg_test_support+eg2", "//ios/testing/earl_grey:eg_test_support+eg2",
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "base/ios/ios_util.h" #include "base/ios/ios_util.h"
#import "base/test/ios/wait_util.h" #import "base/test/ios/wait_util.h"
#import "ios/chrome/browser/ui/fullscreen/fullscreen_features.h"
#import "ios/chrome/browser/ui/thumb_strip/thumb_strip_feature.h" #import "ios/chrome/browser/ui/thumb_strip/thumb_strip_feature.h"
#import "ios/chrome/browser/ui/ui_feature_flags.h" #import "ios/chrome/browser/ui/ui_feature_flags.h"
#import "ios/chrome/test/earl_grey/chrome_earl_grey.h" #import "ios/chrome/test/earl_grey/chrome_earl_grey.h"
...@@ -55,6 +56,8 @@ std::unique_ptr<net::test_server::HttpResponse> HandleQueryTitle( ...@@ -55,6 +56,8 @@ std::unique_ptr<net::test_server::HttpResponse> HandleQueryTitle(
// See crbug.com/1143299. // See crbug.com/1143299.
if (base::ios::IsRunningOnIOS13OrLater()) { if (base::ios::IsRunningOnIOS13OrLater()) {
config.features_enabled.push_back(kExpandedTabStrip); config.features_enabled.push_back(kExpandedTabStrip);
config.features_disabled.push_back(
fullscreen::features::kSmoothScrollingDefault);
} }
return config; return config;
} }
......
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