Commit bb9eb151 authored by Chris Lu's avatar Chris Lu Committed by Commit Bot

Revert "[ios] Remove Unused FullScreen Feature Flags"

This reverts commit 9384d0a2.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> [ios] Remove Unused FullScreen Feature Flags
> 
> There is no need for ViewportAdjustmentExperiment since the only experiment
> remaining is just the kSmoothScrollingDefault feature flag.
> GetActiveViewportExperiment() can just be a function that returns
> whether or not smooth scrolling is turned on. GetActiveViewportExperiment()
> and ViewportAdjustmentExperiment are still needed for downstream compatibility,
> and will be removed in a subsequent CL.
> 
> Bug: 914042
> Change-Id: I57bcd13dafaafb3b3a05ec65f23dacf2936e81e9
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1799663
> Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
> Commit-Queue: Chris Lu <thegreenfrog@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#697673}

TBR=kkhorimoto@chromium.org,thegreenfrog@chromium.org

Change-Id: Id1e0df1c06297d0249717102d5dc87e6343b6b63
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 914042
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1812091Reviewed-by: default avatarChris Lu <thegreenfrog@chromium.org>
Commit-Queue: Chris Lu <thegreenfrog@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697840}
parent 2b52d115
...@@ -326,9 +326,11 @@ const flags_ui::FeatureEntry kFeatureEntries[] = { ...@@ -326,9 +326,11 @@ const flags_ui::FeatureEntry kFeatureEntries[] = {
flags_ui::kOsIos, flags_ui::kOsIos,
FEATURE_VALUE_TYPE(autofill::features::kAutofillRichMetadataQueries)}, FEATURE_VALUE_TYPE(autofill::features::kAutofillRichMetadataQueries)},
{"fullscreen-viewport-adjustment-experiment", {"fullscreen-viewport-adjustment-experiment",
flag_descriptions::kFullscreenSmoothScrollingName, flag_descriptions::kFullscreenViewportAdjustmentExperimentName,
flag_descriptions::kFullscreenSmoothScrollingDescription, flags_ui::kOsIos, flag_descriptions::kFullscreenViewportAdjustmentExperimentDescription,
FEATURE_VALUE_TYPE(fullscreen::features::kSmoothScrollingDefault)}, flags_ui::kOsIos,
MULTI_VALUE_TYPE(
fullscreen::features::kViewportAdjustmentExperimentChoices)},
{"autofill-enforce-min-required-fields-for-heuristics", {"autofill-enforce-min-required-fields-for-heuristics",
flag_descriptions::kAutofillEnforceMinRequiredFieldsForHeuristicsName, flag_descriptions::kAutofillEnforceMinRequiredFieldsForHeuristicsName,
flag_descriptions:: flag_descriptions::
...@@ -507,6 +509,9 @@ const flags_ui::FeatureEntry kFeatureEntries[] = { ...@@ -507,6 +509,9 @@ const flags_ui::FeatureEntry kFeatureEntries[] = {
{"language-settings", flag_descriptions::kLanguageSettingsName, {"language-settings", flag_descriptions::kLanguageSettingsName,
flag_descriptions::kLanguageSettingsDescription, flags_ui::kOsIos, flag_descriptions::kLanguageSettingsDescription, flags_ui::kOsIos,
FEATURE_VALUE_TYPE(kLanguageSettings)}, FEATURE_VALUE_TYPE(kLanguageSettings)},
{"lock-bottom-toolbar", flag_descriptions::kLockBottomToolbarName,
flag_descriptions::kLockBottomToolbarDescription, flags_ui::kOsIos,
FEATURE_VALUE_TYPE(fullscreen::features::kLockBottomToolbar)},
{"toolbar-new-tab-button", flag_descriptions::kToolbarNewTabButtonName, {"toolbar-new-tab-button", flag_descriptions::kToolbarNewTabButtonName,
flag_descriptions::kToolbarNewTabButtonDescription, flags_ui::kOsIos, flag_descriptions::kToolbarNewTabButtonDescription, flags_ui::kOsIos,
FEATURE_VALUE_TYPE(kToolbarNewTabButton)}, FEATURE_VALUE_TYPE(kToolbarNewTabButton)},
......
...@@ -204,11 +204,11 @@ const char kFindInPageiFrameName[] = "Find in Page in iFrames."; ...@@ -204,11 +204,11 @@ const char kFindInPageiFrameName[] = "Find in Page in iFrames.";
const char kFindInPageiFrameDescription[] = const char kFindInPageiFrameDescription[] =
"When enabled, Find In Page will search in iFrames."; "When enabled, Find In Page will search in iFrames.";
const char kFullscreenSmoothScrollingName[] = "Fullscreen Smooth Scrolling"; const char kFullscreenViewportAdjustmentExperimentName[] =
const char kFullscreenSmoothScrollingDescription[] = "Fullscreen Viewport Adjustment Mode";
"When enabled, the web view's insets are updated for scoll events. If " const char kFullscreenViewportAdjustmentExperimentDescription[] =
"disabled, the " "The different ways in which the web view's viewport is updated for scroll "
"the web view's frame are updated."; "events. The default option updates the web view's frame.";
const char kIgnoresViewportScaleLimitsName[] = "Ignore Viewport Scale Limits"; const char kIgnoresViewportScaleLimitsName[] = "Ignore Viewport Scale Limits";
const char kIgnoresViewportScaleLimitsDescription[] = const char kIgnoresViewportScaleLimitsDescription[] =
...@@ -230,6 +230,11 @@ const char kLanguageSettingsDescription[] = ...@@ -230,6 +230,11 @@ const char kLanguageSettingsDescription[] =
"Enables the Language Settings page allowing modifications to user " "Enables the Language Settings page allowing modifications to user "
"preferred languages and translate preferences."; "preferred languages and translate preferences.";
const char kLockBottomToolbarName[] = "Lock bottom toolbar";
const char kLockBottomToolbarDescription[] =
"When enabled, the bottom toolbar will not get collapsed when scrolling "
"into fullscreen mode.";
const char kMarkHttpAsName[] = "Mark non-secure origins as non-secure"; const char kMarkHttpAsName[] = "Mark non-secure origins as non-secure";
const char kMarkHttpAsDescription[] = "Change the UI treatment for HTTP pages"; const char kMarkHttpAsDescription[] = "Change the UI treatment for HTTP pages";
......
...@@ -167,10 +167,10 @@ extern const char kFillOnAccountSelectHttpDescription[]; ...@@ -167,10 +167,10 @@ extern const char kFillOnAccountSelectHttpDescription[];
extern const char kFindInPageiFrameName[]; extern const char kFindInPageiFrameName[];
extern const char kFindInPageiFrameDescription[]; extern const char kFindInPageiFrameDescription[];
// Title and description for the flag to update the web view's insets for scroll // Title and description for the command line switch used to determine the
// events. // active fullscreen viewport adjustment mode.
extern const char kFullscreenSmoothScrollingName[]; extern const char kFullscreenViewportAdjustmentExperimentName[];
extern const char kFullscreenSmoothScrollingDescription[]; extern const char kFullscreenViewportAdjustmentExperimentDescription[];
// Title and description for the flag to ignore viewport scale limits. // Title and description for the flag to ignore viewport scale limits.
extern const char kIgnoresViewportScaleLimitsName[]; extern const char kIgnoresViewportScaleLimitsName[];
...@@ -189,6 +189,10 @@ extern const char kInProductHelpDemoModeDescription[]; ...@@ -189,6 +189,10 @@ extern const char kInProductHelpDemoModeDescription[];
extern const char kLanguageSettingsName[]; extern const char kLanguageSettingsName[];
extern const char kLanguageSettingsDescription[]; extern const char kLanguageSettingsDescription[];
// Title and description for the flag to lock the bottom toolbar into place.
extern const char kLockBottomToolbarName[];
extern const char kLockBottomToolbarDescription[];
// Title, description, and options for the MarkHttpAs setting that controls // Title, description, and options for the MarkHttpAs setting that controls
// display of omnibox warnings about non-secure pages. // display of omnibox warnings about non-secure pages.
extern const char kMarkHttpAsName[]; extern const char kMarkHttpAsName[];
......
...@@ -597,6 +597,9 @@ NSString* const kBrowserViewControllerSnackbarCategory = ...@@ -597,6 +597,9 @@ NSString* const kBrowserViewControllerSnackbarCategory =
// The webState of the active tab. // The webState of the active tab.
@property(nonatomic, readonly) web::WebState* currentWebState; @property(nonatomic, readonly) web::WebState* currentWebState;
// Whether the safe area insets should be used to adjust the viewport.
@property(nonatomic, readonly) BOOL usesSafeInsetsForViewportAdjustments;
// Whether the keyboard observer helper is viewed // Whether the keyboard observer helper is viewed
@property(nonatomic, strong) KeyboardObserverHelper* observer; @property(nonatomic, strong) KeyboardObserverHelper* observer;
...@@ -1069,6 +1072,15 @@ NSString* const kBrowserViewControllerSnackbarCategory = ...@@ -1069,6 +1072,15 @@ NSString* const kBrowserViewControllerSnackbarCategory =
: nullptr; : nullptr;
} }
- (BOOL)usesSafeInsetsForViewportAdjustments {
fullscreen::features::ViewportAdjustmentExperiment viewportExperiment =
fullscreen::features::GetActiveViewportExperiment();
return viewportExperiment ==
fullscreen::features::ViewportAdjustmentExperiment::SAFE_AREA ||
viewportExperiment ==
fullscreen::features::ViewportAdjustmentExperiment::HYBRID;
}
- (BubblePresenter*)bubblePresenter { - (BubblePresenter*)bubblePresenter {
if (!_bubblePresenter && self.browserState) { if (!_bubblePresenter && self.browserState) {
self.bubblePresenter = self.bubblePresenter =
...@@ -1674,7 +1686,7 @@ NSString* const kBrowserViewControllerSnackbarCategory = ...@@ -1674,7 +1686,7 @@ NSString* const kBrowserViewControllerSnackbarCategory =
self.secondaryToolbarNoFullscreenHeightConstraint.constant = self.secondaryToolbarNoFullscreenHeightConstraint.constant =
[self secondaryToolbarHeightWithInset]; [self secondaryToolbarHeightWithInset];
[self updateFootersForFullscreenProgress:self.footerFullscreenProgress]; [self updateFootersForFullscreenProgress:self.footerFullscreenProgress];
if (self.currentWebState) { if (!self.usesSafeInsetsForViewportAdjustments && self.currentWebState) {
UIEdgeInsets contentPadding = UIEdgeInsets contentPadding =
self.currentWebState->GetWebViewProxy().contentInset; self.currentWebState->GetWebViewProxy().contentInset;
contentPadding.bottom = AlignValueToPixel( contentPadding.bottom = AlignValueToPixel(
...@@ -3816,6 +3828,9 @@ NSString* const kBrowserViewControllerSnackbarCategory = ...@@ -3816,6 +3828,9 @@ NSString* const kBrowserViewControllerSnackbarCategory =
// Translates the footer view up and down according to |progress|, where a // Translates the footer view up and down according to |progress|, where a
// progress of 1.0 fully shows the footer and a progress of 0.0 fully hides it. // progress of 1.0 fully shows the footer and a progress of 0.0 fully hides it.
- (void)updateFootersForFullscreenProgress:(CGFloat)progress { - (void)updateFootersForFullscreenProgress:(CGFloat)progress {
// If the bottom toolbar is locked into place, reset |progress| to 1.0.
if (base::FeatureList::IsEnabled(fullscreen::features::kLockBottomToolbar))
progress = 1.0;
self.footerFullscreenProgress = progress; self.footerFullscreenProgress = progress;
...@@ -3858,10 +3873,26 @@ NSString* const kBrowserViewControllerSnackbarCategory = ...@@ -3858,10 +3873,26 @@ NSString* const kBrowserViewControllerSnackbarCategory =
// safe area, so the unsafe top height must be added. // safe area, so the unsafe top height must be added.
CGFloat top = AlignValueToPixel( CGFloat top = AlignValueToPixel(
self.headerHeight + (progress - 1.0) * [self primaryToolbarHeightDelta]); self.headerHeight + (progress - 1.0) * [self primaryToolbarHeightDelta]);
CGFloat bottom = // If the bottom toolbar is locked into place, use 1.0 instead of |progress|.
AlignValueToPixel(progress * [self secondaryToolbarHeightWithInset]); CGFloat bottomProgress =
base::FeatureList::IsEnabled(fullscreen::features::kLockBottomToolbar)
? 1.0
: progress;
CGFloat bottom = AlignValueToPixel(bottomProgress *
[self secondaryToolbarHeightWithInset]);
if (self.usesSafeInsetsForViewportAdjustments) {
if (fullscreen::features::GetActiveViewportExperiment() ==
fullscreen::features::ViewportAdjustmentExperiment::HYBRID) {
[self updateWebViewFrameForBottomOffset:bottom];
}
[self updateContentPaddingForTopToolbarHeight:top bottomToolbarHeight:bottom]; [self updateBrowserSafeAreaForTopToolbarHeight:top
bottomToolbarHeight:bottom];
} else {
[self updateContentPaddingForTopToolbarHeight:top
bottomToolbarHeight:bottom];
}
} }
// Updates the frame of the web view so that it's |offset| from the bottom of // Updates the frame of the web view so that it's |offset| from the bottom of
......
...@@ -11,25 +11,35 @@ ...@@ -11,25 +11,35 @@
namespace fullscreen { namespace fullscreen {
namespace features { namespace features {
// Feature used by finch config to enable smooth scrolling when the default // The name of the command line switch used to control the method by which the
// viewport adjustment experiment is selected via command line switches. // viewport of the content area is updated by scrolling events.
extern const base::Feature kSmoothScrollingDefault; extern const char kViewportAdjustmentExperimentCommandLineSwitch[];
// The available viewport adjustment experiments. The choices in this array
// correspond with the ViewportAdjustmentExperiment values.
extern const flags_ui::FeatureEntry::Choice
kViewportAdjustmentExperimentChoices[6];
// Enum type describing viewport adjustment experiments. // Enum type describing viewport adjustment experiments.
enum class ViewportAdjustmentExperiment : short { enum class ViewportAdjustmentExperiment : short {
FRAME = 0, // Adjust the viewport by resizing the entire WKWebView. FRAME = 0, // Adjust the viewport by resizing the entire WKWebView.
CONTENT_INSET, // Adjust the viewport by updating the WKWebView's scroll view
// contentInset.
SAFE_AREA, // Adjust the viewport by updating the safe area of the browser
// container view.
HYBRID, // Translates the web view up and down and updates the viewport using
// safe area insets.
SMOOTH_SCROLLING, // Adjusts the viewport using the smooth scrolling SMOOTH_SCROLLING, // Adjusts the viewport using the smooth scrolling
// workaround. // workaround.
}; };
// Convenience method for retrieving the active viewport adjustment experiment // Convenience method for retrieving the active viewport adjustment experiment
// from the command line. TODO(crbug.com/914042): Remove once the internal // from the command line.
// references are moved to ShouldUseSmoothScrolling().
ViewportAdjustmentExperiment GetActiveViewportExperiment(); ViewportAdjustmentExperiment GetActiveViewportExperiment();
// Convenience method for determining when to adjust the viewport by resizing // Used to control whether the bottom toolbar should be locked into the extended
// WKWebView or using smooth scrolling. // position (i.e. fullscreen progress == 1.0).
bool ShouldUseSmoothScrolling(); extern const base::Feature kLockBottomToolbar;
} // namespace features } // namespace features
} // namespace fullscreen } // namespace fullscreen
......
...@@ -11,21 +11,63 @@ ...@@ -11,21 +11,63 @@
#error "This file requires ARC support." #error "This file requires ARC support."
#endif #endif
namespace fullscreen { namespace {
namespace features { // The command line values for the content inset and safe area experiment
// choices.
const char kFrameChoiceValue[] = "frame";
const char kContentInsetChoiceValue[] = "content-inset";
const char kSafeAreaChoiceValue[] = "safe-area";
const char kHybridChoiceValue[] = "hybrid";
const char kSmoothScrollingChoiceValue[] = "smooth";
// Feature used by finch config to enable smooth scrolling when the default
// viewport adjustment experiment is selected via command line switches.
const base::Feature kSmoothScrollingDefault{"FullscreenSmoothScrollingDefault", const base::Feature kSmoothScrollingDefault{"FullscreenSmoothScrollingDefault",
base::FEATURE_ENABLED_BY_DEFAULT}; base::FEATURE_ENABLED_BY_DEFAULT};
}
namespace fullscreen {
namespace features {
const char kViewportAdjustmentExperimentCommandLineSwitch[] =
"fullscreen-viewport-adjustment-experiment";
const flags_ui::FeatureEntry::Choice kViewportAdjustmentExperimentChoices[] = {
{flags_ui::kGenericExperimentChoiceDefault, "", ""},
{"Update Content Inset", kViewportAdjustmentExperimentCommandLineSwitch,
"content-inset"},
{"Update Safe Area", kViewportAdjustmentExperimentCommandLineSwitch,
"safe-area"},
{"Use Hybrid Implementation",
kViewportAdjustmentExperimentCommandLineSwitch, "hybrid"},
{"Use Smooth Scrolling", kViewportAdjustmentExperimentCommandLineSwitch,
"smooth"},
{"Update Frame", kViewportAdjustmentExperimentCommandLineSwitch, "frame"}};
ViewportAdjustmentExperiment GetActiveViewportExperiment() { ViewportAdjustmentExperiment GetActiveViewportExperiment() {
return ShouldUseSmoothScrolling() const base::CommandLine* command_line =
base::CommandLine::ForCurrentProcess();
if (command_line->HasSwitch(kViewportAdjustmentExperimentCommandLineSwitch)) {
std::string viewport_experiment = command_line->GetSwitchValueASCII(
kViewportAdjustmentExperimentCommandLineSwitch);
if (viewport_experiment == std::string(kContentInsetChoiceValue))
return ViewportAdjustmentExperiment::CONTENT_INSET;
if (viewport_experiment == std::string(kSafeAreaChoiceValue))
return ViewportAdjustmentExperiment::SAFE_AREA;
if (viewport_experiment == std::string(kHybridChoiceValue))
return ViewportAdjustmentExperiment::HYBRID;
if (viewport_experiment == std::string(kSmoothScrollingChoiceValue))
return ViewportAdjustmentExperiment::SMOOTH_SCROLLING;
if (viewport_experiment == std::string(kFrameChoiceValue))
return ViewportAdjustmentExperiment::FRAME;
}
return base::FeatureList::IsEnabled(kSmoothScrollingDefault)
? ViewportAdjustmentExperiment::SMOOTH_SCROLLING ? ViewportAdjustmentExperiment::SMOOTH_SCROLLING
: ViewportAdjustmentExperiment::FRAME; : ViewportAdjustmentExperiment::FRAME;
} }
bool ShouldUseSmoothScrolling() { const base::Feature kLockBottomToolbar{"LockBottomToolbar",
return base::FeatureList::IsEnabled(kSmoothScrollingDefault); base::FEATURE_DISABLED_BY_DEFAULT};
}
} // namespace features } // namespace features
} // namespace fullscreen } // namespace fullscreen
...@@ -73,10 +73,14 @@ class FullscreenModel : public ChromeBroadcastObserverInterface { ...@@ -73,10 +73,14 @@ 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 {
const CGFloat kBottomToolbarProgress =
base::FeatureList::IsEnabled(fullscreen::features::kLockBottomToolbar)
? 1.0
: progress;
return UIEdgeInsetsMake( return UIEdgeInsetsMake(
collapsed_toolbar_height_ + collapsed_toolbar_height_ +
progress * (expanded_toolbar_height_ - collapsed_toolbar_height_), progress * (expanded_toolbar_height_ - collapsed_toolbar_height_),
0, progress * bottom_toolbar_height_, 0); 0, kBottomToolbarProgress * bottom_toolbar_height_, 0);
} }
// Increments and decrements |disabled_counter_| for features that require the // Increments and decrements |disabled_counter_| for features that require the
......
...@@ -28,6 +28,8 @@ ...@@ -28,6 +28,8 @@
#error "This file requires ARC support." #error "This file requires ARC support."
#endif #endif
using fullscreen::features::ViewportAdjustmentExperiment;
FullscreenWebStateObserver::FullscreenWebStateObserver( FullscreenWebStateObserver::FullscreenWebStateObserver(
FullscreenController* controller, FullscreenController* controller,
FullscreenModel* model, FullscreenModel* model,
...@@ -81,13 +83,18 @@ void FullscreenWebStateObserver::DidFinishNavigation( ...@@ -81,13 +83,18 @@ void FullscreenWebStateObserver::DidFinishNavigation(
// - For normal pages, using |contentInset| breaks the layout of fixed- // - For normal pages, using |contentInset| breaks the layout of fixed-
// position DOM elements, so top padding must be accomplished by updating // position DOM elements, so top padding must be accomplished by updating
// the WKWebView's frame. // the WKWebView's frame.
bool force_content_inset =
fullscreen::features::GetActiveViewportExperiment() ==
ViewportAdjustmentExperiment::CONTENT_INSET;
bool is_pdf = web_state->GetContentsMimeType() == "application/pdf"; bool is_pdf = web_state->GetContentsMimeType() == "application/pdf";
bool use_content_inset = force_content_inset || is_pdf;
id<CRWWebViewProxy> web_view_proxy = web_state->GetWebViewProxy(); id<CRWWebViewProxy> web_view_proxy = web_state->GetWebViewProxy();
web_view_proxy.shouldUseViewContentInset = is_pdf; web_view_proxy.shouldUseViewContentInset = use_content_inset;
model_->SetResizesScrollView(!is_pdf && !ios::GetChromeBrowserProvider() model_->SetResizesScrollView(!use_content_inset &&
->GetFullscreenProvider() !ios::GetChromeBrowserProvider()
->IsInitialized()); ->GetFullscreenProvider()
->IsInitialized());
// Only reset the model for document-changing navigations or same-document // Only reset the model for document-changing navigations or same-document
// navigations that update the visible URL. // navigations that update the visible URL.
......
...@@ -32,6 +32,8 @@ ...@@ -32,6 +32,8 @@
#error "This file requires ARC support." #error "This file requires ARC support."
#endif #endif
using fullscreen::features::ViewportAdjustmentExperiment;
namespace { namespace {
// This enum is used to record the overscroll actions performed by the user on // This enum is used to record the overscroll actions performed by the user on
// the histogram named |OverscrollActions|. // the histogram named |OverscrollActions|.
...@@ -569,9 +571,12 @@ NSString* const kOverscrollActionsDidEnd = @"OverscrollActionsDidStop"; ...@@ -569,9 +571,12 @@ NSString* const kOverscrollActionsDidEnd = @"OverscrollActionsDidStop";
- (BOOL)viewportAdjustsContentInset { - (BOOL)viewportAdjustsContentInset {
if (_webViewProxy.shouldUseViewContentInset) if (_webViewProxy.shouldUseViewContentInset)
return YES; return YES;
return ios::GetChromeBrowserProvider() ViewportAdjustmentExperiment experiment =
->GetFullscreenProvider() fullscreen::features::GetActiveViewportExperiment();
->IsInitialized(); return experiment == ViewportAdjustmentExperiment::SMOOTH_SCROLLING &&
ios::GetChromeBrowserProvider()
->GetFullscreenProvider()
->IsInitialized();
} }
- (void)recordMetricForTriggeredAction:(OverscrollAction)action { - (void)recordMetricForTriggeredAction:(OverscrollAction)action {
......
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