Skip to content

Commit

Permalink
[ios] Create StartSurfaceSplashStartup
Browse files Browse the repository at this point in the history
This feature, enabled by default, will open a new tab to show the Start
Surface when the SceneState activation level is greater than or equal to
SceneActivationLevelForegroundInactive, instead of just occurring at
SceneActivationLevelForegroundActive. It also will not be animated to
minimize the visual disturbance that is seen open app startup. These
combined allows for the tab switch transition to happen sooner and
more seamlessly.

Video: https://drive.google.com/file/d/1OdSXVz0g2rk5-_TEgCbDkedA4o5fM5G6/view?usp=sharing

(cherry picked from commit ef05477)

Bug: 1173160
Change-Id: I5754e4f7892ebd2207726118804e093e1488b8cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3582464
Reviewed-by: Mark Cogan <marq@chromium.org>
Reviewed-by: Sergio Collazos <sczs@chromium.org>
Commit-Queue: Chris Lu <thegreenfrog@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#995326}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3616868
Auto-Submit: Chris Lu <thegreenfrog@chromium.org>
Commit-Queue: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/branch-heads/5005@{#347}
Cr-Branched-From: 5b4d945-refs/heads/main@{#992738}
  • Loading branch information
Chris Lu authored and Chromium LUCI CQ committed May 2, 2022
1 parent ee471bf commit d0f6650
Show file tree
Hide file tree
Showing 26 changed files with 228 additions and 57 deletions.
6 changes: 4 additions & 2 deletions ios/chrome/app/BUILD.gn
Expand Up @@ -64,6 +64,7 @@ source_set("unit_tests") {
"//ios/chrome/browser/prefs:browser_prefs",
"//ios/chrome/browser/tabs",
"//ios/chrome/browser/ui/main:scene",
"//ios/chrome/browser/ui/main:scene_state_header",
"//ios/chrome/browser/ui/main/test",
"//ios/chrome/browser/ui/safe_mode",
"//ios/chrome/common/app_group",
Expand Down Expand Up @@ -169,7 +170,7 @@ source_set("app_metrics_app_state_agent") {
"//ios/chrome/app/application_delegate:application_delegate_internal",
"//ios/chrome/app/application_delegate:observing_app_state_agent",
"//ios/chrome/browser/metrics",
"//ios/chrome/browser/ui/main:scene",
"//ios/chrome/browser/ui/main:scene_state_header",
]
}

Expand Down Expand Up @@ -248,7 +249,7 @@ source_set("first_run_app_state_agent") {
"//ios/chrome/browser/ui/first_run:utils",
"//ios/chrome/browser/ui/main:browser_interface_provider",
"//ios/chrome/browser/ui/main:observing_scene_agent",
"//ios/chrome/browser/ui/main:scene",
"//ios/chrome/browser/ui/main:scene_state_header",
"//ios/chrome/browser/ui/screen:screen_provider",
]
}
Expand Down Expand Up @@ -358,6 +359,7 @@ source_set("app_internal") {
"//ios/chrome/browser/ui/first_run:utils",
"//ios/chrome/browser/ui/main",
"//ios/chrome/browser/ui/main:scene",
"//ios/chrome/browser/ui/main:scene_state_header",
"//ios/chrome/browser/ui/tab_switcher/tab_grid",
"//ios/chrome/browser/ui/tab_switcher/tab_grid:tab_grid_ui",
"//ios/chrome/browser/ui/table_view",
Expand Down
1 change: 1 addition & 0 deletions ios/chrome/app/application_delegate/BUILD.gn
Expand Up @@ -171,6 +171,7 @@ source_set("observing_app_state_agent") {
public_deps = [
":app_state_header",
"//ios/chrome/browser/ui/main:scene_state_header",
"//ios/chrome/browser/ui/main:scene_state_observer",
]
}

Expand Down
8 changes: 8 additions & 0 deletions ios/chrome/browser/ntp/new_tab_page_tab_helper.h
Expand Up @@ -33,6 +33,10 @@ class NewTabPageTabHelper : public web::WebStateObserver,
// Sets the delegate. The delegate is not owned by the tab helper.
void SetDelegate(id<NewTabPageTabHelperDelegate> delegate);

// Setter/Getter for whether to show the Start Surface.
bool ShouldShowStartSurface() const;
void SetShowStartSurface(bool show_start_surface);

// Returns true when the current web_state is an NTP and the underlying
// controllers have been created.
bool IsActive() const;
Expand Down Expand Up @@ -106,6 +110,10 @@ class NewTabPageTabHelper : public web::WebStateObserver,
// |YES| if the current tab helper is active.
BOOL active_ = NO;

// |YES| if the NTP for this WebState should be configured to show the Start
// Surface.
BOOL show_start_surface_ = false;

// |YES| if the NTP's underlying ios/web page is still loading.
BOOL ignore_load_requests_ = NO;

Expand Down
8 changes: 8 additions & 0 deletions ios/chrome/browser/ntp/new_tab_page_tab_helper.mm
Expand Up @@ -70,6 +70,14 @@
}
}

bool NewTabPageTabHelper::ShouldShowStartSurface() const {
return show_start_surface_;
}

void NewTabPageTabHelper::SetShowStartSurface(bool show_start_surface) {
show_start_surface_ = show_start_surface;
}

bool NewTabPageTabHelper::IsActive() const {
return active_;
}
Expand Down
2 changes: 1 addition & 1 deletion ios/chrome/browser/ui/authentication/BUILD.gn
Expand Up @@ -189,7 +189,7 @@ source_set("eg_app_support+eg2") {
"//ios/chrome/browser/ui/authentication/unified_consent:unified_consent_ui",
"//ios/chrome/browser/ui/authentication/unified_consent/identity_chooser:identity_chooser_ui",
"//ios/chrome/browser/ui/commands",
"//ios/chrome/browser/ui/main:scene",
"//ios/chrome/browser/ui/main:scene_state_header",
"//ios/chrome/browser/ui/util",
"//ios/chrome/test/app:test_support",
"//ios/public/provider/chrome/browser/signin:fake_chrome_identity",
Expand Down
1 change: 1 addition & 0 deletions ios/chrome/browser/ui/browser_view/BUILD.gn
Expand Up @@ -154,6 +154,7 @@ source_set("browser_view") {
"//ios/chrome/browser/ui/sharing",
"//ios/chrome/browser/ui/side_swipe",
"//ios/chrome/browser/ui/snackbar",
"//ios/chrome/browser/ui/start_surface:feature_flags",
"//ios/chrome/browser/ui/tab_switcher/tab_strip",
"//ios/chrome/browser/ui/tabs",
"//ios/chrome/browser/ui/tabs:constants",
Expand Down
12 changes: 10 additions & 2 deletions ios/chrome/browser/ui/browser_view/browser_view_controller.mm
Expand Up @@ -102,6 +102,7 @@
#import "ios/chrome/browser/ui/settings/sync/utils/sync_util.h"
#import "ios/chrome/browser/ui/side_swipe/side_swipe_controller.h"
#import "ios/chrome/browser/ui/side_swipe/swipe_view.h"
#import "ios/chrome/browser/ui/start_surface/start_surface_features.h"
#import "ios/chrome/browser/ui/tab_switcher/tab_strip/tab_strip_coordinator.h"
#import "ios/chrome/browser/ui/tabs/background_tab_animation_view.h"
#import "ios/chrome/browser/ui/tabs/foreground_tab_animation_view.h"
Expand Down Expand Up @@ -4034,8 +4035,14 @@ - (void)webStateList:(WebStateList*)webStateList
self /* id<SyncPresenter> */);
}

BOOL inBackground = !activating;
if (IsStartSurfaceSplashStartupEnabled()) {
inBackground =
inBackground ||
NewTabPageTabHelper::FromWebState(webState)->ShouldShowStartSurface();
}
[self initiateNewTabAnimationForWebState:webState
willOpenInBackground:!activating];
willOpenInBackground:inBackground];
}

#pragma mark - WebStateListObserver helpers (new tab animations)
Expand Down Expand Up @@ -4372,7 +4379,8 @@ - (void)captivePortalTabHelper:(CaptivePortalTabHelper*)tabHelper
web_navigation_util::CreateWebLoadParams(
landingURL, ui::PAGE_TRANSITION_TYPED, nullptr),
nil, false, self.browser->GetWebStateList()->count(),
/*in_background=*/false, /*inherit_opener=*/false);
/*in_background=*/false, /*inherit_opener=*/false,
/*should_show_start_surface=*/false);
}

#pragma mark - PageInfoPresentation
Expand Down
Expand Up @@ -24,6 +24,7 @@
#include "ios/chrome/browser/favicon/ios_chrome_large_icon_service_factory.h"
#include "ios/chrome/browser/favicon/large_icon_cache.h"
#import "ios/chrome/browser/main/browser.h"
#import "ios/chrome/browser/ntp/new_tab_page_tab_helper.h"
#include "ios/chrome/browser/ntp_tiles/ios_most_visited_sites_factory.h"
#import "ios/chrome/browser/policy/policy_util.h"
#include "ios/chrome/browser/pref_names.h"
Expand Down Expand Up @@ -340,7 +341,6 @@ - (void)viewDidDisappear {
DiscoverFeedServiceFactory::GetForBrowserState(
self.browser->GetBrowserState())
->SetIsShownOnStartSurface(false);
// Update DiscoverFeedService to NO
if (ShouldShowReturnToMostRecentTabForStartSurface()) {
[self.contentSuggestionsMediator hideRecentTabTile];
}
Expand Down Expand Up @@ -510,8 +510,14 @@ - (UIContextMenuConfiguration*)contextMenuConfigurationForItem:
- (void)configureStartSurfaceIfNeeded {
SceneState* scene =
SceneStateBrowserAgent::FromBrowser(self.browser)->GetSceneState();
if (!scene.modifytVisibleNTPForStartSurface)
if (IsStartSurfaceSplashStartupEnabled()) {
if (!NewTabPageTabHelper::FromWebState(self.webState)
->ShouldShowStartSurface()) {
return;
}
} else if (!scene.modifytVisibleNTPForStartSurface) {
return;
}

// Update Mediator property to signal the NTP is currently showing Start.
self.contentSuggestionsMediator.showingStartSurface = YES;
Expand Down Expand Up @@ -544,7 +550,12 @@ - (void)configureStartSurfaceIfNeeded {
base::RecordAction(
base::UserMetricsAction("IOS.StartSurface.HideShortcuts"));
}
scene.modifytVisibleNTPForStartSurface = NO;
if (IsStartSurfaceSplashStartupEnabled()) {
NewTabPageTabHelper::FromWebState(self.webState)
->SetShowStartSurface(false);
} else {
scene.modifytVisibleNTPForStartSurface = NO;
}
}

// Triggers the URL sharing flow for the given |URL| and |title|, with the
Expand Down
2 changes: 1 addition & 1 deletion ios/chrome/browser/ui/first_run/BUILD.gn
Expand Up @@ -270,7 +270,7 @@ source_set("eg_app_support+eg2") {
"//ios/chrome/browser",
"//ios/chrome/browser/sync",
"//ios/chrome/browser/ui:feature_flags",
"//ios/chrome/browser/ui/main:scene",
"//ios/chrome/browser/ui/main:scene_state_header",
"//ios/chrome/browser/ui/main:scene_testing",
"//ios/chrome/test/app:test_support",
"//ios/third_party/earl_grey2:app_framework+link",
Expand Down
9 changes: 7 additions & 2 deletions ios/chrome/browser/ui/main/BUILD.gn
Expand Up @@ -36,13 +36,18 @@ source_set("observing_scene_agent") {
source_set("scene_state_header") {
configs += [ "//build/config/compiler:enable_arc" ]
sources = [
"connection_information.h",
"scene_controller.h",
"scene_state.h",
"scene_state_browser_agent.h",
]
public_deps = [
":scene_state_observer",
"//ios/chrome/app/application_delegate:tab_opening",
"//ios/chrome/browser/main:public",
"//ios/chrome/browser/ui/commands",
"//ios/chrome/browser/ui/scoped_ui_blocker",
"//ios/chrome/browser/web_state_list",
"//ios/chrome/browser/window_activities",
]
}
Expand Down Expand Up @@ -82,8 +87,6 @@ source_set("incognito_blocker_scene_agent") {
source_set("scene") {
configs += [ "//build/config/compiler:enable_arc" ]
sources = [
"connection_information.h",
"scene_controller.h",
"scene_controller.mm",
"scene_delegate.h",
"scene_delegate.mm",
Expand All @@ -99,6 +102,7 @@ source_set("scene") {
":incognito_blocker_scene_agent",
":main",
":observing_scene_agent",
":scene_state_header",
":scene_testing",
"//base",
"//base/ios",
Expand Down Expand Up @@ -164,6 +168,7 @@ source_set("scene") {
"//ios/chrome/browser/ui/settings:settings_root",
"//ios/chrome/browser/ui/settings/sync",
"//ios/chrome/browser/ui/start_surface",
"//ios/chrome/browser/ui/start_surface:feature_flags",
"//ios/chrome/browser/ui/tab_switcher/tab_grid",
"//ios/chrome/browser/ui/thumb_strip:feature_flags",
"//ios/chrome/browser/ui/toolbar/public",
Expand Down
3 changes: 3 additions & 0 deletions ios/chrome/browser/ui/main/scene_controller.h
Expand Up @@ -39,6 +39,9 @@
// YES if the scene is presenting the signin view.
@property(nonatomic, readonly) BOOL isPresentingSigninView;

// YES if the tab grid is the main user interface at the moment.
@property(nonatomic, readonly, getter=isTabGridVisible) BOOL tabGridVisible;

// The view controller that is active. Can be either a BrowserViewController or
// TabGridViewController.
@property(nonatomic, readonly) UIViewController* activeViewController;
Expand Down
16 changes: 13 additions & 3 deletions ios/chrome/browser/ui/main/scene_controller.mm
Expand Up @@ -102,6 +102,7 @@
#import "ios/chrome/browser/ui/main/ui_blocker_scene_agent.h"
#import "ios/chrome/browser/ui/scoped_ui_blocker/scoped_ui_blocker.h"
#import "ios/chrome/browser/ui/settings/settings_navigation_controller.h"
#import "ios/chrome/browser/ui/start_surface/start_surface_features.h"
#import "ios/chrome/browser/ui/start_surface/start_surface_recent_tab_browser_agent.h"
#import "ios/chrome/browser/ui/start_surface/start_surface_scene_agent.h"
#import "ios/chrome/browser/ui/start_surface/start_surface_util.h"
Expand Down Expand Up @@ -361,6 +362,10 @@ - (BOOL)isPresentingSigninView {
return self.signinCoordinator != nil;
}

- (BOOL)tabGridVisible {
return self.mainCoordinator.isTabGridActive;
}

- (UIViewController*)activeViewController {
return self.mainCoordinator.activeViewController;
}
Expand Down Expand Up @@ -711,7 +716,9 @@ - (void)transitionToSceneActivationLevel:(SceneActivationLevel)level
[self finishActivatingBrowserDismissingTabSwitcher:YES];
}

[self handleShowStartSurfaceIfNecessary];
if (!IsStartSurfaceSplashStartupEnabled()) {
[self handleShowStartSurfaceIfNecessary];
}
}

[self recordWindowCreationForSceneState:self.sceneState];
Expand Down Expand Up @@ -977,7 +984,9 @@ - (void)createInitialUI:(ApplicationMode)launchMode {
// currentInterface is set in case a new tab needs to be opened. Since this is
// synchronous with |setActivePage:| above, then the user should not see the
// last tab if the Start Surface is opened.
[self handleShowStartSurfaceIfNecessary];
if (!IsStartSurfaceSplashStartupEnabled()) {
[self handleShowStartSurfaceIfNecessary];
}

// Figure out what UI to show initially.

Expand Down Expand Up @@ -3039,7 +3048,8 @@ - (void)addANewTabAndPresentBrowser:(Browser*)browser
withURLLoadParams:(const UrlLoadParams&)urlLoadParams {
TabInsertionBrowserAgent::FromBrowser(browser)->InsertWebState(
urlLoadParams.web_params, nil, false, browser->GetWebStateList()->count(),
/*in_background=*/false, /*inherit_opener=*/false);
/*in_background=*/false, /*inherit_opener=*/false,
/*should_show_start_surface=*/false);
[self beginActivatingBrowser:browser dismissTabSwitcher:YES focusOmnibox:NO];
}

Expand Down
1 change: 1 addition & 0 deletions ios/chrome/browser/ui/ntp/BUILD.gn
Expand Up @@ -255,6 +255,7 @@ source_set("unit_tests") {
"//ios/chrome/browser/browser_state:test_support",
"//ios/chrome/browser/favicon",
"//ios/chrome/browser/main:test_support",
"//ios/chrome/browser/ntp",
"//ios/chrome/browser/ntp_snippets:ntp_snippets",
"//ios/chrome/browser/search_engines",
"//ios/chrome/browser/sessions",
Expand Down
Expand Up @@ -9,6 +9,7 @@
#include "ios/chrome/browser/browser_state/test_chrome_browser_state_manager.h"
#include "ios/chrome/browser/favicon/ios_chrome_large_icon_service_factory.h"
#include "ios/chrome/browser/main/test_browser.h"
#import "ios/chrome/browser/ntp/new_tab_page_tab_helper.h"
#include "ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.h"
#include "ios/chrome/browser/search_engines/template_url_service_factory.h"
#import "ios/chrome/browser/signin/authentication_service_factory.h"
Expand Down Expand Up @@ -72,6 +73,7 @@ void CreateCoordinator(bool off_the_record) {
scene_state_ = OCMClassMock([SceneState class]);
SceneStateBrowserAgent::CreateForBrowser(browser_.get(), scene_state_);
}
NewTabPageTabHelper::CreateForWebState(&web_state_);
coordinator_ = [[NewTabPageCoordinator alloc]
initWithBaseViewController:base_view_controller_
browser:browser_.get()];
Expand Down
2 changes: 2 additions & 0 deletions ios/chrome/browser/ui/start_surface/BUILD.gn
Expand Up @@ -37,11 +37,13 @@ source_set("start_surface") {
"//ios/chrome/app/application_delegate:app_state_header",
"//ios/chrome/app/strings:ios_strings",
"//ios/chrome/browser",
"//ios/chrome/browser/ntp",
"//ios/chrome/browser/ui:feature_flags",
"//ios/chrome/browser/ui/main:browser_interface_provider",
"//ios/chrome/browser/ui/main:observing_scene_agent",
"//ios/chrome/browser/ui/main:scene_state_header",
"//ios/chrome/browser/web_state_list",
"//ios/chrome/browser/web_state_list:agents",
"//ios/web/public",
"//ios/web/public:web_state_observer",
"//ui/base",
Expand Down
6 changes: 6 additions & 0 deletions ios/chrome/browser/ui/start_surface/start_surface_features.h
Expand Up @@ -15,13 +15,19 @@ extern const char kStartSurfaceReturnToRecentTabParam[];
// The feature to enable or disable the Start Surface.
extern const base::Feature kStartSurface;

// The feature to use the new logic to open a new tab to show the Start Surface.
extern const base::Feature kStartSurfaceSplashStartup;

// The feature parameter to indicate inactive duration to return to the Start
// Surface in seconds.
extern const char kReturnToStartSurfaceInactiveDurationInSeconds[];

// Checks whether the Start Surface should be enabled.
bool IsStartSurfaceEnabled();

// Checks whether the Start Splash Startup change should be enabled.
bool IsStartSurfaceSplashStartupEnabled();

// Returns the inactive duration to show the Start Surface.
double GetReturnToStartSurfaceDuration();

Expand Down
7 changes: 7 additions & 0 deletions ios/chrome/browser/ui/start_surface/start_surface_features.mm
Expand Up @@ -12,6 +12,9 @@
const base::Feature kStartSurface{"StartSurface",
base::FEATURE_ENABLED_BY_DEFAULT};

const base::Feature kStartSurfaceSplashStartup{
"StartSurfaceSplashStartup", base::FEATURE_ENABLED_BY_DEFAULT};

const char kReturnToStartSurfaceInactiveDurationInSeconds[] =
"ReturnToStartSurfaceInactiveDurationInSeconds";

Expand All @@ -23,6 +26,10 @@ bool IsStartSurfaceEnabled() {
return base::FeatureList::IsEnabled(kStartSurface);
}

bool IsStartSurfaceSplashStartupEnabled() {
return base::FeatureList::IsEnabled(kStartSurfaceSplashStartup);
}

double GetReturnToStartSurfaceDuration() {
return base::GetFieldTrialParamByFeatureAsDouble(
kStartSurface, kReturnToStartSurfaceInactiveDurationInSeconds,
Expand Down

0 comments on commit d0f6650

Please sign in to comment.