From d0f66506d0b0977fb054e04f22e46f64e175ec3a Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Mon, 2 May 2022 17:58:13 +0000 Subject: [PATCH] [ios] Create StartSurfaceSplashStartup 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 ef05477aa7436ee480989b21ac5094b31113a343) Bug: 1173160 Change-Id: I5754e4f7892ebd2207726118804e093e1488b8cd Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3582464 Reviewed-by: Mark Cogan Reviewed-by: Sergio Collazos Commit-Queue: Chris Lu Cr-Original-Commit-Position: refs/heads/main@{#995326} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3616868 Auto-Submit: Chris Lu Commit-Queue: Mark Cogan Cr-Commit-Position: refs/branch-heads/5005@{#347} Cr-Branched-From: 5b4d9450fee01f821b6400e947b3839727643a71-refs/heads/main@{#992738} --- ios/chrome/app/BUILD.gn | 6 +- ios/chrome/app/application_delegate/BUILD.gn | 1 + .../browser/ntp/new_tab_page_tab_helper.h | 8 ++ .../browser/ntp/new_tab_page_tab_helper.mm | 8 ++ ios/chrome/browser/ui/authentication/BUILD.gn | 2 +- ios/chrome/browser/ui/browser_view/BUILD.gn | 1 + .../browser_view/browser_view_controller.mm | 12 ++- .../content_suggestions_coordinator.mm | 17 +++- ios/chrome/browser/ui/first_run/BUILD.gn | 2 +- ios/chrome/browser/ui/main/BUILD.gn | 9 +- ios/chrome/browser/ui/main/scene_controller.h | 3 + .../browser/ui/main/scene_controller.mm | 16 +++- ios/chrome/browser/ui/ntp/BUILD.gn | 1 + .../ntp/new_tab_page_coordinator_unittest.mm | 2 + ios/chrome/browser/ui/start_surface/BUILD.gn | 2 + .../ui/start_surface/start_surface_features.h | 6 ++ .../start_surface/start_surface_features.mm | 7 ++ .../start_surface_scene_agent.mm | 64 +++++++++++++ .../url_loading/url_loading_browser_agent.mm | 3 +- .../web/web_state_delegate_browser_agent.mm | 5 +- ios/chrome/browser/web_state_list/BUILD.gn | 1 + .../tab_insertion_browser_agent.h | 3 +- .../tab_insertion_browser_agent.mm | 9 +- .../tab_insertion_browser_agent_unittest.mm | 92 +++++++++++-------- .../view_source_browser_agent.mm | 3 +- ios/chrome/test/app/BUILD.gn | 2 +- 26 files changed, 228 insertions(+), 57 deletions(-) diff --git a/ios/chrome/app/BUILD.gn b/ios/chrome/app/BUILD.gn index 9a37d1f4ecb223..8876046a2e431a 100644 --- a/ios/chrome/app/BUILD.gn +++ b/ios/chrome/app/BUILD.gn @@ -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", @@ -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", ] } @@ -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", ] } @@ -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", diff --git a/ios/chrome/app/application_delegate/BUILD.gn b/ios/chrome/app/application_delegate/BUILD.gn index 8c8412d75ce33a..33b5fed844a41b 100644 --- a/ios/chrome/app/application_delegate/BUILD.gn +++ b/ios/chrome/app/application_delegate/BUILD.gn @@ -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", ] } diff --git a/ios/chrome/browser/ntp/new_tab_page_tab_helper.h b/ios/chrome/browser/ntp/new_tab_page_tab_helper.h index 06e8c992fa74cd..4e55697a196d61 100644 --- a/ios/chrome/browser/ntp/new_tab_page_tab_helper.h +++ b/ios/chrome/browser/ntp/new_tab_page_tab_helper.h @@ -33,6 +33,10 @@ class NewTabPageTabHelper : public web::WebStateObserver, // Sets the delegate. The delegate is not owned by the tab helper. void SetDelegate(id 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; @@ -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; diff --git a/ios/chrome/browser/ntp/new_tab_page_tab_helper.mm b/ios/chrome/browser/ntp/new_tab_page_tab_helper.mm index b6b4e39fdf6f0f..b622952c17f4cd 100644 --- a/ios/chrome/browser/ntp/new_tab_page_tab_helper.mm +++ b/ios/chrome/browser/ntp/new_tab_page_tab_helper.mm @@ -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_; } diff --git a/ios/chrome/browser/ui/authentication/BUILD.gn b/ios/chrome/browser/ui/authentication/BUILD.gn index b64d53e85a773c..be162a5955e4be 100644 --- a/ios/chrome/browser/ui/authentication/BUILD.gn +++ b/ios/chrome/browser/ui/authentication/BUILD.gn @@ -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", diff --git a/ios/chrome/browser/ui/browser_view/BUILD.gn b/ios/chrome/browser/ui/browser_view/BUILD.gn index e343323ff45979..982251656f27a7 100644 --- a/ios/chrome/browser/ui/browser_view/BUILD.gn +++ b/ios/chrome/browser/ui/browser_view/BUILD.gn @@ -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", diff --git a/ios/chrome/browser/ui/browser_view/browser_view_controller.mm b/ios/chrome/browser/ui/browser_view/browser_view_controller.mm index 61aad823044e19..74c4a086d2f78d 100644 --- a/ios/chrome/browser/ui/browser_view/browser_view_controller.mm +++ b/ios/chrome/browser/ui/browser_view/browser_view_controller.mm @@ -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" @@ -4034,8 +4035,14 @@ - (void)webStateList:(WebStateList*)webStateList self /* id */); } + 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) @@ -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 diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_coordinator.mm b/ios/chrome/browser/ui/content_suggestions/content_suggestions_coordinator.mm index 1bdfc4e8223b38..7395446f184b45 100644 --- a/ios/chrome/browser/ui/content_suggestions/content_suggestions_coordinator.mm +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_coordinator.mm @@ -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" @@ -340,7 +341,6 @@ - (void)viewDidDisappear { DiscoverFeedServiceFactory::GetForBrowserState( self.browser->GetBrowserState()) ->SetIsShownOnStartSurface(false); - // Update DiscoverFeedService to NO if (ShouldShowReturnToMostRecentTabForStartSurface()) { [self.contentSuggestionsMediator hideRecentTabTile]; } @@ -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; @@ -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 diff --git a/ios/chrome/browser/ui/first_run/BUILD.gn b/ios/chrome/browser/ui/first_run/BUILD.gn index 13b8cceb01dbc6..148ba62e7f3b8a 100644 --- a/ios/chrome/browser/ui/first_run/BUILD.gn +++ b/ios/chrome/browser/ui/first_run/BUILD.gn @@ -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", diff --git a/ios/chrome/browser/ui/main/BUILD.gn b/ios/chrome/browser/ui/main/BUILD.gn index e6ff1761160c92..31292d17563f7e 100644 --- a/ios/chrome/browser/ui/main/BUILD.gn +++ b/ios/chrome/browser/ui/main/BUILD.gn @@ -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", ] } @@ -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", @@ -99,6 +102,7 @@ source_set("scene") { ":incognito_blocker_scene_agent", ":main", ":observing_scene_agent", + ":scene_state_header", ":scene_testing", "//base", "//base/ios", @@ -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", diff --git a/ios/chrome/browser/ui/main/scene_controller.h b/ios/chrome/browser/ui/main/scene_controller.h index 583c145e316772..5251e0a638a00b 100644 --- a/ios/chrome/browser/ui/main/scene_controller.h +++ b/ios/chrome/browser/ui/main/scene_controller.h @@ -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; diff --git a/ios/chrome/browser/ui/main/scene_controller.mm b/ios/chrome/browser/ui/main/scene_controller.mm index e9eac0d5c4a394..47b34987c39b15 100644 --- a/ios/chrome/browser/ui/main/scene_controller.mm +++ b/ios/chrome/browser/ui/main/scene_controller.mm @@ -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" @@ -361,6 +362,10 @@ - (BOOL)isPresentingSigninView { return self.signinCoordinator != nil; } +- (BOOL)tabGridVisible { + return self.mainCoordinator.isTabGridActive; +} + - (UIViewController*)activeViewController { return self.mainCoordinator.activeViewController; } @@ -711,7 +716,9 @@ - (void)transitionToSceneActivationLevel:(SceneActivationLevel)level [self finishActivatingBrowserDismissingTabSwitcher:YES]; } - [self handleShowStartSurfaceIfNecessary]; + if (!IsStartSurfaceSplashStartupEnabled()) { + [self handleShowStartSurfaceIfNecessary]; + } } [self recordWindowCreationForSceneState:self.sceneState]; @@ -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. @@ -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]; } diff --git a/ios/chrome/browser/ui/ntp/BUILD.gn b/ios/chrome/browser/ui/ntp/BUILD.gn index a17fc3d0c20152..9a26a06a22f932 100644 --- a/ios/chrome/browser/ui/ntp/BUILD.gn +++ b/ios/chrome/browser/ui/ntp/BUILD.gn @@ -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", diff --git a/ios/chrome/browser/ui/ntp/new_tab_page_coordinator_unittest.mm b/ios/chrome/browser/ui/ntp/new_tab_page_coordinator_unittest.mm index 5ef99adc2cec67..2cca29dd13a39d 100644 --- a/ios/chrome/browser/ui/ntp/new_tab_page_coordinator_unittest.mm +++ b/ios/chrome/browser/ui/ntp/new_tab_page_coordinator_unittest.mm @@ -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" @@ -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()]; diff --git a/ios/chrome/browser/ui/start_surface/BUILD.gn b/ios/chrome/browser/ui/start_surface/BUILD.gn index 36b4a2b27d10de..12e6612fc34839 100644 --- a/ios/chrome/browser/ui/start_surface/BUILD.gn +++ b/ios/chrome/browser/ui/start_surface/BUILD.gn @@ -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", diff --git a/ios/chrome/browser/ui/start_surface/start_surface_features.h b/ios/chrome/browser/ui/start_surface/start_surface_features.h index 4889cdee4e18a1..6ab4e7c37698fd 100644 --- a/ios/chrome/browser/ui/start_surface/start_surface_features.h +++ b/ios/chrome/browser/ui/start_surface/start_surface_features.h @@ -15,6 +15,9 @@ 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[]; @@ -22,6 +25,9 @@ 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(); diff --git a/ios/chrome/browser/ui/start_surface/start_surface_features.mm b/ios/chrome/browser/ui/start_surface/start_surface_features.mm index bb3c3625eac531..3aedc82d26173e 100644 --- a/ios/chrome/browser/ui/start_surface/start_surface_features.mm +++ b/ios/chrome/browser/ui/start_surface/start_surface_features.mm @@ -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"; @@ -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, diff --git a/ios/chrome/browser/ui/start_surface/start_surface_scene_agent.mm b/ios/chrome/browser/ui/start_surface/start_surface_scene_agent.mm index c1428eb38113de..e040c155d3bcf6 100644 --- a/ios/chrome/browser/ui/start_surface/start_surface_scene_agent.mm +++ b/ios/chrome/browser/ui/start_surface/start_surface_scene_agent.mm @@ -5,14 +5,23 @@ #import "ios/chrome/browser/ui/start_surface/start_surface_scene_agent.h" #include "base/feature_list.h" #include "base/metrics/histogram_macros.h" +#include "base/metrics/user_metrics.h" +#include "base/metrics/user_metrics_action.h" +#include "ios/chrome/browser/chrome_url_constants.h" #import "ios/chrome/browser/chrome_url_util.h" #import "ios/chrome/browser/main/browser.h" +#import "ios/chrome/browser/ntp/new_tab_page_tab_helper.h" #import "ios/chrome/browser/ui/main/browser_interface_provider.h" +#import "ios/chrome/browser/ui/main/scene_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_util.h" #include "ios/chrome/browser/ui/ui_feature_flags.h" +#import "ios/chrome/browser/web_state_list/tab_insertion_browser_agent.h" #import "ios/chrome/browser/web_state_list/web_state_list.h" #import "ios/web/public/navigation/navigation_manager.h" #import "ios/web/public/web_state.h" +#include "url/gurl.h" #if !defined(__has_feature) || !__has_feature(objc_arc) #error "This file requires ARC support." @@ -61,9 +70,64 @@ - (void)sceneState:(SceneState*)sceneState .incognitoInterface.browser]; } } + if (level >= SceneActivationLevelForegroundInactive && + self.previousActivationLevel < SceneActivationLevelForegroundInactive) { + if (IsStartSurfaceSplashStartupEnabled()) { + [self handleShowStartSurfaceIfNecessary]; + } + } self.previousActivationLevel = level; } +- (void)handleShowStartSurfaceIfNecessary { + if (!ShouldShowStartSurfaceForSceneState(self.sceneState)) { + return; + } + + // Do not show the Start Surface no matter whether it is enabled or not when + // the Tab grid is active by design. + if (self.sceneState.controller.isTabGridVisible) { + return; + } + + // If there is no active tab, a NTP will be added, and since there is no + // recent tab, there is no need to mark |modifytVisibleNTPForStartSurface|. + // Keep showing the last active NTP tab no matter whether the Start Surface is + // enabled or not by design. + // Note that activeWebState could only be nullptr when the Tab grid is active + // for now. + web::WebState* activeWebState = + self.sceneState.interfaceProvider.mainInterface.browser->GetWebStateList() + ->GetActiveWebState(); + if (!activeWebState || IsURLNtp(activeWebState->GetVisibleURL())) { + return; + } + + base::RecordAction(base::UserMetricsAction("IOS.StartSurface.Show")); + Browser* browser = self.sceneState.interfaceProvider.mainInterface.browser; + StartSurfaceRecentTabBrowserAgent::FromBrowser(browser)->SaveMostRecentTab(); + + // Activate the existing NTP tab for the Start surface. + WebStateList* webStateList = browser->GetWebStateList(); + for (int i = 0; i < webStateList->count(); i++) { + web::WebState* webState = webStateList->GetWebStateAt(i); + if (IsURLNtp(webState->GetVisibleURL())) { + NewTabPageTabHelper::FromWebState(webState)->SetShowStartSurface(true); + webStateList->ActivateWebStateAt(i); + return; + } + } + + // Create a new NTP since there is no existing one. + TabInsertionBrowserAgent* insertion_agent = + TabInsertionBrowserAgent::FromBrowser(browser); + web::NavigationManager::WebLoadParams params((GURL(kChromeUINewTabURL))); + insertion_agent->InsertWebState( + params, nullptr, /*opened_by_dom=*/false, + TabInsertion::kPositionAutomatically, /*in_background=*/false, + /*inherit_opener=*/false, /*should_show_start_surface=*/true); +} + // Removes duplicate NTP tabs in |browser|'s WebStateList. - (void)removeExcessNTPsInBrowser:(Browser*)browser { WebStateList* webStateList = browser->GetWebStateList(); diff --git a/ios/chrome/browser/url_loading/url_loading_browser_agent.mm b/ios/chrome/browser/url_loading/url_loading_browser_agent.mm index dbff879a871184..fc4a009d3d59c0 100644 --- a/ios/chrome/browser/url_loading/url_loading_browser_agent.mm +++ b/ios/chrome/browser/url_loading/url_loading_browser_agent.mm @@ -397,7 +397,8 @@ NOINLINE void InduceBrowserCrash(const GURL& url) { web::WebState* web_state = insertion_agent->InsertWebState( params.web_params, parent_web_state, /*opened_by_dom=*/false, - insertion_index, params.in_background(), params.inherit_opener); + insertion_index, params.in_background(), params.inherit_opener, + /*should_show_start_surface=*/false); web_state->GetNavigationManager()->LoadIfNecessary(); notifier_->NewTabDidLoadUrl(params.web_params.url, params.user_initiated); } diff --git a/ios/chrome/browser/web/web_state_delegate_browser_agent.mm b/ios/chrome/browser/web/web_state_delegate_browser_agent.mm index 8f3bcad5d160b8..93425ead34dc89 100644 --- a/ios/chrome/browser/web/web_state_delegate_browser_agent.mm +++ b/ios/chrome/browser/web/web_state_delegate_browser_agent.mm @@ -205,7 +205,7 @@ void OnHTTPAuthOverlayFinished(web::WebStateDelegate::AuthCallback callback, return tab_insertion_agent_->InsertWebState( load_params, source, false, TabInsertion::kPositionAutomatically, (params.disposition == WindowOpenDisposition::NEW_BACKGROUND_TAB), - /*inherit_opener=*/false); + /*inherit_opener=*/false, /*should_show_start_surface=*/false); } case WindowOpenDisposition::CURRENT_TAB: { source->GetNavigationManager()->LoadURLWithParams(load_params); @@ -214,7 +214,8 @@ void OnHTTPAuthOverlayFinished(web::WebStateDelegate::AuthCallback callback, case WindowOpenDisposition::NEW_POPUP: { return tab_insertion_agent_->InsertWebState( load_params, source, true, TabInsertion::kPositionAutomatically, - /*in_background=*/false, /*inherit_opener=*/false); + /*in_background=*/false, /*inherit_opener=*/false, + /*should_show_start_surface=*/false); } default: NOTIMPLEMENTED(); diff --git a/ios/chrome/browser/web_state_list/BUILD.gn b/ios/chrome/browser/web_state_list/BUILD.gn index 481aa1d25aa850..9220b1c6d497d5 100644 --- a/ios/chrome/browser/web_state_list/BUILD.gn +++ b/ios/chrome/browser/web_state_list/BUILD.gn @@ -78,6 +78,7 @@ source_set("agents") { "//ios/chrome/browser/browser_state_metrics", "//ios/chrome/browser/crash_report", "//ios/chrome/browser/main:public", + "//ios/chrome/browser/ntp", "//ios/chrome/browser/sessions:restoration_agent", "//ios/chrome/browser/sessions:restoration_observer", "//ios/chrome/browser/snapshots", diff --git a/ios/chrome/browser/web_state_list/tab_insertion_browser_agent.h b/ios/chrome/browser/web_state_list/tab_insertion_browser_agent.h index 6163945244d2c8..3e23aba7bbaf80 100644 --- a/ios/chrome/browser/web_state_list/tab_insertion_browser_agent.h +++ b/ios/chrome/browser/web_state_list/tab_insertion_browser_agent.h @@ -37,7 +37,8 @@ class TabInsertionBrowserAgent bool opened_by_dom, int index, bool in_background, - bool inherit_opener); + bool inherit_opener, + bool should_show_start_surface); web::WebState* InsertWebStateOpenedByDOM(web::WebState* parent); diff --git a/ios/chrome/browser/web_state_list/tab_insertion_browser_agent.mm b/ios/chrome/browser/web_state_list/tab_insertion_browser_agent.mm index d0b5e143f512bb..4912268f1edaa9 100644 --- a/ios/chrome/browser/web_state_list/tab_insertion_browser_agent.mm +++ b/ios/chrome/browser/web_state_list/tab_insertion_browser_agent.mm @@ -5,6 +5,7 @@ #import "ios/chrome/browser/web_state_list/tab_insertion_browser_agent.h" #include "ios/chrome/browser/browser_state/chrome_browser_state.h" +#import "ios/chrome/browser/ntp/new_tab_page_tab_helper.h" #import "ios/chrome/browser/web_state_list/web_state_list.h" #import "ios/chrome/browser/web_state_list/web_state_opener.h" #import "ios/web/public/web_state.h" @@ -27,7 +28,8 @@ bool opened_by_dom, int index, bool in_background, - bool inherit_opener) { + bool inherit_opener, + bool should_show_start_surface) { DCHECK(index == TabInsertion::kPositionAutomatically || (index >= 0 && index <= web_state_list_->count())); @@ -56,6 +58,11 @@ std::unique_ptr web_state = web::WebState::Create(create_params); + if (should_show_start_surface) { + NewTabPageTabHelper::CreateForWebState(web_state.get()); + NewTabPageTabHelper::FromWebState(web_state.get()) + ->SetShowStartSurface(true); + } web_state->GetNavigationManager()->LoadURLWithParams(params); int inserted_index = diff --git a/ios/chrome/browser/web_state_list/tab_insertion_browser_agent_unittest.mm b/ios/chrome/browser/web_state_list/tab_insertion_browser_agent_unittest.mm index e415eaa80e6804..65e02573bb9e60 100644 --- a/ios/chrome/browser/web_state_list/tab_insertion_browser_agent_unittest.mm +++ b/ios/chrome/browser/web_state_list/tab_insertion_browser_agent_unittest.mm @@ -53,35 +53,43 @@ } // namespace TEST_F(TabInsertionBrowserAgentTest, InsertUrlSingle) { - web::WebState* web_state = agent_->InsertWebState(Params(GURL(kURL1)), - /*parent=*/nil, - /*opened_by_dom=*/false, - /*index=*/0, - /*in_background=*/false, - /*inherit_opener=*/false); + web::WebState* web_state = + agent_->InsertWebState(Params(GURL(kURL1)), + /*parent=*/nil, + /*opened_by_dom=*/false, + /*index=*/0, + /*in_background=*/false, + /*inherit_opener=*/false, + /*should_show_start_surface=*/false); ASSERT_EQ(1, browser_->GetWebStateList()->count()); EXPECT_EQ(web_state, browser_->GetWebStateList()->GetWebStateAt(0)); } TEST_F(TabInsertionBrowserAgentTest, InsertUrlMultiple) { - web::WebState* web_state0 = agent_->InsertWebState(Params(GURL(kURL1)), - /*parent=*/nil, - /*opened_by_dom=*/false, - /*index=*/0, - /*in_background=*/false, - /*inherit_opener=*/false); - web::WebState* web_state1 = agent_->InsertWebState(Params(GURL(kURL1)), - /*parent=*/nil, - /*opened_by_dom=*/false, - /*index=*/0, - /*in_background=*/false, - /*inherit_opener=*/false); - web::WebState* web_state2 = agent_->InsertWebState(Params(GURL(kURL1)), - /*parent=*/nil, - /*opened_by_dom=*/false, - /*index=*/1, - /*in_background=*/false, - /*inherit_opener=*/false); + web::WebState* web_state0 = + agent_->InsertWebState(Params(GURL(kURL1)), + /*parent=*/nil, + /*opened_by_dom=*/false, + /*index=*/0, + /*in_background=*/false, + /*inherit_opener=*/false, + /*should_show_start_surface=*/false); + web::WebState* web_state1 = + agent_->InsertWebState(Params(GURL(kURL1)), + /*parent=*/nil, + /*opened_by_dom=*/false, + /*index=*/0, + /*in_background=*/false, + /*inherit_opener=*/false, + /*should_show_start_surface=*/false); + web::WebState* web_state2 = + agent_->InsertWebState(Params(GURL(kURL1)), + /*parent=*/nil, + /*opened_by_dom=*/false, + /*index=*/1, + /*in_background=*/false, + /*inherit_opener=*/false, + /*should_show_start_surface=*/false); ASSERT_EQ(3, browser_->GetWebStateList()->count()); EXPECT_EQ(web_state1, browser_->GetWebStateList()->GetWebStateAt(0)); @@ -96,7 +104,8 @@ /*opened_by_dom=*/false, /*index=*/browser_->GetWebStateList()->count(), /*in_background=*/false, - /*inherit_opener=*/false); + /*inherit_opener=*/false, + /*should_show_start_surface=*/false); ASSERT_EQ(1, browser_->GetWebStateList()->count()); EXPECT_EQ(web_state, browser_->GetWebStateList()->GetWebStateAt(0)); @@ -109,21 +118,24 @@ /*opened_by_dom=*/false, /*index=*/browser_->GetWebStateList()->count(), /*in_background=*/false, - /*inherit_opener=*/false); + /*inherit_opener=*/false, + /*should_show_start_surface=*/false); web::WebState* web_state1 = agent_->InsertWebState(Params(GURL(kURL1)), /*parent=*/nil, /*opened_by_dom=*/false, /*index=*/browser_->GetWebStateList()->count(), /*in_background=*/false, - /*inherit_opener=*/false); + /*inherit_opener=*/false, + /*should_show_start_surface=*/false); web::WebState* web_state2 = agent_->InsertWebState(Params(GURL(kURL1)), /*parent=*/nil, /*opened_by_dom=*/false, /*index=*/browser_->GetWebStateList()->count(), /*in_background=*/false, - /*inherit_opener=*/false); + /*inherit_opener=*/false, + /*should_show_start_surface=*/false); ASSERT_EQ(3, browser_->GetWebStateList()->count()); EXPECT_EQ(web_state0, browser_->GetWebStateList()->GetWebStateAt(0)); @@ -139,19 +151,22 @@ /*opened_by_dom=*/false, /*index=*/browser_->GetWebStateList()->count(), /*in_background=*/false, - /*inherit_opener=*/false); + /*inherit_opener=*/false, + /*should_show_start_surface=*/false); agent_->InsertWebState(Params(GURL(kURL1)), /*parent=*/nil, /*opened_by_dom=*/false, /*index=*/browser_->GetWebStateList()->count(), /*in_background=*/false, - /*inherit_opener=*/false); + /*inherit_opener=*/false, + /*should_show_start_surface=*/false); agent_->InsertWebState(Params(GURL(kURL1)), /*parent=*/nil, /*opened_by_dom=*/false, /*index=*/browser_->GetWebStateList()->count(), /*in_background=*/false, - /*inherit_opener=*/false); + /*inherit_opener=*/false, + /*should_show_start_surface=*/false); // Add a new tab, it should be added behind the parent. web::WebState* child = @@ -160,7 +175,8 @@ /*opened_by_dom=*/false, /*index=*/TabInsertion::kPositionAutomatically, /*in_background=*/false, - /*inherit_opener=*/false); + /*inherit_opener=*/false, + /*should_show_start_surface=*/false); EXPECT_EQ(browser_->GetWebStateList()->GetIndexOfWebState(parent), 0); EXPECT_EQ(browser_->GetWebStateList()->GetIndexOfWebState(child), 1); @@ -171,7 +187,8 @@ /*opened_by_dom=*/false, /*index=*/TabInsertion::kPositionAutomatically, /*in_background=*/false, - /*inherit_opener=*/false); + /*inherit_opener=*/false, + /*should_show_start_surface=*/false); EXPECT_EQ(browser_->GetWebStateList()->GetIndexOfWebState(web_state), browser_->GetWebStateList()->count() - 1); @@ -182,7 +199,8 @@ /*opened_by_dom=*/false, /*index=*/browser_->GetWebStateList()->count(), /*in_background=*/false, - /*inherit_opener=*/false); + /*inherit_opener=*/false, + /*should_show_start_surface=*/false); EXPECT_EQ(browser_->GetWebStateList()->GetIndexOfWebState(web_state2), browser_->GetWebStateList()->count() - 1); @@ -193,7 +211,8 @@ /*opened_by_dom=*/false, /*index=*/TabInsertion::kPositionAutomatically, /*in_background=*/false, - /*inherit_opener=*/false); + /*inherit_opener=*/false, + /*should_show_start_surface=*/false); EXPECT_EQ(browser_->GetWebStateList()->GetIndexOfWebState(web_state3), browser_->GetWebStateList()->GetIndexOfWebState(web_state) + 1); @@ -204,7 +223,8 @@ /*opened_by_dom=*/false, /*index=*/TabInsertion::kPositionAutomatically, /*in_background=*/false, - /*inherit_opener=*/false); + /*inherit_opener=*/false, + /*should_show_start_surface=*/false); EXPECT_EQ(browser_->GetWebStateList()->GetIndexOfWebState(web_state4), browser_->GetWebStateList()->GetIndexOfWebState(web_state3) + 1); } diff --git a/ios/chrome/browser/web_state_list/view_source_browser_agent.mm b/ios/chrome/browser/web_state_list/view_source_browser_agent.mm index 1c82828e4e47f6..4866538ca6af80 100644 --- a/ios/chrome/browser/web_state_list/view_source_browser_agent.mm +++ b/ios/chrome/browser/web_state_list/view_source_browser_agent.mm @@ -66,5 +66,6 @@ TabInsertionBrowserAgent::FromBrowser(browser_); insertionAgent->InsertWebState( loadParams, web_state, true, TabInsertion::kPositionAutomatically, - /*in_background=*/false, /*inherit_opener=*/false); + /*in_background=*/false, /*inherit_opener=*/false, + /*should_show_start_surface=*/false); } diff --git a/ios/chrome/test/app/BUILD.gn b/ios/chrome/test/app/BUILD.gn index 9ab213fa27a61f..36a8326536bf28 100644 --- a/ios/chrome/test/app/BUILD.gn +++ b/ios/chrome/test/app/BUILD.gn @@ -76,7 +76,7 @@ source_set("test_support") { "//ios/chrome/browser/ui/browser_view", "//ios/chrome/browser/ui/commands", "//ios/chrome/browser/ui/main", - "//ios/chrome/browser/ui/main:scene", + "//ios/chrome/browser/ui/main:scene_state_header", "//ios/chrome/browser/ui/main:scene_testing", "//ios/chrome/browser/ui/settings", "//ios/chrome/browser/ui/settings:settings_root",