Commit c6504305 authored by Eugene But's avatar Eugene But Committed by Commit Bot

Log Find In Page user actions on iOS.

These are existing User Actions, which were not logged on iOS.

Bug: None
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Id1700628a2c54dbd2ba2bbc43bf631a9e474b1ff
Reviewed-on: https://chromium-review.googlesource.com/1175208Reviewed-by: default avatarPeter Lee <pkl@chromium.org>
Commit-Queue: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583339}
parent 3045c529
......@@ -17,6 +17,11 @@
typedef void (^FindInPageCompletionBlock)(FindInPageModel*);
// Names for Find In Page UMA actions (Find, FindNext, FindPrevious).
extern const char kFindActionName[];
extern const char kFindNextActionName[];
extern const char kFindPreviousActionName[];
// Adds support for the "Find in page" feature.
class FindTabHelper : public web::WebStateObserver,
public web::WebStateUserData<FindTabHelper> {
......
......@@ -5,6 +5,8 @@
#import "ios/chrome/browser/find_in_page/find_tab_helper.h"
#include "base/memory/ptr_util.h"
#include "base/metrics/user_metrics.h"
#include "base/metrics/user_metrics_action.h"
#import "ios/chrome/browser/find_in_page/find_in_page_controller.h"
#import "ios/chrome/browser/find_in_page/find_in_page_model.h"
......@@ -12,6 +14,10 @@
#error "This file requires ARC support."
#endif
const char kFindActionName[] = "Find";
const char kFindNextActionName[] = "FindNext";
const char kFindPreviousActionName[] = "FindPrevious";
DEFINE_WEB_STATE_USER_DATA_KEY(FindTabHelper);
FindTabHelper::FindTabHelper(web::WebState* web_state) {
......@@ -23,6 +29,7 @@ FindTabHelper::~FindTabHelper() {}
void FindTabHelper::StartFinding(NSString* search_term,
FindInPageCompletionBlock completion) {
base::RecordAction(base::UserMetricsAction(kFindActionName));
[controller_ findStringInPage:search_term
completionHandler:^{
FindInPageModel* model = controller_.findInPageModel;
......@@ -35,11 +42,13 @@ void FindTabHelper::ContinueFinding(FindDirection direction,
FindInPageModel* model = controller_.findInPageModel;
if (direction == FORWARD) {
base::RecordAction(base::UserMetricsAction(kFindNextActionName));
[controller_ findNextStringInPageWithCompletionHandler:^{
completion(model);
}];
} else if (direction == REVERSE) {
base::RecordAction(base::UserMetricsAction(kFindPreviousActionName));
[controller_ findPreviousStringInPageWithCompletionHandler:^{
completion(model);
}];
......
......@@ -7,6 +7,7 @@
#include "base/macros.h"
#include "base/run_loop.h"
#import "base/test/ios/wait_util.h"
#include "base/test/metrics/user_action_tester.h"
#import "ios/chrome/browser/find_in_page/find_in_page_model.h"
#import "ios/chrome/browser/web/chrome_web_test.h"
#import "ios/web/public/test/fakes/test_web_state.h"
......@@ -59,6 +60,8 @@ class FindTabHelperTest : public ChromeWebTest {
LoadHtml(html);
}
base::UserActionTester user_action_tester_;
private:
DISALLOW_COPY_AND_ASSIGN(FindTabHelperTest);
};
......@@ -81,21 +84,25 @@ TEST_F(FindTabHelperTest, FindInPage) {
};
// Search for "Test string" and verify that there are five matches.
ASSERT_EQ(0, user_action_tester_.GetActionCount(kFindActionName));
helper->StartFinding(@"Test string", ^(FindInPageModel* model) {
EXPECT_EQ(5U, model.matches);
EXPECT_EQ(1U, model.currentIndex);
completion_handler_block_was_called = YES;
});
base::test::ios::WaitUntilCondition(wait_block);
EXPECT_EQ(1, user_action_tester_.GetActionCount(kFindActionName));
// Search forward in the page for additional matches and verify that
// |currentIndex| is updated.
ASSERT_EQ(0, user_action_tester_.GetActionCount(kFindNextActionName));
helper->ContinueFinding(FindTabHelper::FORWARD, ^(FindInPageModel* model) {
EXPECT_EQ(5U, model.matches);
EXPECT_EQ(2U, model.currentIndex);
completion_handler_block_was_called = YES;
});
base::test::ios::WaitUntilCondition(wait_block);
EXPECT_EQ(1, user_action_tester_.GetActionCount(kFindNextActionName));
helper->ContinueFinding(FindTabHelper::FORWARD, ^(FindInPageModel* model) {
EXPECT_EQ(5U, model.matches);
......@@ -103,15 +110,18 @@ TEST_F(FindTabHelperTest, FindInPage) {
completion_handler_block_was_called = YES;
});
base::test::ios::WaitUntilCondition(wait_block);
EXPECT_EQ(2, user_action_tester_.GetActionCount(kFindNextActionName));
// Search backwards in the page for previous matches and verify that
// |currentIndex| is updated.
ASSERT_EQ(0, user_action_tester_.GetActionCount(kFindPreviousActionName));
helper->ContinueFinding(FindTabHelper::REVERSE, ^(FindInPageModel* model) {
EXPECT_EQ(5U, model.matches);
EXPECT_EQ(2U, model.currentIndex);
completion_handler_block_was_called = YES;
});
base::test::ios::WaitUntilCondition(wait_block);
EXPECT_EQ(1, user_action_tester_.GetActionCount(kFindPreviousActionName));
// Stop finding and verify that the completion block was called properly.
helper->StopFinding(^{
......@@ -138,30 +148,36 @@ TEST_F(FindTabHelperTest, ContinueFindingWrapsAround) {
};
// Search for "Test string".
ASSERT_EQ(0, user_action_tester_.GetActionCount(kFindActionName));
helper->StartFinding(@"Test string", ^(FindInPageModel* model) {
EXPECT_EQ(2U, model.matches);
EXPECT_EQ(1U, model.currentIndex);
completion_handler_block_was_called = YES;
});
base::test::ios::WaitUntilCondition(wait_block);
EXPECT_EQ(1, user_action_tester_.GetActionCount(kFindActionName));
// Search backwards in the page and verify that |currentIndex| wraps around to
// the last match.
ASSERT_EQ(0, user_action_tester_.GetActionCount(kFindPreviousActionName));
helper->ContinueFinding(FindTabHelper::REVERSE, ^(FindInPageModel* model) {
EXPECT_EQ(2U, model.matches);
EXPECT_EQ(2U, model.currentIndex);
completion_handler_block_was_called = YES;
});
base::test::ios::WaitUntilCondition(wait_block);
EXPECT_EQ(1, user_action_tester_.GetActionCount(kFindPreviousActionName));
// Search forward in the page and verify that |currentIndex| wraps around to
// the first match.
ASSERT_EQ(0, user_action_tester_.GetActionCount(kFindNextActionName));
helper->ContinueFinding(FindTabHelper::FORWARD, ^(FindInPageModel* model) {
EXPECT_EQ(2U, model.matches);
EXPECT_EQ(1U, model.currentIndex);
completion_handler_block_was_called = YES;
});
base::test::ios::WaitUntilCondition(wait_block);
EXPECT_EQ(1, user_action_tester_.GetActionCount(kFindNextActionName));
}
// Tests that the FindInPageModel returned by GetFindResults() is updated to
......@@ -183,10 +199,12 @@ TEST_F(FindTabHelperTest, GetFindResults) {
};
// Search for "Test string".
ASSERT_EQ(0, user_action_tester_.GetActionCount(kFindActionName));
helper->StartFinding(@"Test string", ^(FindInPageModel* model) {
completion_handler_block_was_called = YES;
});
base::test::ios::WaitUntilCondition(wait_block);
EXPECT_EQ(1, user_action_tester_.GetActionCount(kFindActionName));
{
FindInPageModel* model = helper->GetFindResult();
EXPECT_EQ(2U, model.matches);
......@@ -195,10 +213,12 @@ TEST_F(FindTabHelperTest, GetFindResults) {
// Search forward in the page and verify that |currentIndex| wraps around to
// the first match.
ASSERT_EQ(0, user_action_tester_.GetActionCount(kFindNextActionName));
helper->ContinueFinding(FindTabHelper::FORWARD, ^(FindInPageModel* model) {
completion_handler_block_was_called = YES;
});
base::test::ios::WaitUntilCondition(wait_block);
EXPECT_EQ(1, user_action_tester_.GetActionCount(kFindNextActionName));
{
FindInPageModel* model = helper->GetFindResult();
EXPECT_EQ(2U, model.matches);
......
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