Commit 7940bca7 authored by Olivier Robin's avatar Olivier Robin Committed by Commit Bot

Report NewTabPage.TimeSpent on iOS

Metric reports the time a NTP is on screen.
Incognito NTP are not reported.

Bug: 1131209
Change-Id: I25fb6bb004efd89cbef855e2455c71f616986e55
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2487105
Commit-Queue: Olivier Robin <olivierrobin@chromium.org>
Reviewed-by: default avatarOlivier Robin <olivierrobin@chromium.org>
Reviewed-by: default avatarGauthier Ambard <gambard@chromium.org>
Reviewed-by: default avatarRobert Kaplow <rkaplow@chromium.org>
Reviewed-by: default avatarChris Lu <thegreenfrog@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824131}
parent 7e3f354c
...@@ -2413,6 +2413,7 @@ NSString* const kBrowserViewControllerSnackbarCategory = ...@@ -2413,6 +2413,7 @@ NSString* const kBrowserViewControllerSnackbarCategory =
if (NTPHelper && NTPHelper->IsActive()) { if (NTPHelper && NTPHelper->IsActive()) {
UIViewController* viewController = UIViewController* viewController =
_ntpCoordinatorsForWebStates[webState].viewController; _ntpCoordinatorsForWebStates[webState].viewController;
[_ntpCoordinatorsForWebStates[webState] ntpDidChangeVisibility:YES];
viewController.view.frame = [self ntpFrameForWebState:webState]; viewController.view.frame = [self ntpFrameForWebState:webState];
[viewController.view layoutIfNeeded]; [viewController.view layoutIfNeeded];
// TODO(crbug.com/873729): For a newly created WebState, the session will // TODO(crbug.com/873729): For a newly created WebState, the session will
...@@ -4437,6 +4438,7 @@ NSString* const kBrowserViewControllerSnackbarCategory = ...@@ -4437,6 +4438,7 @@ NSString* const kBrowserViewControllerSnackbarCategory =
if (oldWebState) { if (oldWebState) {
oldWebState->WasHidden(); oldWebState->WasHidden();
oldWebState->SetKeepRenderProcessAlive(false); oldWebState->SetKeepRenderProcessAlive(false);
[_ntpCoordinatorsForWebStates[oldWebState] ntpDidChangeVisibility:NO];
[self dismissPopups]; [self dismissPopups];
} }
// NOTE: webStateSelected expects to always be called with a // NOTE: webStateSelected expects to always be called with a
...@@ -4445,6 +4447,7 @@ NSString* const kBrowserViewControllerSnackbarCategory = ...@@ -4445,6 +4447,7 @@ NSString* const kBrowserViewControllerSnackbarCategory =
return; return;
self.currentWebState->GetWebViewProxy().scrollViewProxy.clipsToBounds = NO; self.currentWebState->GetWebViewProxy().scrollViewProxy.clipsToBounds = NO;
[_ntpCoordinatorsForWebStates[newWebState] ntpDidChangeVisibility:YES];
[self webStateSelected:newWebState notifyToolbar:YES]; [self webStateSelected:newWebState notifyToolbar:YES];
} }
......
...@@ -25,6 +25,8 @@ source_set("coordinator") { ...@@ -25,6 +25,8 @@ source_set("coordinator") {
"//ios/chrome/browser/main:public", "//ios/chrome/browser/main:public",
"//ios/chrome/browser/ui/content_suggestions", "//ios/chrome/browser/ui/content_suggestions",
"//ios/chrome/browser/ui/coordinators:chrome_coordinators", "//ios/chrome/browser/ui/coordinators:chrome_coordinators",
"//ios/chrome/browser/ui/main:scene_state_header",
"//ios/chrome/browser/ui/main:scene_state_observer",
"//ios/chrome/browser/url_loading", "//ios/chrome/browser/url_loading",
"//ios/chrome/browser/web_state_list", "//ios/chrome/browser/web_state_list",
"//ios/public/provider/chrome/browser/voice", "//ios/public/provider/chrome/browser/voice",
...@@ -175,6 +177,7 @@ source_set("unit_tests") { ...@@ -175,6 +177,7 @@ source_set("unit_tests") {
"//ios/chrome/browser/ui/content_suggestions:content_suggestions_constant", "//ios/chrome/browser/ui/content_suggestions:content_suggestions_constant",
"//ios/chrome/browser/ui/content_suggestions:content_suggestions_ui", "//ios/chrome/browser/ui/content_suggestions:content_suggestions_ui",
"//ios/chrome/browser/ui/favicon", "//ios/chrome/browser/ui/favicon",
"//ios/chrome/browser/ui/main:scene_state_header",
"//ios/chrome/browser/web_state_list:test_support", "//ios/chrome/browser/web_state_list:test_support",
"//ios/chrome/browser/web_state_list:web_state_list", "//ios/chrome/browser/web_state_list:web_state_list",
"//ios/chrome/common/app_group", "//ios/chrome/common/app_group",
...@@ -203,10 +206,12 @@ source_set("eg2_tests") { ...@@ -203,10 +206,12 @@ source_set("eg2_tests") {
"//base/test:test_support", "//base/test:test_support",
"//components/strings", "//components/strings",
"//ios/chrome/app/strings", "//ios/chrome/app/strings",
"//ios/chrome/browser/metrics:eg_test_support+eg2",
"//ios/chrome/browser/ui/content_suggestions:content_suggestions_constant", "//ios/chrome/browser/ui/content_suggestions:content_suggestions_constant",
"//ios/chrome/test/earl_grey:eg_test_support+eg2", "//ios/chrome/test/earl_grey:eg_test_support+eg2",
"//ios/testing/earl_grey:eg_test_support+eg2", "//ios/testing/earl_grey:eg_test_support+eg2",
"//ios/third_party/earl_grey2:test_lib", "//ios/third_party/earl_grey2:test_lib",
"//net:test_support",
"//ui/base", "//ui/base",
] ]
frameworks = [ "UIKit.framework" ] frameworks = [ "UIKit.framework" ]
......
...@@ -57,6 +57,9 @@ class WebState; ...@@ -57,6 +57,9 @@ class WebState;
// Reloads the content of the NewTabPage. // Reloads the content of the NewTabPage.
- (void)reload; - (void)reload;
// Calls when the visibility of the NTP changes.
- (void)ntpDidChangeVisibility:(BOOL)visible;
// The location bar has lost focus. // The location bar has lost focus.
- (void)locationBarDidResignFirstResponder; - (void)locationBarDidResignFirstResponder;
......
...@@ -4,12 +4,16 @@ ...@@ -4,12 +4,16 @@
#import "ios/chrome/browser/ui/ntp/new_tab_page_coordinator.h" #import "ios/chrome/browser/ui/ntp/new_tab_page_coordinator.h"
#include "base/metrics/histogram_functions.h"
#include "base/metrics/user_metrics.h" #include "base/metrics/user_metrics.h"
#include "base/metrics/user_metrics_action.h" #include "base/metrics/user_metrics_action.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h" #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
#import "ios/chrome/browser/main/browser.h" #import "ios/chrome/browser/main/browser.h"
#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_coordinator.h" #import "ios/chrome/browser/ui/content_suggestions/content_suggestions_coordinator.h"
#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_header_view_controller.h" #import "ios/chrome/browser/ui/content_suggestions/content_suggestions_header_view_controller.h"
#import "ios/chrome/browser/ui/main/scene_state.h"
#import "ios/chrome/browser/ui/main/scene_state_browser_agent.h"
#import "ios/chrome/browser/ui/main/scene_state_observer.h"
#import "ios/chrome/browser/ui/ntp/incognito_view_controller.h" #import "ios/chrome/browser/ui/ntp/incognito_view_controller.h"
#import "ios/chrome/browser/url_loading/url_loading_browser_agent.h" #import "ios/chrome/browser/url_loading/url_loading_browser_agent.h"
#import "ios/web/public/navigation/navigation_context.h" #import "ios/web/public/navigation/navigation_context.h"
...@@ -21,7 +25,7 @@ ...@@ -21,7 +25,7 @@
#error "This file requires ARC support." #error "This file requires ARC support."
#endif #endif
@interface NewTabPageCoordinator () @interface NewTabPageCoordinator () <SceneStateObserver>
// Coordinator for the ContentSuggestions. // Coordinator for the ContentSuggestions.
@property(nonatomic, strong) @property(nonatomic, strong)
...@@ -30,6 +34,20 @@ ...@@ -30,6 +34,20 @@
// View controller for incognito. // View controller for incognito.
@property(nonatomic, strong) IncognitoViewController* incognitoViewController; @property(nonatomic, strong) IncognitoViewController* incognitoViewController;
// The timetick of the last time the NTP was displayed.
@property(nonatomic, assign) base::TimeTicks didAppearTime;
// Tracks the visibility of the NTP to report NTP usage metrics.
// True if the NTP view is currently displayed to the user.
@property(nonatomic, assign) BOOL visible;
// Whether the view is new tab view is currently presented (possibly in
// background). Used to report NTP usage metrics.
@property(nonatomic, assign) BOOL viewPresented;
// Wheter the scene is currently in foreground.
@property(nonatomic, assign) BOOL sceneInForeground;
@end @end
@implementation NewTabPageCoordinator @implementation NewTabPageCoordinator
...@@ -64,6 +82,11 @@ ...@@ -64,6 +82,11 @@
[self.contentSuggestionsCoordinator start]; [self.contentSuggestionsCoordinator start];
base::RecordAction(base::UserMetricsAction("MobileNTPShowMostVisited")); base::RecordAction(base::UserMetricsAction("MobileNTPShowMostVisited"));
SceneState* sceneState =
SceneStateBrowserAgent::FromBrowser(self.browser)->GetSceneState();
[sceneState addObserver:self];
self.sceneInForeground =
sceneState.activationLevel >= SceneActivationLevelForegroundInactive;
} }
self.started = YES; self.started = YES;
...@@ -72,12 +95,41 @@ ...@@ -72,12 +95,41 @@
- (void)stop { - (void)stop {
if (!self.started) if (!self.started)
return; return;
self.viewPresented = NO;
[self updateVisible];
[self.contentSuggestionsCoordinator stop]; [self.contentSuggestionsCoordinator stop];
SceneState* sceneState =
SceneStateBrowserAgent::FromBrowser(self.browser)->GetSceneState();
[sceneState removeObserver:self];
self.contentSuggestionsCoordinator = nil; self.contentSuggestionsCoordinator = nil;
self.incognitoViewController = nil; self.incognitoViewController = nil;
self.started = NO; self.started = NO;
} }
// Updates the visible property based on viewPresented and sceneInForeground
// properties.
// Sends metrics when NTP becomes invisible.
- (void)updateVisible {
BOOL visible = self.viewPresented && self.sceneInForeground;
if (visible == self.visible) {
return;
}
self.visible = visible;
if (self.browser->GetBrowserState()->IsOffTheRecord()) {
// Do not report metrics on incognito NTP.
return;
}
if (visible) {
self.didAppearTime = base::TimeTicks::Now();
} else {
if (!self.didAppearTime.is_null()) {
UmaHistogramMediumTimes("NewTabPage.TimeSpent",
base::TimeTicks::Now() - self.didAppearTime);
self.didAppearTime = base::TimeTicks();
}
}
}
#pragma mark - Properties #pragma mark - Properties
- (UIViewController*)viewController { - (UIViewController*)viewController {
...@@ -128,6 +180,11 @@ ...@@ -128,6 +180,11 @@
constrainDiscoverHeaderMenuButtonNamedGuide]; constrainDiscoverHeaderMenuButtonNamedGuide];
} }
- (void)ntpDidChangeVisibility:(BOOL)visible {
self.viewPresented = visible;
[self updateVisible];
}
#pragma mark - LogoAnimationControllerOwnerOwner #pragma mark - LogoAnimationControllerOwnerOwner
- (id<LogoAnimationControllerOwner>)logoAnimationControllerOwner { - (id<LogoAnimationControllerOwner>)logoAnimationControllerOwner {
...@@ -135,4 +192,12 @@ ...@@ -135,4 +192,12 @@
.headerController logoAnimationControllerOwner]; .headerController logoAnimationControllerOwner];
} }
#pragma mark - SceneStateObserver
- (void)sceneState:(SceneState*)sceneState
transitionedToActivationLevel:(SceneActivationLevel)level {
self.sceneInForeground = level >= SceneActivationLevelForegroundInactive;
[self updateVisible];
}
@end @end
...@@ -14,6 +14,8 @@ ...@@ -14,6 +14,8 @@
#import "ios/chrome/browser/ui/commands/command_dispatcher.h" #import "ios/chrome/browser/ui/commands/command_dispatcher.h"
#import "ios/chrome/browser/ui/commands/snackbar_commands.h" #import "ios/chrome/browser/ui/commands/snackbar_commands.h"
#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.h" #import "ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.h"
#import "ios/chrome/browser/ui/main/scene_state.h"
#import "ios/chrome/browser/ui/main/scene_state_browser_agent.h"
#import "ios/chrome/browser/ui/ntp/incognito_view_controller.h" #import "ios/chrome/browser/ui/ntp/incognito_view_controller.h"
#import "ios/chrome/browser/ui/ntp/new_tab_page_controller_delegate.h" #import "ios/chrome/browser/ui/ntp/new_tab_page_controller_delegate.h"
#include "ios/chrome/test/ios_chrome_scoped_testing_chrome_browser_state_manager.h" #include "ios/chrome/test/ios_chrome_scoped_testing_chrome_browser_state_manager.h"
...@@ -59,6 +61,8 @@ class NewTabPageCoordinatorTest : public PlatformTest { ...@@ -59,6 +61,8 @@ class NewTabPageCoordinatorTest : public PlatformTest {
[[NewTabPageCoordinator alloc] initWithBrowser:browser_.get()]; [[NewTabPageCoordinator alloc] initWithBrowser:browser_.get()];
} else { } else {
browser_ = std::make_unique<TestBrowser>(browser_state_.get()); browser_ = std::make_unique<TestBrowser>(browser_state_.get());
scene_state_ = OCMClassMock([SceneState class]);
SceneStateBrowserAgent::CreateForBrowser(browser_.get(), scene_state_);
coordinator_ = coordinator_ =
[[NewTabPageCoordinator alloc] initWithBrowser:browser_.get()]; [[NewTabPageCoordinator alloc] initWithBrowser:browser_.get()];
} }
...@@ -73,6 +77,7 @@ class NewTabPageCoordinatorTest : public PlatformTest { ...@@ -73,6 +77,7 @@ class NewTabPageCoordinatorTest : public PlatformTest {
IOSChromeScopedTestingChromeBrowserStateManager scoped_browser_state_manager_; IOSChromeScopedTestingChromeBrowserStateManager scoped_browser_state_manager_;
std::unique_ptr<TestChromeBrowserState> browser_state_; std::unique_ptr<TestChromeBrowserState> browser_state_;
std::unique_ptr<Browser> browser_; std::unique_ptr<Browser> browser_;
id scene_state_;
NewTabPageCoordinator* coordinator_; NewTabPageCoordinator* coordinator_;
}; };
......
...@@ -5,12 +5,16 @@ ...@@ -5,12 +5,16 @@
#import "base/test/ios/wait_util.h" #import "base/test/ios/wait_util.h"
#include "base/test/scoped_command_line.h" #include "base/test/scoped_command_line.h"
#include "components/strings/grit/components_strings.h" #include "components/strings/grit/components_strings.h"
#import "ios/chrome/browser/metrics/metrics_app_interface.h"
#import "ios/chrome/browser/ui/content_suggestions/ntp_home_constant.h" #import "ios/chrome/browser/ui/content_suggestions/ntp_home_constant.h"
#include "ios/chrome/grit/ios_strings.h" #include "ios/chrome/grit/ios_strings.h"
#import "ios/chrome/test/earl_grey/chrome_earl_grey.h" #import "ios/chrome/test/earl_grey/chrome_earl_grey.h"
#import "ios/chrome/test/earl_grey/chrome_matchers.h" #import "ios/chrome/test/earl_grey/chrome_matchers.h"
#import "ios/chrome/test/earl_grey/chrome_test_case.h" #import "ios/chrome/test/earl_grey/chrome_test_case.h"
#import "ios/testing/earl_grey/earl_grey_test.h" #import "ios/testing/earl_grey/earl_grey_test.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "net/test/embedded_test_server/http_request.h"
#include "net/test/embedded_test_server/http_response.h"
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
#if !defined(__has_feature) || !__has_feature(objc_arc) #if !defined(__has_feature) || !__has_feature(objc_arc)
...@@ -19,6 +23,25 @@ ...@@ -19,6 +23,25 @@
namespace { namespace {
const char kPageLoadedString[] = "Page loaded!";
const char kPageURL[] = "/test-page.html";
const char kPageTitle[] = "Page title!";
// Provides responses for redirect and changed window location URLs.
std::unique_ptr<net::test_server::HttpResponse> StandardResponse(
const net::test_server::HttpRequest& request) {
if (request.relative_url != kPageURL) {
return nullptr;
}
std::unique_ptr<net::test_server::BasicHttpResponse> http_response =
std::make_unique<net::test_server::BasicHttpResponse>();
http_response->set_code(net::HTTP_OK);
http_response->set_content("<html><head><title>" + std::string(kPageTitle) +
"</title></head><body>" +
std::string(kPageLoadedString) + "</body></html>");
return std::move(http_response);
}
// Pauses until the history label has disappeared. History should not show on // Pauses until the history label has disappeared. History should not show on
// incognito. // incognito.
BOOL WaitForHistoryToDisappear() { BOOL WaitForHistoryToDisappear() {
...@@ -39,10 +62,34 @@ BOOL WaitForHistoryToDisappear() { ...@@ -39,10 +62,34 @@ BOOL WaitForHistoryToDisappear() {
} // namespace } // namespace
@interface NewTabPageTestCase : ChromeTestCase @interface NewTabPageTestCase : ChromeTestCase
@property(nonatomic, assign) BOOL histogramTesterSet;
@end @end
@implementation NewTabPageTestCase @implementation NewTabPageTestCase
- (void)tearDown {
[self releaseHistogramTester];
[super tearDown];
}
- (void)setupHistogramTester {
if (self.histogramTesterSet) {
return;
}
self.histogramTesterSet = YES;
GREYAssertNil([MetricsAppInterface setupHistogramTester],
@"Cannot setup histogram tester.");
}
- (void)releaseHistogramTester {
if (!self.histogramTesterSet) {
return;
}
self.histogramTesterSet = NO;
GREYAssertNil([MetricsAppInterface releaseHistogramTester],
@"Cannot reset histogram tester.");
}
#pragma mark - Tests #pragma mark - Tests
// Tests that all items are accessible on the most visited page. // Tests that all items are accessible on the most visited page.
...@@ -74,6 +121,79 @@ BOOL WaitForHistoryToDisappear() { ...@@ -74,6 +121,79 @@ BOOL WaitForHistoryToDisappear() {
assertWithMatcher:grey_sufficientlyVisible()]; assertWithMatcher:grey_sufficientlyVisible()];
} }
// Tests the metrics are reported correctly.
- (void)testNTPMetrics {
self.testServer->RegisterRequestHandler(base::Bind(&StandardResponse));
GREYAssertTrue(self.testServer->Start(), @"Test server failed to start.");
const GURL pageURL = self.testServer->GetURL(kPageURL);
[ChromeEarlGrey closeAllTabs];
// Open and close an NTP.
[self setupHistogramTester];
NSError* error =
[MetricsAppInterface expectTotalCount:0
forHistogram:@"NewTabPage.TimeSpent"];
GREYAssertNil(error, error.description);
[ChromeEarlGrey openNewTab];
[ChromeEarlGrey closeAllTabs];
error = [MetricsAppInterface expectTotalCount:1
forHistogram:@"NewTabPage.TimeSpent"];
GREYAssertNil(error, error.description);
[self releaseHistogramTester];
// Open an incognito NTP and close it.
[ChromeEarlGrey closeAllTabs];
[self setupHistogramTester];
error = [MetricsAppInterface expectTotalCount:0
forHistogram:@"NewTabPage.TimeSpent"];
GREYAssertNil(error, error.description);
[ChromeEarlGrey openNewIncognitoTab];
[ChromeEarlGrey closeAllTabs];
error = [MetricsAppInterface expectTotalCount:0
forHistogram:@"NewTabPage.TimeSpent"];
GREYAssertNil(error, error.description);
[self releaseHistogramTester];
// Open an NTP and navigate to another URL.
[self setupHistogramTester];
error = [MetricsAppInterface expectTotalCount:0
forHistogram:@"NewTabPage.TimeSpent"];
GREYAssertNil(error, error.description);
[ChromeEarlGrey openNewTab];
[ChromeEarlGrey loadURL:pageURL];
[ChromeEarlGrey waitForWebStateContainingText:kPageLoadedString];
error = [MetricsAppInterface expectTotalCount:1
forHistogram:@"NewTabPage.TimeSpent"];
GREYAssertNil(error, error.description);
[self releaseHistogramTester];
// Open an NTP and switch tab.
[ChromeEarlGrey closeAllTabs];
[ChromeEarlGrey openNewTab];
[ChromeEarlGrey loadURL:pageURL];
[ChromeEarlGrey waitForWebStateContainingText:kPageLoadedString];
[self setupHistogramTester];
error = [MetricsAppInterface expectTotalCount:0
forHistogram:@"NewTabPage.TimeSpent"];
GREYAssertNil(error, error.description);
[ChromeEarlGrey openNewTab];
[ChromeEarlGrey selectTabAtIndex:0];
error = [MetricsAppInterface expectTotalCount:1
forHistogram:@"NewTabPage.TimeSpent"];
GREYAssertNil(error, error.description);
[ChromeEarlGrey selectTabAtIndex:1];
error = [MetricsAppInterface expectTotalCount:1
forHistogram:@"NewTabPage.TimeSpent"];
GREYAssertNil(error, error.description);
[ChromeEarlGrey selectTabAtIndex:0];
error = [MetricsAppInterface expectTotalCount:2
forHistogram:@"NewTabPage.TimeSpent"];
GREYAssertNil(error, error.description);
[self releaseHistogramTester];
}
// Tests that all items are accessible on the incognito page. // Tests that all items are accessible on the incognito page.
- (void)testAccessibilityOnIncognitoTab { - (void)testAccessibilityOnIncognitoTab {
[ChromeEarlGrey openNewIncognitoTab]; [ChromeEarlGrey openNewIncognitoTab];
......
...@@ -1446,10 +1446,11 @@ reviews. Googlers can read more about this at go/gwsq-gerrit. ...@@ -1446,10 +1446,11 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
<histogram name="NewTabPage.TimeSpent" units="ms" expires_after="2021-07-01"> <histogram name="NewTabPage.TimeSpent" units="ms" expires_after="2021-07-01">
<owner>freedjm@chromium.org</owner> <owner>freedjm@chromium.org</owner>
<owner>feed@chromium.org</owner> <owner>feed@chromium.org</owner>
<owner>olivierrobin@chromium.org</owner>
<summary> <summary>
The time spent on the new tab page as measured from when it was loaded or The time spent on the new tab page as measured from when it was loaded or
last brought to the foreground until it was navigated away from or hidden. last brought to the foreground until it was navigated away from or hidden.
Only measured on Android. Only measured on Android and iOS.
</summary> </summary>
</histogram> </histogram>
......
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