Skip to content

GitLab

T
tangled
Commit 7ffe1230 authored by Stefan Zager's avatar Stefan Zager Committed by Commit Bot
Browse files
Options

Revert "[iOS] Fix tab strip views when using FullscreenProvider" 

This reverts commit c756c75c.

Reason for revert: Suspected cause of crbug.com/1142873

Original change's description:
> [iOS] Fix tab strip views when using FullscreenProvider
>
> When the FullscreenProvider is used, the layout of the browser views is
> slightly different. Instead of having the web content views start from
> the bottom of the toolbar, the web content extends all the way to the
> top of the BrowserContainerViewController, behind the tab strip.
>
> This means that the prior approach of making all the top background
> views clear does not work when using FullscreenProvider. Instead, this
> CL uses the following approach: set these background views to have
> the same background color as the tab grid. As soon as the panning
> gesture starts, make them clear so the tab grid shows through and the
> animations appear correctly. At the same time, move the web content
> views down so they don't obstruct the animations either.
>
> Bug: 1094335
> Change-Id: Ieea7d268e781da5e1ccd4c2c0ad3c0925407d133
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2489898
> Reviewed-by: Mark Cogan <marq@chromium.org>
> Commit-Queue: Robbie Gibson <rkgibson@google.com>
> Cr-Commit-Position: refs/heads/master@{#821200}

TBR=marq@chromium.org,rkgibson@google.com,mouraroberto@google.com

Change-Id: I2b026d35000d34b29de9235ed2752273f6f7dd33
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1142873
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2502574Reviewed-by: default avatarStefan Zager <szager@chromium.org>
Commit-Queue: Stefan Zager <szager@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821278}
parent 8c2c507f
Hide whitespace changes
Inline Side-by-side
Showing with 7 additions and 219 deletions
ios/chrome/browser/ui/browser_view/browser_view_controller.mm
View file @ 7ffe1230
......@@ -1891,15 +1891,11 @@ NSString* const kBrowserViewControllerSnackbarCategory =
}
- (void)installFakeStatusBar {
if (IsThumbStripEnabled() && !ios::GetChromeBrowserProvider()
->GetFullscreenProvider()
->IsInitialized()) {
if (IsThumbStripEnabled()) {
// A fake status bar on the browser view is not necessary when the thumb
// strip feature is enabled because the view behind the browser view already
// has a dark background. Adding a fake status bar would block the
// visibility of the thumb strip thumbnails when moving the browser view.
// However, if the Fullscreen Provider is used, then the web content extends
// up to behind the tab strip, making the fake status bar necessary.
return;
}
CGRect statusBarFrame = CGRectMake(0, 0, CGRectGetWidth(self.view.bounds), 0);
......@@ -2238,21 +2234,16 @@ NSString* const kBrowserViewControllerSnackbarCategory =
}
}
// Sets up the frame for the fake status bar. View must be loaded.
- (void)setupStatusBarLayout {
// Set the frame for the various views. View must be loaded.
- (void)setUpViewLayout:(BOOL)initialLayout {
DCHECK([self isViewLoaded]);
CGFloat topInset = self.view.safeAreaInsets.top;
// Update the fake toolbar background height.
CGRect fakeStatusBarFrame = _fakeStatusBarView.frame;
fakeStatusBarFrame.size.height = topInset;
_fakeStatusBarView.frame = fakeStatusBarFrame;
}
// Set the frame for the various views. View must be loaded.
- (void)setUpViewLayout:(BOOL)initialLayout {
DCHECK([self isViewLoaded]);
[self setupStatusBarLayout];
if (initialLayout) {
// Add the toolbars as child view controllers.
......@@ -2834,6 +2825,7 @@ NSString* const kBrowserViewControllerSnackbarCategory =
_fullscreenDisabler = std::make_unique<ScopedFullscreenDisabler>(
FullscreenController::FromBrowser(_browser));
}
// Hide the tab strip and take a snapshot of it. If a snapshot of a hidden
// view is taken, the snapshot will be a blank view. However, if the view's
// parent is hidden but the view itself is not, the snapshot will not be a
......@@ -2846,33 +2838,6 @@ NSString* const kBrowserViewControllerSnackbarCategory =
0, self.tabStripView.frame.size.height);
self.tabStripView.hidden = YES;
[self.contentArea addSubview:self.tabStripSnapshot];
// Remove the fake status bar to allow the thumb strip animations to appear.
[_fakeStatusBarView removeFromSuperview];
if (currentViewRevealState == ViewRevealState::Hidden) {
// When the Fullscreen Provider is used, the web content extends up to the
// top of the BVC view. It has a visible background and blocks the thumb
// strip. Thus, when the view revealing process starts, the web content
// frame must be moved down. To prevent the actual web content from jumping,
// the content offset must be moved up by a corresponding amount.
if (self.currentWebState && ![self isNTPActiveForCurrentWebState] &&
ios::GetChromeBrowserProvider()
->GetFullscreenProvider()
->IsInitialized()) {
CGFloat toolbarHeight = [self expandedTopToolbarHeight];
CGRect webStateViewFrame = UIEdgeInsetsInsetRect(
[self viewForWebState:self.currentWebState].frame,
UIEdgeInsetsMake(toolbarHeight, 0, 0, 0));
[self viewForWebState:self.currentWebState].frame = webStateViewFrame;
CRWWebViewScrollViewProxy* scrollProxy =
self.currentWebState->GetWebViewProxy().scrollViewProxy;
CGPoint scrollOffset = scrollProxy.contentOffset;
scrollOffset.y += toolbarHeight;
scrollProxy.contentOffset = scrollOffset;
}
}
}
- (void)animateViewReveal:(ViewRevealState)nextViewRevealState {
......@@ -2909,29 +2874,6 @@ NSString* const kBrowserViewControllerSnackbarCategory =
if (viewRevealState == ViewRevealState::Hidden) {
// Stop disabling fullscreen.
_fullscreenDisabler.reset();
// Add the status bar back to cover the web content.
[self installFakeStatusBar];
[self setupStatusBarLayout];
// See the comments in |-willAnimateViewReveal:| for the explantation of why
// this is necessary.
if (self.currentWebState && ![self isNTPActiveForCurrentWebState] &&
ios::GetChromeBrowserProvider()
->GetFullscreenProvider()
->IsInitialized()) {
CGFloat toolbarHeight = [self expandedTopToolbarHeight];
CGRect webStateViewFrame = UIEdgeInsetsInsetRect(
[self viewForWebState:self.currentWebState].frame,
UIEdgeInsetsMake(-toolbarHeight, 0, 0, 0));
[self viewForWebState:self.currentWebState].frame = webStateViewFrame;
CRWWebViewScrollViewProxy* scrollProxy =
self.currentWebState->GetWebViewProxy().scrollViewProxy;
CGPoint scrollOffset = scrollProxy.contentOffset;
scrollOffset.y -= toolbarHeight;
scrollProxy.contentOffset = scrollOffset;
}
}
}
......
ios/chrome/browser/ui/tabs/BUILD.gn
View file @ 7ffe1230
......@@ -68,8 +68,6 @@ source_set("tabs") {
"//ios/chrome/common/ui/elements",
"//ios/chrome/common/ui/resources:default_world_favicon",
"//ios/chrome/common/ui/util",
"//ios/public/provider/chrome/browser",
"//ios/public/provider/chrome/browser/ui",
"//ios/third_party/material_components_ios",
"//ios/web",
"//ui/base",
......
ios/chrome/browser/ui/tabs/tab_strip_controller.mm
View file @ 7ffe1230
......@@ -58,8 +58,6 @@
#include "ios/chrome/browser/web_state_list/web_state_opener.h"
#import "ios/chrome/common/ui/colors/semantic_color_names.h"
#include "ios/chrome/grit/ios_strings.h"
#include "ios/public/provider/chrome/browser/chrome_browser_provider.h"
#import "ios/public/provider/chrome/browser/ui/fullscreen_provider.h"
#import "ios/web/public/navigation/navigation_manager.h"
#import "ios/web/public/web_state.h"
#import "ios/web/public/web_state_observer_bridge.h"
......@@ -122,14 +120,7 @@ UIColor* BackgroundColor() {
if (base::FeatureList::IsEnabled(kExpandedTabStrip)) {
// The background needs to be clear to allow the thumb strip to be seen
// from behind the tab strip during the enter/exit thumb strip animation.
// However, when using the fullscreen provider, the WKWebView extends behind
// the tab strip. In this case, a clear background would lead to seeing the
// WKWebView instead of the thumb strip.
return ios::GetChromeBrowserProvider()
->GetFullscreenProvider()
->IsInitialized()
? UIColor.blackColor
: UIColor.clearColor;
return UIColor.clearColor;
}
return UIColor.blackColor;
}
......@@ -180,7 +171,6 @@ UIColor* BackgroundColor() {
CRWWebStateObserver,
TabStripViewLayoutDelegate,
TabViewDelegate,
ViewRevealingAnimatee,
WebStateListObserving,
WebStateFaviconDriverObserver,
UIGestureRecognizerDelegate,
......@@ -577,8 +567,6 @@ UIColor* BackgroundColor() {
- (void)setPanGestureHandler:
(ViewRevealingVerticalPanHandler*)panGestureHandler {
_panGestureHandler = panGestureHandler;
[self.panGestureHandler addAnimatee:self];
[self.view removeGestureRecognizer:self.panGestureRecognizer];
UIPanGestureRecognizer* panGestureRecognizer = [[UIPanGestureRecognizer alloc]
......@@ -1814,26 +1802,4 @@ UIColor* BackgroundColor() {
self.useTabStacking = [self shouldUseTabStacking];
}
#pragma mark - ViewRevealingAnimatee
- (void)willAnimateViewReveal:(ViewRevealState)currentViewRevealState {
// Specifically when using the FullscreenProvider, the background of the view
// is non-clear to cover the WKWebView. In this case, make the tab strip
// background clear as soon as view revealing begins so any animations that
// should be visible behind the tab strip are visible. See the comment on
// |BackgroundColor()| for more details.
self.view.backgroundColor = UIColor.clearColor;
}
- (void)animateViewReveal:(ViewRevealState)nextViewRevealState {
// No-op.
}
- (void)didAnimateViewReveal:(ViewRevealState)viewRevealState {
if (viewRevealState == ViewRevealState::Hidden) {
// Reset the background color to cover up the WKWebView if it is behind
// the tab strip.
self.view.backgroundColor = BackgroundColor();
}
}
@end
ios/chrome/browser/ui/thumb_strip/BUILD.gn
View file @ 7ffe1230
......@@ -31,26 +31,3 @@ source_set("feature_flags") {
]
configs += [ "//build/config/compiler:enable_arc" ]
}
source_set("eg2_tests") {
defines = [ "CHROME_EARL_GREY_2" ]
configs += [
"//build/config/compiler:enable_arc",
"//build/config/ios:xctest_config",
]
testonly = true
sources = [ "thumb_strip_egtest.mm" ]
deps = [
":feature_flags",
"//base",
"//base/test:test_support",
"//ios/chrome/browser/ui:feature_flags",
"//ios/chrome/test:eg_test_support+eg2",
"//ios/chrome/test/earl_grey:eg_test_support+eg2",
"//ios/testing/earl_grey:eg_test_support+eg2",
"//ios/third_party/earl_grey2:test_lib",
"//net:test_support",
"//url",
]
frameworks = [ "UIKit.framework" ]
}
ios/chrome/browser/ui/thumb_strip/thumb_strip_egtest.mm deleted 100644 → 0
View file @ 8c2c507f
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#import "base/test/ios/wait_util.h"
#import "ios/chrome/browser/ui/thumb_strip/thumb_strip_feature.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_ui.h"
#import "ios/chrome/test/earl_grey/chrome_matchers.h"
#import "ios/chrome/test/earl_grey/chrome_test_case.h"
#import "ios/testing/earl_grey/app_launch_configuration.h"
#import "ios/testing/earl_grey/earl_grey_test.h"
#include "net/test/embedded_test_server/http_request.h"
#include "net/test/embedded_test_server/http_response.h"
#include "net/test/embedded_test_server/request_handler_util.h"
#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
#endif
using base::test::ios::kWaitForPageLoadTimeout;
using base::test::ios::kWaitForJSCompletionTimeout;
using base::test::ios::WaitUntilConditionOrTimeout;
using chrome_test_util::PrimaryToolbar;
namespace {
// net::EmbeddedTestServer handler that responds with the request's query as the
// title and body.
std::unique_ptr<net::test_server::HttpResponse> HandleQueryTitle(
const net::test_server::HttpRequest& request) {
std::unique_ptr<net::test_server::BasicHttpResponse> http_response(
new net::test_server::BasicHttpResponse);
http_response->set_content_type("text/html");
http_response->set_content("<html><head><title>" + request.GetURL().query() +
"</title></head><body>" +
request.GetURL().query() + "</body></html>");
return std::move(http_response);
}
} // namespace
// Thumb Strip tests for Chrome.
@interface ThumbStripTestCase : ChromeTestCase
@end
@implementation ThumbStripTestCase
- (AppLaunchConfiguration)appConfigurationForTestCase {
AppLaunchConfiguration config;
config.features_enabled.push_back(kExpandedTabStrip);
return config;
}
// Sets up the EmbeddedTestServer as needed for tests.
- (void)setUpTestServer {
self.testServer->RegisterDefaultHandler(
base::Bind(net::test_server::HandlePrefixedRequest, "/querytitle",
base::Bind(&HandleQueryTitle)));
GREYAssertTrue(self.testServer->Start(), @"Test server failed to start");
}
// Tests that the entire thumb strip is visible in peeked state. Specifically,
// this tests that the thumb strip is not partially covered when using the
// FullscreenProvider.
- (void)testThumbStripVisibleInPeekedState {
// The feature only works on iPad.
if (![ChromeEarlGrey isIPadIdiom]) {
EARL_GREY_TEST_SKIPPED(@"Thumb strip is not enabled on iPhone");
}
[self setUpTestServer];
const GURL URL = self.testServer->GetURL("/querytitle?Hello");
[ChromeEarlGrey loadURL:URL];
[ChromeEarlGrey waitForWebStateContainingText:"Hello"];
// Swipe down to reveal the thumb strip.
[[EarlGrey selectElementWithMatcher:PrimaryToolbar()]
performAction:grey_swipeSlowInDirection(kGREYDirectionDown)];
// Make sure that the entire tab thumbnail is fully visible and not covered.
// This acts as a good proxy to the entire thumbstrip being visible.
[[EarlGrey
selectElementWithMatcher:grey_allOf(grey_accessibilityLabel(@"Hello"),
grey_kindOfClassName(@"GridCell"),
grey_minimumVisiblePercent(1), nil)]
assertWithMatcher:grey_notNil()];
}
@end
ios/chrome/test/earl_grey2/BUILD.gn
View file @ 7ffe1230
......@@ -170,7 +170,6 @@ chrome_ios_eg2_test("ios_chrome_ui_eg2tests_module") {
"//ios/chrome/browser/ui/side_swipe:eg2_tests",
"//ios/chrome/browser/ui/tab_grid:eg2_tests",
"//ios/chrome/browser/ui/tabs:eg2_tests",
"//ios/chrome/browser/ui/thumb_strip:eg2_tests",
"//ios/chrome/browser/ui/toolbar:eg2_tests",
"//ios/chrome/browser/ui/webui:eg2_tests",
"//ios/chrome/browser/ui/webui/interstitials:eg2_tests",
......
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