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

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-Commit-Position: refs/heads/main@{#995326}
  • Loading branch information
Chris Lu authored and Chromium LUCI CQ committed Apr 22, 2022
1 parent c6cf6fa commit ef05477
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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 @@ -4043,8 +4044,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 @@ -4381,7 +4388,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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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 @@ -167,6 +171,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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@
#import "ios/chrome/browser/ui/ntp/new_tab_page_feature.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 @@ -364,6 +365,10 @@ - (BOOL)isPresentingSigninView {
return self.signinCoordinator != nil;
}

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

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

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

if (IsWebChannelsEnabled()) {
// Creating the DiscoverFeedService.
Expand Down Expand Up @@ -986,7 +993,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 @@ -3048,7 +3057,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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Loading

0 comments on commit ef05477

Please sign in to comment.