Commit 5ca77216 authored by Stepan Khapugin's avatar Stepan Khapugin Committed by Commit Bot

[multiball] Clean up content suggestion notifications logic

Creates new class ContentSuggestionsSchedulerAppAgent which brings
together all the Content Suggestion service notifications code.

Introduces AppStateAgents (app state associated objects).
Adds a new AppStateObserver method, sceneDidConnect:, that allows for
unified logic between iOS 12 and 13.

Bug: 1072366,1094916
Change-Id: Idd520e71668e6fc230b73eb3485085bf423b39bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2414223
Commit-Queue: Stepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: default avatarMark Cogan <marq@chromium.org>
Reviewed-by: default avatarGauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809925}
parent a079462e
......@@ -118,6 +118,22 @@ compile_entitlements("entitlements") {
output_name = "$target_gen_dir/$chromium_short_name.entitlements"
}
source_set("content_suggestions_scheduler_app_state_agent") {
configs += [ "//build/config/compiler:enable_arc" ]
sources = [
"content_suggestions_scheduler_app_state_agent.h",
"content_suggestions_scheduler_app_state_agent.mm",
]
deps = [
"//components/ntp_snippets",
"//ios/chrome/app/application_delegate:app_state_header",
"//ios/chrome/browser/metrics",
"//ios/chrome/browser/metrics:previous_session_info",
"//ios/chrome/browser/ntp_snippets",
"//ios/chrome/browser/ui/main:scene_state_header",
]
}
source_set("app_internal") {
configs += [ "//build/config/compiler:enable_arc" ]
sources = [
......@@ -139,6 +155,7 @@ source_set("app_internal") {
deps = [
":app",
":blocking_scene_commands",
":content_suggestions_scheduler_app_state_agent",
":mode",
":tests_hook",
"//base",
......@@ -159,7 +176,6 @@ source_set("app_internal") {
"//components/keyed_service/core",
"//components/keyed_service/ios",
"//components/metrics",
"//components/ntp_snippets",
"//components/password_manager/core/common",
"//components/prefs",
"//components/prefs/ios",
......@@ -201,7 +217,6 @@ source_set("app_internal") {
"//ios/chrome/browser/metrics:previous_session_info",
"//ios/chrome/browser/net",
"//ios/chrome/browser/ntp:features",
"//ios/chrome/browser/ntp_snippets",
"//ios/chrome/browser/omaha",
"//ios/chrome/browser/passwords",
"//ios/chrome/browser/reading_list",
......
......@@ -138,7 +138,10 @@ source_set("metric_kit_subscriber") {
source_set("app_state_header") {
configs += [ "//build/config/compiler:enable_arc" ]
sources = [ "app_state.h" ]
sources = [
"app_state.h",
"app_state_agent.h",
]
public_deps = [
"//ios/chrome/browser/ui/main:scene_state_observer",
"//ios/chrome/browser/ui/scoped_ui_blocker",
......
......@@ -7,6 +7,7 @@
#import <UIKit/UIKit.h>
#import "ios/chrome/app/application_delegate/app_state_agent.h"
#import "ios/chrome/browser/ui/main/scene_state_observer.h"
#import "ios/chrome/browser/ui/scoped_ui_blocker/ui_blocker_manager.h"
......@@ -27,6 +28,10 @@ class ChromeBrowserState;
@optional
// Called when a scene is connected.
// On iOS 12, called when the mainSceneState is set.
- (void)appState:(AppState*)appState sceneConnected:(SceneState*)sceneState;
// Called when the first scene becomes active.
- (void)appState:(AppState*)appState
firstSceneActivated:(SceneState*)sceneState;
......@@ -58,6 +63,9 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher
// The ChromeBrowserState associated with the main (non-OTR) browsing mode.
@property(nonatomic, assign) ChromeBrowserState* mainBrowserState;
// Container for startup information.
@property(nonatomic, weak) id<StartupInformation> startupInformation;
// YES if the user has ever interacted with the application. May be NO if the
// application has been woken up by the system for background work.
@property(nonatomic, readonly) BOOL userInteracted;
......@@ -146,6 +154,10 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher
// AppStateObserver callbacks.
- (void)removeObserver:(id<AppStateObserver>)observer;
// Adds a new agent. Agents are owned by the app state.
// This automatically sets the app state on the |agent|.
- (void)addAgent:(id<AppStateAgent>)agent;
@end
#endif // IOS_CHROME_APP_APPLICATION_DELEGATE_APP_STATE_H_
......@@ -40,7 +40,6 @@
#import "ios/chrome/browser/metrics/ios_profile_session_durations_service.h"
#import "ios/chrome/browser/metrics/ios_profile_session_durations_service_factory.h"
#import "ios/chrome/browser/metrics/previous_session_info.h"
#import "ios/chrome/browser/ntp_snippets/content_suggestions_scheduler_notifications.h"
#import "ios/chrome/browser/signin/authentication_service.h"
#import "ios/chrome/browser/signin/authentication_service_factory.h"
#import "ios/chrome/browser/ui/authentication/signed_in_accounts_view_controller.h"
......@@ -88,8 +87,6 @@ NSString* const kStartupAttemptReset = @"StartupAttempReset";
#pragma mark - AppState
@interface AppState () <SafeModeCoordinatorDelegate> {
// Container for startup information.
__weak id<StartupInformation> _startupInformation;
// Browser launcher to launch browser in different states.
__weak id<BrowserLauncher> _browserLauncher;
// UIApplicationDelegate for the application.
......@@ -149,6 +146,9 @@ NSString* const kStartupAttemptReset = @"StartupAttempReset";
// incrementBlockingUICounterForScene or the ScopedUIBlocker.
@property(nonatomic, assign) NSUInteger blockingUICounter;
// Agents attached to this app state.
@property(nonatomic, strong) NSMutableArray<id<AppStateAgent>>* agents;
@end
@implementation AppState
......@@ -165,16 +165,33 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher
if (self) {
_observers = [AppStateObserverList
observersWithProtocol:@protocol(AppStateObserver)];
_agents = [[NSMutableArray alloc] init];
_startupInformation = startupInformation;
_browserLauncher = browserLauncher;
_mainApplicationDelegate = applicationDelegate;
_appCommandDispatcher = [[CommandDispatcher alloc] init];
if (@available(iOS 13, *)) {
// Subscribe to scene connection notifications.
[[NSNotificationCenter defaultCenter]
addObserver:self
selector:@selector(sceneWillConnect:)
name:UISceneWillConnectNotification
object:nil];
}
}
return self;
}
#pragma mark - Properties implementation
- (void)setMainSceneState:(SceneState*)mainSceneState {
DCHECK(!_mainSceneState);
_mainSceneState = mainSceneState;
[self.observers appState:self sceneConnected:mainSceneState];
}
- (SafeModeCoordinator*)safeModeCoordinator {
return _safeModeCoordinator;
}
......@@ -194,11 +211,6 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher
}
}
- (void)setMainSceneState:(SceneState*)mainSceneState {
DCHECK(!_mainSceneState);
_mainSceneState = mainSceneState;
}
#pragma mark - Public methods.
- (void)applicationDidEnterBackground:(UIApplication*)application
......@@ -242,7 +254,7 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher
applicationDidEnterBackground:[memoryHelper
foregroundMemoryWarningCount]];
[_startupInformation expireFirstUserActionRecorder];
[self.startupInformation expireFirstUserActionRecorder];
// Do not save cookies if it is already in progress.
id<BrowserInterface> currentInterface =
......@@ -316,8 +328,9 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher
GetApplicationContext()->OnAppEnterForeground();
[MetricsMediator logLaunchMetricsWithStartupInformation:_startupInformation
connectedScenes:self.connectedScenes];
[MetricsMediator
logLaunchMetricsWithStartupInformation:self.startupInformation
connectedScenes:self.connectedScenes];
[memoryHelper resetForegroundMemoryWarningCount];
// If the current browser state is not OTR, check for cookie loss.
......@@ -364,7 +377,7 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher
[UserActivityHandler
handleStartupParametersWithTabOpener:tabOpener
connectionInformation:connectionInformation
startupInformation:_startupInformation
startupInformation:self.startupInformation
browserState:currentInterface.browserState];
} else if ([tabOpener shouldOpenNTPTabOnActivationOfBrowser:currentInterface
.browser]) {
......@@ -388,7 +401,7 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher
if (psdService)
psdService->OnSessionStarted(_sessionStartTime);
[MetricsMediator logStartupDuration:_startupInformation
[MetricsMediator logStartupDuration:self.startupInformation
connectionInformation:connectionInformation];
}
......@@ -427,7 +440,7 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher
self.mainSceneState.activationLevel = SceneActivationLevelUnattached;
}
[_startupInformation stopChromeMain];
[self.startupInformation stopChromeMain];
}
- (void)application:(UIApplication*)application
......@@ -456,9 +469,9 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher
return;
}
// Set [_startupInformation isColdStart] to NO in anticipation of the next
// Set [self.startupInformation isColdStart] to NO in anticipation of the next
// time the app becomes active.
[_startupInformation setIsColdStart:NO];
[self.startupInformation setIsColdStart:NO];
id<BrowserInterface> currentInterface =
_browserLauncher.interfaceProvider.currentInterface;
......@@ -507,6 +520,12 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher
[self.observers removeObserver:observer];
}
- (void)addAgent:(id<AppStateAgent>)agent {
DCHECK(agent);
[self.agents addObject:agent];
[agent setAppState:self];
}
#pragma mark - Multiwindow-related
- (SceneState*)foregroundActiveScene {
......@@ -614,7 +633,7 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher
// Don't add code here. Add it in MainController's
// -startUpBrowserForegroundInitialization.
DCHECK([_startupInformation isColdStart]);
DCHECK([self.startupInformation isColdStart]);
[_browserLauncher startUpBrowserToStage:INITIALIZATION_STAGE_FOREGROUND];
}
......@@ -676,4 +695,17 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher
}
}
- (void)sceneWillConnect:(NSNotification*)notification {
if (@available(iOS 13, *)) {
UIWindowScene* scene =
base::mac::ObjCCastStrict<UIWindowScene>(notification.object);
SceneDelegate* sceneDelegate =
base::mac::ObjCCastStrict<SceneDelegate>(scene.delegate);
SceneState* sceneState = sceneDelegate.sceneState;
DCHECK(sceneState);
[self.observers appState:self sceneConnected:sceneState];
}
}
@end
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef IOS_CHROME_APP_APPLICATION_DELEGATE_APP_STATE_AGENT_H_
#define IOS_CHROME_APP_APPLICATION_DELEGATE_APP_STATE_AGENT_H_
#import <UIKit/UIKit.h>
@class AppState;
// AppState agents are objects owned by the app state and providing some
// app-scoped function. They can be driven by AppStateObserver events.
@protocol AppStateAgent <NSObject>
@required
// Sets the associated app state. Called once and only once. Consider starting
// the app state observation in your implementation of this method.
// Do not call this method directly. Calling [AppState addAgent]: will call it.
- (void)setAppState:(AppState*)appState;
@end
#endif // IOS_CHROME_APP_APPLICATION_DELEGATE_APP_STATE_AGENT_H_
......@@ -28,7 +28,6 @@
#import "ios/chrome/browser/main/test_browser.h"
#import "ios/chrome/browser/metrics/ios_profile_session_durations_service.h"
#import "ios/chrome/browser/metrics/ios_profile_session_durations_service_factory.h"
#import "ios/chrome/browser/ntp_snippets/content_suggestions_scheduler_notifications.h"
#include "ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.h"
#import "ios/chrome/browser/signin/authentication_service_factory.h"
#import "ios/chrome/browser/signin/authentication_service_fake.h"
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef IOS_CHROME_APP_CONTENT_SUGGESTIONS_SCHEDULER_APP_STATE_AGENT_H_
#define IOS_CHROME_APP_CONTENT_SUGGESTIONS_SCHEDULER_APP_STATE_AGENT_H_
#import "ios/chrome/app/application_delegate/app_state_agent.h"
// The agent that notifies the content suggestions service about app lifecycle
// events to keep the model up to date.
@interface ContentSuggestionsSchedulerAppAgent : NSObject <AppStateAgent>
@end
#endif // IOS_CHROME_APP_CONTENT_SUGGESTIONS_SCHEDULER_APP_STATE_AGENT_H_
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#import "ios/chrome/app/content_suggestions_scheduler_app_state_agent.h"
#include "components/ntp_snippets/content_suggestions_service.h"
#import "ios/chrome/app/application_delegate/app_state.h"
#import "ios/chrome/browser/metrics/previous_session_info.h"
#include "ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.h"
#import "ios/chrome/browser/ui/main/scene_state.h"
#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
#endif
@interface ContentSuggestionsSchedulerAppAgent () <AppStateObserver,
SceneStateObserver>
// Observed app state.
@property(nonatomic, weak) AppState* appState;
// Flag to keep track if we notified the service once at the cold app start.
@property(nonatomic, assign) BOOL hasNotifiedColdStart;
@end
@implementation ContentSuggestionsSchedulerAppAgent
#pragma mark - AppStateAgent
- (void)setAppState:(AppState*)appState {
// This should only be called once!
DCHECK(!_appState);
_appState = appState;
[appState addObserver:self];
}
#pragma mark - AppStateObserver
- (void)appState:(AppState*)appState sceneConnected:(SceneState*)sceneState {
[sceneState addObserver:self];
}
#pragma mark - SceneStateObserver
- (void)sceneState:(SceneState*)sceneState
transitionedToActivationLevel:(SceneActivationLevel)level {
if (level >= SceneActivationLevelForegroundInactive) {
[self notifyForeground];
}
}
#pragma mark - private
// Convenience getter for the content suggestion service.
- (ntp_snippets::ContentSuggestionsService*)service {
if (!self.appState.mainBrowserState) {
return nil;
}
return IOSChromeContentSuggestionsServiceFactory::GetForBrowserState(
self.appState.mainBrowserState);
}
// Notify the serivce about the cold start.
- (void)notifyColdStart {
if ([self service]) {
if ([PreviousSessionInfo sharedInstance]
.isFirstSessionAfterLanguageChange) {
[self service]->ClearAllCachedSuggestions();
}
[self service]->remote_suggestions_scheduler() -> OnBrowserColdStart();
}
}
// Notify the serivce when the app is brought to foreground. The first time this
// happens, also notify about a cold start.
- (void)notifyForeground {
if (!self.hasNotifiedColdStart) {
[self notifyColdStart];
self.hasNotifiedColdStart = YES;
}
if ([self service]) {
[self service]->remote_suggestions_scheduler() -> OnBrowserForegrounded();
}
}
@end
......@@ -78,7 +78,6 @@
startupInformation:_startupInformation
applicationDelegate:self];
[_mainController setAppState:_appState];
[_appState addObserver:_mainController];
if (!IsSceneStartupSupported()) {
// When the UIScene APU is not supported, this object holds a "scene"
......
......@@ -18,7 +18,6 @@
#include "components/feature_engagement/public/tracker.h"
#include "components/metrics/metrics_pref_names.h"
#include "components/metrics/metrics_service.h"
#include "components/ntp_snippets/content_suggestions_service.h"
#include "components/password_manager/core/common/passwords_directory_util_ios.h"
#include "components/prefs/ios/pref_observer_bridge.h"
#include "components/prefs/pref_change_registrar.h"
......@@ -26,6 +25,7 @@
#include "components/web_resource/web_resource_pref_names.h"
#import "ios/chrome/app/application_delegate/metrics_mediator.h"
#import "ios/chrome/app/blocking_scene_commands.h"
#import "ios/chrome/app/content_suggestions_scheduler_app_state_agent.h"
#import "ios/chrome/app/deferred_initialization_runner.h"
#import "ios/chrome/app/memory_monitor.h"
#import "ios/chrome/app/spotlight/spotlight_manager.h"
......@@ -68,8 +68,6 @@
#include "ios/chrome/browser/metrics/first_user_action_recorder.h"
#import "ios/chrome/browser/metrics/previous_session_info.h"
#import "ios/chrome/browser/net/cookie_util.h"
#import "ios/chrome/browser/ntp_snippets/content_suggestions_scheduler_notifications.h"
#include "ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.h"
#import "ios/chrome/browser/omaha/omaha_service.h"
#include "ios/chrome/browser/pref_names.h"
#import "ios/chrome/browser/screenshot/screenshot_metrics_recorder.h"
......@@ -520,12 +518,6 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData(
self.appState.mainBrowserState);
}
if ([PreviousSessionInfo sharedInstance].isFirstSessionAfterLanguageChange) {
IOSChromeContentSuggestionsServiceFactory::GetForBrowserState(
chromeBrowserState)
->ClearAllCachedSuggestions();
}
return needRestoration;
}
......@@ -546,12 +538,6 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData(
[MetricsMediator
logLaunchMetricsWithStartupInformation:self
connectedScenes:self.appState.connectedScenes];
if (self.isColdStart) {
[ContentSuggestionsSchedulerNotifications
notifyColdStart:self.appState.mainBrowserState];
[ContentSuggestionsSchedulerNotifications
notifyForeground:self.appState.mainBrowserState];
}
ios::GetChromeBrowserProvider()->GetOverridesProvider()->InstallOverrides();
......@@ -630,6 +616,15 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData(
#pragma mark - Property implementation.
- (void)setAppState:(AppState*)appState {
DCHECK(!_appState);
_appState = appState;
[appState addObserver:self];
// Create app state agents.
[appState addAgent:[[ContentSuggestionsSchedulerAppAgent alloc] init]];
}
- (id<BrowserInterfaceProvider>)interfaceProvider {
if (self.appState.foregroundActiveScene) {
return self.appState.foregroundActiveScene.interfaceProvider;
......
......@@ -5,8 +5,6 @@
source_set("ntp_snippets") {
configs += [ "//build/config/compiler:enable_arc" ]
sources = [
"content_suggestions_scheduler_notifications.h",
"content_suggestions_scheduler_notifications.mm",
"ios_chrome_content_suggestions_service_factory.cc",
"ios_chrome_content_suggestions_service_factory.h",
"ios_chrome_content_suggestions_service_factory_util.cc",
......
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef IOS_CHROME_BROWSER_NTP_SNIPPETS_CONTENT_SUGGESTIONS_SCHEDULER_NOTIFICATIONS_H_
#define IOS_CHROME_BROWSER_NTP_SNIPPETS_CONTENT_SUGGESTIONS_SCHEDULER_NOTIFICATIONS_H_
#import <Foundation/Foundation.h>
class ChromeBrowserState;
// Notify the scheduler of the Content Suggestions services of the app lifecycle
// events.
@interface ContentSuggestionsSchedulerNotifications : NSObject
// Notifies that the application is launching from cold state.
+ (void)notifyColdStart:(ChromeBrowserState*)browserState;
// Notifies that the application has been foregrounded.
+ (void)notifyForeground:(ChromeBrowserState*)browserState;
@end
#endif // IOS_CHROME_BROWSER_NTP_SNIPPETS_CONTENT_SUGGESTIONS_SCHEDULER_NOTIFICATIONS_H_
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#import "ios/chrome/browser/ntp_snippets/content_suggestions_scheduler_notifications.h"
#include "components/ntp_snippets/content_suggestions_service.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
#include "ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.h"
#include "ios/chrome/browser/system_flags.h"
#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
#endif
@implementation ContentSuggestionsSchedulerNotifications
+ (void)notifyColdStart:(ChromeBrowserState*)browserState {
ntp_snippets::ContentSuggestionsService* contentSuggestionsService =
IOSChromeContentSuggestionsServiceFactory::GetForBrowserState(
browserState);
contentSuggestionsService->remote_suggestions_scheduler()
->OnBrowserColdStart();
}
+ (void)notifyForeground:(ChromeBrowserState*)browserState {
ntp_snippets::ContentSuggestionsService* contentSuggestionsService =
IOSChromeContentSuggestionsServiceFactory::GetForBrowserState(
browserState);
contentSuggestionsService->remote_suggestions_scheduler()
->OnBrowserForegrounded();
}
@end
......@@ -86,7 +86,6 @@ source_set("scene") {
"//ios/chrome/browser/main",
"//ios/chrome/browser/metrics:previous_session_info",
"//ios/chrome/browser/ntp:features",
"//ios/chrome/browser/ntp_snippets:ntp_snippets",
"//ios/chrome/browser/screenshot",
"//ios/chrome/browser/signin",
"//ios/chrome/browser/snapshots",
......
......@@ -40,7 +40,6 @@
#import "ios/chrome/browser/main/browser_util.h"
#import "ios/chrome/browser/metrics/previous_session_info.h"
#include "ios/chrome/browser/ntp/features.h"
#import "ios/chrome/browser/ntp_snippets/content_suggestions_scheduler_notifications.h"
#include "ios/chrome/browser/screenshot/screenshot_delegate.h"
#include "ios/chrome/browser/signin/constants.h"
#include "ios/chrome/browser/signin/identity_manager_factory.h"
......@@ -311,13 +310,6 @@ const char kMultiWindowOpenInNewWindowHistogram[] =
if (![self presentSigninUpgradePromoIfPossible]) {
[self presentSignInAccountsViewControllerIfNecessary];
}
// Mitigation for crbug.com/1092326, where a nil browser state is passed
// (presumably because mainInterface is nil as well).
// TODO(crbug.com/1094916): Handle this more cleanly.
if (self.mainInterface.browserState) {
[ContentSuggestionsSchedulerNotifications
notifyForeground:self.mainInterface.browserState];
}
[self handleExternalIntents];
......
......@@ -25,9 +25,6 @@ class SceneControllerTest : public PlatformTest {
SceneState* scene_state_;
};
// TODO(crbug.com/1072366): Add a test for
// ContentSuggestionsSchedulerNotifications receiving notifyForeground:.
// TODO(crbug.com/1084905): Add a test for keeping validity of detecting a fresh
// open in new window coming from ios dock. 'Dock' is considered the default
// when the new window opening request is external to chrome and unknown.
......
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