Skip to content

Commit

Permalink
Record PageLoad metrics w/ TranslateMetricsLogger
Browse files Browse the repository at this point in the history 
Added and translate_metrics_logger_ private member following TranslatePageLoadMetricsObserver. Correspondences:
DidStartNavigation <-> OnStart, DidFinishNavigation <-> OnCommit, DidStartNavigation/WebStateDestroyed <-> OnComplete.

TranslateMetricsLogger lifetime tied to page load via ChromeIOSTranslateClient::DidStartNavigation.

(cherry picked from commit dd427e9)

Bug: 1326717
Test: Build iOS simulator, run Chromium, navigate to chrome://histograms/Translate.PageLoad, then any website in a new tab, then refresh chrome://histograms/Translate.PageLoad tab. Automated unit tests: build and run ios_chrome_unittests with --gtest_filter="ChromeIOSTranslateClientTest.*" flag.
Change-Id: Ie7652a2545f094e4226abd0820a5b2020460baa9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3662122
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Reviewed-by: Michael Crouse <mcrouse@chromium.org>
Reviewed-by: Trevor Perrier <perrier@chromium.org>
Reviewed-by: Raj T <rajendrant@chromium.org>
Commit-Queue: Zhixiang Teoh <zhteoh@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1008509}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3684613
Commit-Queue: Raj T <rajendrant@chromium.org>
Cr-Commit-Position: refs/branch-heads/5060@{#470}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
  • Loading branch information
@zhixiangteoh
zhixiangteoh authored and Chromium LUCI CQ committed Jun 1, 2022
1 parent de66d3d commit f7f0747
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 2 deletions.
2 changes: 1 addition & 1 deletion ios/chrome/browser/translate/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ source_set("unit_tests") {
"//base/test:test_support",
"//components/language/core/browser",
"//components/language/ios/browser",
"//components/translate/core/browser:test_support",
"//components/translate/core/browser",
"//components/translate/core/common",
"//components/translate/ios/browser",
"//ios/chrome/browser",
Expand Down
21 changes: 21 additions & 0 deletions ios/chrome/browser/translate/chrome_ios_translate_client.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ namespace translate {
class TranslateAcceptLanguages;
class TranslatePrefs;
class TranslateManager;
class TranslateMetricsLogger;
} // namespace translate

namespace web {
Expand Down Expand Up @@ -69,17 +70,37 @@ class ChromeIOSTranslateClient
private:
ChromeIOSTranslateClient(web::WebState* web_state);
friend class web::WebStateUserData<ChromeIOSTranslateClient>;
FRIEND_TEST_ALL_PREFIXES(ChromeIOSTranslateClientTest,
NewMetricsOnPageLoadCommits);
FRIEND_TEST_ALL_PREFIXES(ChromeIOSTranslateClientTest,
NoNewMetricsOnErrorPage);
FRIEND_TEST_ALL_PREFIXES(ChromeIOSTranslateClientTest,
PageTranslationCorrectlyUpdatesMetrics);

// web::WebStateObserver implementation.
void DidStartNavigation(web::WebState* web_state,
web::NavigationContext* navigation_context) override;
void DidFinishNavigation(web::WebState* web_state,
web::NavigationContext* navigation_context) override;
void WasShown(web::WebState* web_state) override;
void WasHidden(web::WebState* web_state) override;
void WebStateDestroyed(web::WebState* web_state) override;

// Triggered when the foreground page is changed by a new navigation (in
// DidStartNavigation) or is permanently closed (in WebStateDestroyed),
// similar to PageLoadMetricsObserver::OnComplete.
void DidPageLoadComplete();

// The WebState this instance is observing. Will be null after
// WebStateDestroyed has been called.
web::WebState* web_state_ = nullptr;

std::unique_ptr<translate::TranslateManager> translate_manager_;
translate::IOSTranslateDriver translate_driver_;

// Metrics recorder for page load events.
std::unique_ptr<translate::TranslateMetricsLogger> translate_metrics_logger_;

WEB_STATE_USER_DATA_KEY_DECL();
};

Expand Down
51 changes: 51 additions & 0 deletions ios/chrome/browser/translate/chrome_ios_translate_client.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "components/translate/core/browser/translate_accept_languages.h"
#include "components/translate/core/browser/translate_infobar_delegate.h"
#include "components/translate/core/browser/translate_manager.h"
#include "components/translate/core/browser/translate_metrics_logger_impl.h"
#include "components/translate/core/browser/translate_step.h"
#include "components/translate/core/common/language_detection_details.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
Expand All @@ -32,6 +33,7 @@
#include "ios/chrome/browser/translate/translate_service_ios.h"
#include "ios/chrome/grit/ios_theme_resources.h"
#include "ios/web/public/browser_state.h"
#include "ios/web/public/navigation/navigation_context.h"
#import "ios/web/public/web_state.h"
#include "third_party/metrics_proto/translate_event.pb.h"
#include "url/gurl.h"
Expand Down Expand Up @@ -154,15 +156,64 @@
return false;
}

void ChromeIOSTranslateClient::DidStartNavigation(
web::WebState* web_state,
web::NavigationContext* navigation_context) {
if (navigation_context->IsSameDocument())
return;

DidPageLoadComplete();
// Lifetime of TranslateMetricsLogger should be each page load. So, we need to
// detect the page load completion, i.e. the tab was closed, new navigation
// replaced the page load, etc, and clear the logger.
translate_metrics_logger_ =
std::make_unique<translate::TranslateMetricsLoggerImpl>(
translate_manager_->GetWeakPtr());
translate_metrics_logger_->OnPageLoadStart(web_state->IsVisible());
}

void ChromeIOSTranslateClient::DidFinishNavigation(
web::WebState* web_state,
web::NavigationContext* navigation_context) {
if (navigation_context->GetError()) {
translate_metrics_logger_.reset();
return;
}

if (!navigation_context->IsSameDocument() && translate_metrics_logger_) {
translate_metrics_logger_->SetUkmSourceId(
translate_driver_.GetUkmSourceId());
}
}

void ChromeIOSTranslateClient::WasShown(web::WebState* web_state) {
if (translate_metrics_logger_)
translate_metrics_logger_->OnForegroundChange(true);
};

void ChromeIOSTranslateClient::WasHidden(web::WebState* web_state) {
if (translate_metrics_logger_)
translate_metrics_logger_->OnForegroundChange(false);
};

void ChromeIOSTranslateClient::WebStateDestroyed(web::WebState* web_state) {
DCHECK_EQ(web_state_, web_state);
web_state_->RemoveObserver(this);
web_state_ = nullptr;

DidPageLoadComplete();

// Translation process can be interrupted.
// Destroying the TranslateManager now guarantees that it never has to deal
// with nullptr WebState.
translate_manager_.reset();
}

void ChromeIOSTranslateClient::DidPageLoadComplete() {
if (translate_metrics_logger_) {
translate_metrics_logger_->RecordMetrics(true);
translate_metrics_logger_.reset();
}
}

WEB_STATE_USER_DATA_KEY_IMPL(ChromeIOSTranslateClient)
78 changes: 77 additions & 1 deletion ios/chrome/browser/translate/chrome_ios_translate_client_unittest.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@
// found in the LICENSE file.

#include "ios/chrome/browser/translate/chrome_ios_translate_client.h"
#import "base/metrics/metrics_hashes.h"
#import "base/test/metrics/histogram_tester.h"
#import "base/test/scoped_feature_list.h"
#import "base/test/task_environment.h"
#import "components/language/ios/browser/ios_language_detection_tab_helper.h"
#import "components/translate/core/browser/translate_metrics_logger.h"
#import "components/translate/core/common/translate_util.h"
#import "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
#import "ios/chrome/browser/infobars/infobar_manager_impl.h"
#include "ios/chrome/browser/infobars/infobar_manager_impl.h"
#import "ios/chrome/browser/language/language_model_manager_factory.h"
#import "ios/chrome/browser/optimization_guide/optimization_guide_service.h"
#import "ios/chrome/browser/optimization_guide/optimization_guide_service_factory.h"
Expand All @@ -20,6 +22,7 @@
#import "ios/web/public/test/fakes/fake_navigation_manager.h"
#import "ios/web/public/test/fakes/fake_web_state.h"
#import "testing/platform_test.h"
#import "url/gurl.h"

#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
Expand Down Expand Up @@ -64,3 +67,76 @@ void SetUp() override {
/*triggered_from_menu=*/false);
EXPECT_EQ(1U, InfoBarManagerImpl::FromWebState(&web_state_)->infobar_count());
}

TEST_F(ChromeIOSTranslateClientTest, NewMetricsOnPageLoadCommits) {
ChromeIOSTranslateClient* translate_client =
ChromeIOSTranslateClient::FromWebState(&web_state_);

web::FakeNavigationContext context;
translate_client->DidStartNavigation(&web_state_, &context);
translate_client->DidFinishNavigation(&web_state_, &context);
base::RunLoop().RunUntilIdle();
histogram_tester_.ExpectTotalCount("Translate.PageLoad.NumTranslations", 0);

// Navigate to new URL within same tab (web state).
translate_client->DidStartNavigation(&web_state_, &context);
translate_client->DidFinishNavigation(&web_state_, &context);
base::RunLoop().RunUntilIdle();
histogram_tester_.ExpectUniqueSample("Translate.PageLoad.NumTranslations", 0,
1);

// Close tab (web state).
translate_client->WebStateDestroyed(&web_state_);
histogram_tester_.ExpectUniqueSample("Translate.PageLoad.NumTranslations", 0,
2);
}

TEST_F(ChromeIOSTranslateClientTest, NoNewMetricsOnErrorPage) {
ChromeIOSTranslateClient* translate_client =
ChromeIOSTranslateClient::FromWebState(&web_state_);

web::FakeNavigationContext context;
translate_client->DidStartNavigation(&web_state_, &context); // needed?
context.SetError([NSError
errorWithDomain:@"commm"
code:200
userInfo:@{@"Error reason" : @"Invalid Input"}]);
EXPECT_TRUE(context.GetError());
translate_client->DidFinishNavigation(&web_state_, &context);
translate_client->WebStateDestroyed(&web_state_);

histogram_tester_.ExpectTotalCount("Translate.PageLoad.NumTranslations", 0);
}

TEST_F(ChromeIOSTranslateClientTest, PageTranslationCorrectlyUpdatesMetrics) {
ChromeIOSTranslateClient* translate_client =
ChromeIOSTranslateClient::FromWebState(&web_state_);

histogram_tester_.ExpectTotalCount("Translate.PageLoad.InitialSourceLanguage",
0);
histogram_tester_.ExpectTotalCount("Translate.PageLoad.FinalTargetLanguage",
0);
histogram_tester_.ExpectTotalCount("Translate.PageLoad.NumTranslations", 0);

web::FakeNavigationContext context;
translate_client->DidStartNavigation(&web_state_, &context);
translate_client->DidFinishNavigation(&web_state_, &context);
translate_client->translate_metrics_logger_->LogInitialSourceLanguage(
"en", /*is_in_users_content_language=*/true);
translate_client->translate_metrics_logger_->LogTargetLanguage(
"ko", /*target_language_origin=*/translate::TranslateBrowserMetrics::
TargetLanguageOrigin::kUninitialized);
translate_client->translate_metrics_logger_->LogTranslationStarted(
translate::TranslationType::kUninitialized);
translate_client->translate_metrics_logger_->LogTranslationFinished(
true, translate::TranslateErrors::NONE);
translate_client->WebStateDestroyed(&web_state_);

histogram_tester_.ExpectUniqueSample(
"Translate.PageLoad.InitialSourceLanguage", base::HashMetricName("en"),
1);
histogram_tester_.ExpectUniqueSample("Translate.PageLoad.FinalTargetLanguage",
base::HashMetricName("ko"), 1);
histogram_tester_.ExpectUniqueSample("Translate.PageLoad.NumTranslations", 1,
1);
}

0 comments on commit f7f0747

Please sign in to comment.