Commit 3d377f0c authored by Findit's avatar Findit

Revert "[LanguageDetection] SimpleKeyedService and optimization guide."

This reverts commit ffac4483.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 836656 as the
culprit for flakes in the build cycles as shown on:
https://analysis.chromium.org/p/chromium/flake-portal/analysis/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vZmZhYzQ0ODNlNTQ4Y2RmODk3NzJlYmM5ODk5MTM3MDU4OTczOGZjNww

Sample Failed Build: https://ci.chromium.org/b/8860905823939722448

Sample Failed Step: browser_tests

Sample Flaky Test: TranslateModelServiceWithoutOptimizationGuideBrowserTest.TranslateModelServiceEnabled

Original change's description:
> [LanguageDetection] SimpleKeyedService and optimization guide.
> 
> 
> This change converts the translate model service to a
> SimpleKeyedService and moves it to components which will enable
> easier support beyond just browser use cases.
> 
> This change also registers the translate model service to
> the optimization guide for provided necessary model files.
> 
> A future change will add loading and service the model files
> to consumers of the translate model service.
> 
> Bug: 1151407
> Change-Id: I4e903115c82a30757d46d75ed8d733874726d830
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2580339
> Commit-Queue: Michael Crouse <mcrouse@chromium.org>
> Reviewed-by: Colin Blundell <blundell@chromium.org>
> Reviewed-by: Trevor  Perrier <perrier@chromium.org>
> Reviewed-by: Scott Little <sclittle@chromium.org>
> Reviewed-by: Tarun Bansal <tbansal@chromium.org>
> Reviewed-by: Sophie Chang <sophiechang@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#836656}


Change-Id: I9f63ed714aafbe14ce8c48c23474becadd752186
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1151407
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2591947
Cr-Commit-Position: refs/heads/master@{#836880}
parent 9a568e2c
......@@ -1844,6 +1844,8 @@ static_library("browser") {
"translate/translate_accept_languages_factory.h",
"translate/translate_model_service_factory.cc",
"translate/translate_model_service_factory.h",
"translate/translate_model_service_impl.cc",
"translate/translate_model_service_impl.h",
"translate/translate_ranker_factory.cc",
"translate/translate_ranker_factory.h",
"translate/translate_ranker_metrics_provider.cc",
......
......@@ -7,13 +7,11 @@
#include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_key.h"
#include "chrome/browser/translate/translate_model_service_factory.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "components/metrics/content/subprocess_metrics_provider.h"
#include "components/optimization_guide/optimization_guide_features.h"
#include "components/translate/core/common/translate_util.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h"
......@@ -61,31 +59,8 @@ using TranslateModelServiceDisabledBrowserTest = InProcessBrowserTest;
IN_PROC_BROWSER_TEST_F(TranslateModelServiceDisabledBrowserTest,
TranslateModelServiceDisabled) {
EXPECT_FALSE(TranslateModelServiceFactory::GetOrBuildForKey(
browser()->profile()->GetProfileKey()));
}
class TranslateModelServiceWithoutOptimizationGuideBrowserTest
: public TranslateModelServiceDisabledBrowserTest {
public:
TranslateModelServiceWithoutOptimizationGuideBrowserTest() {
scoped_feature_list_.InitWithFeatures(
{translate::kTFLiteLanguageDetectionEnabled}, {});
}
~TranslateModelServiceWithoutOptimizationGuideBrowserTest() override =
default;
private:
base::test::ScopedFeatureList scoped_feature_list_;
};
// This test confirms the translate model service is not available if
// the optimization guide does not exist.
IN_PROC_BROWSER_TEST_F(TranslateModelServiceWithoutOptimizationGuideBrowserTest,
TranslateModelServiceEnabled) {
EXPECT_FALSE(TranslateModelServiceFactory::GetOrBuildForKey(
browser()->profile()->GetProfileKey()));
EXPECT_FALSE(
TranslateModelServiceFactory::GetForProfile(browser()->profile()));
}
IN_PROC_BROWSER_TEST_F(TranslateModelServiceDisabledBrowserTest,
......@@ -102,11 +77,8 @@ class TranslateModelServiceBrowserTest
: public TranslateModelServiceDisabledBrowserTest {
public:
TranslateModelServiceBrowserTest() {
scoped_feature_list_.InitWithFeatures(
{translate::kTFLiteLanguageDetectionEnabled,
optimization_guide::features::kOptimizationHints,
optimization_guide::features::kRemoteOptimizationGuideFetching},
{});
scoped_feature_list_.InitAndEnableFeature(
translate::kTFLiteLanguageDetectionEnabled);
}
~TranslateModelServiceBrowserTest() override = default;
......@@ -117,14 +89,14 @@ class TranslateModelServiceBrowserTest
IN_PROC_BROWSER_TEST_F(TranslateModelServiceBrowserTest,
TranslateModelServiceEnabled) {
EXPECT_TRUE(TranslateModelServiceFactory::GetOrBuildForKey(
browser()->profile()->GetProfileKey()));
EXPECT_TRUE(
TranslateModelServiceFactory::GetForProfile(browser()->profile()));
}
IN_PROC_BROWSER_TEST_F(TranslateModelServiceBrowserTest,
TranslateModelServiceEnabled_OffTheRecord) {
EXPECT_TRUE(TranslateModelServiceFactory::GetOrBuildForKey(
browser()->profile()->GetPrimaryOTRProfile()->GetProfileKey()));
EXPECT_TRUE(TranslateModelServiceFactory::GetForProfile(
browser()->profile()->GetPrimaryOTRProfile()));
}
IN_PROC_BROWSER_TEST_F(TranslateModelServiceBrowserTest,
......
......@@ -4,21 +4,21 @@
#include "chrome/browser/translate/translate_model_service_factory.h"
#include "chrome/browser/optimization_guide/optimization_guide_keyed_service.h"
#include "chrome/browser/optimization_guide/optimization_guide_keyed_service_factory.h"
#include "chrome/browser/profiles/incognito_helpers.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_key.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/translate/translate_model_service_impl.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/keyed_service/core/simple_dependency_manager.h"
#include "components/translate/content/browser/translate_model_service.h"
#include "components/translate/core/common/translate_util.h"
#include "content/public/browser/browser_context.h"
// static
translate::TranslateModelService*
TranslateModelServiceFactory::GetOrBuildForKey(SimpleFactoryKey* key) {
return static_cast<translate::TranslateModelService*>(
GetInstance()->GetServiceForKey(key, true));
TranslateModelServiceImpl* TranslateModelServiceFactory::GetForProfile(
Profile* profile) {
if (translate::IsTFLiteLanguageDetectionEnabled()) {
return static_cast<TranslateModelServiceImpl*>(
GetInstance()->GetServiceForBrowserContext(profile, true));
}
return nullptr;
}
// static
......@@ -28,31 +28,19 @@ TranslateModelServiceFactory* TranslateModelServiceFactory::GetInstance() {
}
TranslateModelServiceFactory::TranslateModelServiceFactory()
: SimpleKeyedServiceFactory("TranslateModelService",
SimpleDependencyManager::GetInstance()) {}
: BrowserContextKeyedServiceFactory(
"TranslateModelService",
BrowserContextDependencyManager::GetInstance()) {}
TranslateModelServiceFactory::~TranslateModelServiceFactory() = default;
std::unique_ptr<KeyedService>
TranslateModelServiceFactory::BuildServiceInstanceFor(
SimpleFactoryKey* key) const {
if (!translate::IsTFLiteLanguageDetectionEnabled())
return nullptr;
// The optimization guide service must be available for the translate model
// service to be created.
auto* opt_guide = OptimizationGuideKeyedServiceFactory::GetForProfile(
ProfileManager::GetProfileFromProfileKey(
ProfileKey::FromSimpleFactoryKey(key)));
if (opt_guide)
return std::make_unique<translate::TranslateModelService>(opt_guide);
return nullptr;
KeyedService* TranslateModelServiceFactory::BuildServiceInstanceFor(
content::BrowserContext* context) const {
return new TranslateModelServiceImpl();
}
SimpleFactoryKey* TranslateModelServiceFactory::GetKeyToUse(
SimpleFactoryKey* key) const {
// The translate model service should be able to
// operate in off the record sessions if the model is available from the
// optimization guide.
ProfileKey* profile_key = ProfileKey::FromSimpleFactoryKey(key);
return profile_key->GetOriginalKey();
content::BrowserContext* TranslateModelServiceFactory::GetBrowserContextToUse(
content::BrowserContext* context) const {
// Use the original profile's TranslateModelService, even in Incognito mode.
return chrome::GetBrowserContextRedirectedInIncognito(context);
}
......@@ -7,22 +7,24 @@
#include "base/macros.h"
#include "base/no_destructor.h"
#include "components/keyed_service/core/simple_keyed_service_factory.h"
#include "components/translate/content/browser/translate_model_service.h"
#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
class SimpleFactoryKey;
namespace content {
class BrowserContext;
} // namespace content
class TranslateModelServiceImpl;
class Profile;
// LazyInstance that owns all TranslateModelService(s) and associates
// them with Profiles.
class TranslateModelServiceFactory : public SimpleKeyedServiceFactory {
class TranslateModelServiceFactory : public BrowserContextKeyedServiceFactory {
public:
// Gets the TranslateModelService for the profile.
//
// Returns null if the features that allow for this to provide useful
// information are disabled. Importantly, only available when the
// optimization guide service is.
static translate::TranslateModelService* GetOrBuildForKey(
SimpleFactoryKey* key);
// information are disabled.
static TranslateModelServiceImpl* GetForProfile(Profile* profile);
// Gets the LazyInstance that owns all TranslateModelService(s).
static TranslateModelServiceFactory* GetInstance();
......@@ -33,10 +35,11 @@ class TranslateModelServiceFactory : public SimpleKeyedServiceFactory {
TranslateModelServiceFactory();
~TranslateModelServiceFactory() override;
// SimpleKeyedServiceFactory overrides:
std::unique_ptr<KeyedService> BuildServiceInstanceFor(
SimpleFactoryKey* key) const override;
SimpleFactoryKey* GetKeyToUse(SimpleFactoryKey* key) const override;
// BrowserContextKeyedServiceFactory:
KeyedService* BuildServiceInstanceFor(
content::BrowserContext* context) const override;
content::BrowserContext* GetBrowserContextToUse(
content::BrowserContext* context) const override;
};
#endif // CHROME_BROWSER_TRANSLATE_TRANSLATE_MODEL_SERVICE_FACTORY_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.
#include "chrome/browser/translate/translate_model_service_impl.h"
#include "base/files/file.h"
#include "base/optional.h"
TranslateModelServiceImpl::TranslateModelServiceImpl() {
// TODO(crbug.com/1151407): Register with the Optimiziation Guide for the
// language detection model.
}
TranslateModelServiceImpl::~TranslateModelServiceImpl() = default;
base::Optional<base::File>
TranslateModelServiceImpl::GetLanguageDetectionModelFile() {
// TODO(crbug.com/1151406): Implement loading the model on a background thread
// and return it for use by translate.
return base::nullopt;
}
// 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 CHROME_BROWSER_TRANSLATE_TRANSLATE_MODEL_SERVICE_IMPL_H_
#define CHROME_BROWSER_TRANSLATE_TRANSLATE_MODEL_SERVICE_IMPL_H_
#include "components/keyed_service/core/keyed_service.h"
#include "components/translate/core/browser/translate_model_service.h"
// Service that manages models required to support translation in the browser.
class TranslateModelServiceImpl : public KeyedService,
public translate::TranslateModelService {
public:
TranslateModelServiceImpl();
~TranslateModelServiceImpl() override;
// translate::TranslateModelService:
base::Optional<base::File> GetLanguageDetectionModelFile() override;
};
#endif // CHROME_BROWSER_TRANSLATE_TRANSLATE_MODEL_SERVICE_IMPL_H_
......@@ -14,8 +14,6 @@ static_library("browser") {
"content_translate_util.h",
"per_frame_content_translate_driver.cc",
"per_frame_content_translate_driver.h",
"translate_model_service.cc",
"translate_model_service.h",
]
public_deps = [
......@@ -26,10 +24,7 @@ static_library("browser") {
"//content/public/browser",
]
deps = [
"//components/keyed_service/core",
"//components/language/core/browser",
"//components/optimization_guide",
"//components/optimization_guide/proto:optimization_guide_proto",
"//components/search_engines:search_engines",
"//components/services/language_detection/public/cpp",
"//components/services/language_detection/public/mojom",
......
include_rules = [
"+content/public/browser",
"+components/google/core/common",
"+components/keyed_service/core",
"+components/optimization_guide",
"+components/services/language_detection/public/cpp",
"+components/services/language_detection/public/mojom",
"+components/translate/core/language_detection",
......
// 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.
#include "components/translate/content/browser/translate_model_service.h"
#include "base/files/file.h"
#include "base/files/file_path.h"
#include "components/optimization_guide/optimization_guide_decider.h"
#include "components/optimization_guide/proto/models.pb.h"
namespace translate {
TranslateModelService::TranslateModelService(
optimization_guide::OptimizationGuideDecider* opt_guide)
: opt_guide_(opt_guide) {
opt_guide_->AddObserverForOptimizationTargetModel(
optimization_guide::proto::OPTIMIZATION_TARGET_LANGUAGE_DETECTION, this);
}
TranslateModelService::~TranslateModelService() = default;
void TranslateModelService::Shutdown() {
// This and the optimization guide are keyed services, currently optimization
// guide is a BrowserContextKeyedService, it will be cleaned first so removing
// the observer should not be performed.
}
void TranslateModelService::OnModelFileUpdated(
optimization_guide::proto::OptimizationTarget optimization_target,
const base::FilePath& file_path) {
if (optimization_target !=
optimization_guide::proto::OPTIMIZATION_TARGET_LANGUAGE_DETECTION) {
return;
}
// TODO(crbug.com/1151406): Implement loading the model on a background thread
// and return it for use by translate.
}
base::Optional<base::File>
TranslateModelService::GetLanguageDetectionModelFile() {
// TODO(crbug.com/1151406): Implement loading the model on a background thread
// and return it for use by translate.
return base::nullopt;
}
} // namespace translate
// 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 COMPONENTS_TRANSLATE_CONTENT_BROWSER_TRANSLATE_MODEL_SERVICE_H_
#define COMPONENTS_TRANSLATE_CONTENT_BROWSER_TRANSLATE_MODEL_SERVICE_H_
#include "base/optional.h"
#include "components/keyed_service/core/keyed_service.h"
#include "components/optimization_guide/optimization_target_model_observer.h"
namespace base {
class File;
class FilePath;
} // namespace base
namespace optimization_guide {
class OptimizationGuideDecider;
} // namespace optimization_guide
namespace translate {
// Service that manages models required to support translation in the browser.
// Currently, the service should only be used in the browser as it relies on
// the Optimization Guide.
class TranslateModelService
: public KeyedService,
public optimization_guide::OptimizationTargetModelObserver {
public:
explicit TranslateModelService(
optimization_guide::OptimizationGuideDecider* opt_guide);
~TranslateModelService() override;
// KeyedService implementation:
void Shutdown() override;
// optimization_guide::OptimizationTargetModelObserver implementation:
void OnModelFileUpdated(
optimization_guide::proto::OptimizationTarget optimization_target,
const base::FilePath& file_path) override;
// Returns a loaded file containing the TFLite model capable of detecting the
// language of a web page's text.
base::Optional<base::File> GetLanguageDetectionModelFile();
private:
// Optimization Guide Service that provides model files for this
// service. Optimization Guide Service is a
// BrowserContextKeyedServiceFactory and should not
// be used after ShutDown.
optimization_guide::OptimizationGuideDecider* opt_guide_;
};
} // namespace translate
#endif // COMPONENTS_TRANSLATE_CONTENT_BROWSER_TRANSLATE_MODEL_SERVICE_H_
......@@ -30,6 +30,7 @@ static_library("browser") {
"translate_metrics_logger.h",
"translate_metrics_logger_impl.cc",
"translate_metrics_logger_impl.h",
"translate_model_service.h",
"translate_prefs.cc",
"translate_prefs.h",
"translate_ranker.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 COMPONENTS_TRANSLATE_CORE_BROWSER_TRANSLATE_MODEL_SERVICE_H_
#define COMPONENTS_TRANSLATE_CORE_BROWSER_TRANSLATE_MODEL_SERVICE_H_
#include "base/optional.h"
namespace base {
class File;
} // namespace base
namespace translate {
// Service that manages models required to support translation in the browser.
class TranslateModelService {
public:
TranslateModelService() = default;
// Returns a loaded file containing the TFLite model capable of detecting the
// language of a web page's text.
virtual base::Optional<base::File> GetLanguageDetectionModelFile() = 0;
protected:
virtual ~TranslateModelService() = default;
};
} // namespace translate
#endif // COMPONENTS_TRANSLATE_CORE_BROWSER_TRANSLATE_MODEL_SERVICE_H_
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