-
Notifications
You must be signed in to change notification settings - Fork 6.7k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Scope LanguageDetectionModel to a ChromeBrowserState
TranslateModelService downloads the model and makes it available as a file, but does not load it in a LanguageDetectionModel (and in TFLite). On other platforms, this done in the ContentTranslateDriver in a content process. But on iOS, tabs are in the same process, so it is not needed to load the model multiple times and multiple tabs trying to load the model file in the LanguageDetectionModel can lead to conflicts. Instead, create a new service that will handle the loading once and provide the LanguageDetectionModel to every tabs. Bug: 1285224 Change-Id: Ie4f936cedc53634ac7e8e88c1c5213f12068ba87 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3592978 Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Reviewed-by: Megan Jablonski <megjablon@chromium.org> Reviewed-by: Michael Crouse <mcrouse@chromium.org> Reviewed-by: Colin Blundell <blundell@chromium.org> Commit-Queue: Olivier Robin <olivierrobin@chromium.org> Cr-Commit-Position: refs/heads/main@{#1002571}
- Loading branch information
1 parent
0b4c87a
commit 1d4b895
Showing
11 changed files
with
279 additions
and
45 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| include_rules = [ | ||
| "+components/keyed_service/core/keyed_service.h", | ||
| "+components/language/core/browser", | ||
| "+components/ukm/ios", | ||
| ] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
53 changes: 53 additions & 0 deletions
53
components/translate/ios/browser/language_detection_model_service.h
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| // Copyright 2022 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_IOS_BROWSER_LANGUAGE_DETECTION_MODEL_SERVICE_H_ | ||
| #define COMPONENTS_TRANSLATE_IOS_BROWSER_LANGUAGE_DETECTION_MODEL_SERVICE_H_ | ||
|
|
||
| #include "base/files/file.h" | ||
| #include "base/memory/raw_ptr.h" | ||
| #include "base/memory/weak_ptr.h" | ||
| #include "base/sequence_checker.h" | ||
| #include "components/keyed_service/core/keyed_service.h" | ||
| #include "components/translate/core/browser/translate_model_service.h" | ||
|
|
||
| namespace translate { | ||
|
|
||
| class LanguageDetectionModelContainer; | ||
| class LanguageDetectionModel; | ||
|
|
||
| // A service that contains the LanguageDetectionModel and handles its loading. | ||
| // This is a workaround for crbug/1324530 on iOS where it is mandatory to have | ||
| // LanguageDetectionModel scoped by BrowserState. | ||
| // TODO(crbug.com/1324530): remove this class once TranslateModelService does | ||
| // this. | ||
| class LanguageDetectionModelService : public KeyedService { | ||
| public: | ||
| LanguageDetectionModelService( | ||
| TranslateModelService* opt_guide, | ||
| const scoped_refptr<base::SequencedTaskRunner>& background_task_runner); | ||
| ~LanguageDetectionModelService() override; | ||
|
|
||
| // Get for the actual TFLite language detection model. | ||
| LanguageDetectionModel* GetLanguageDetectionModel(); | ||
|
|
||
| // Utility function to check if the model is already loaded. | ||
| // |GetLanguageDetectionModel| can be used even if this return false. | ||
| bool IsModelAvailable(); | ||
|
|
||
| private: | ||
| // Notifies |this| that the translate model service is available for model | ||
| // requests or is invalidating existing requests specified by |is_available|. | ||
| void OnLanguageModelFileAvailabilityChanged(bool available); | ||
| // The TranslageModelService that will handle the downloading and provide | ||
| // the file containing the model. | ||
| raw_ptr<TranslateModelService> translate_model_service_; | ||
| scoped_refptr<base::SequencedTaskRunner> background_task_runner_; | ||
| scoped_refptr<LanguageDetectionModelContainer> language_detection_model_; | ||
| base::WeakPtrFactory<LanguageDetectionModelService> weak_ptr_factory_{this}; | ||
| }; | ||
|
|
||
| } // namespace translate | ||
|
|
||
| #endif // COMPONENTS_TRANSLATE_IOS_BROWSER_LANGUAGE_DETECTION_MODEL_SERVICE_H_ |
83 changes: 83 additions & 0 deletions
83
components/translate/ios/browser/language_detection_model_service.mm
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| // Copyright 2022 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/ios/browser/language_detection_model_service.h" | ||
|
|
||
| #include <memory> | ||
|
|
||
| #include "base/task/sequenced_task_runner.h" | ||
| #include "components/translate/core/language_detection/language_detection_model.h" | ||
|
|
||
| #if !defined(__has_feature) || !__has_feature(objc_arc) | ||
| #error "This file requires ARC support." | ||
| #endif | ||
|
|
||
| namespace translate { | ||
|
|
||
| class LanguageDetectionModelContainer | ||
| : public base::RefCountedThreadSafe<LanguageDetectionModelContainer>, | ||
| public LanguageDetectionModel { | ||
| public: | ||
| LanguageDetectionModelContainer() {} | ||
|
|
||
| private: | ||
| // Allow destruction by RefCounted<>. | ||
| friend class RefCountedThreadSafe<LanguageDetectionModelContainer>; | ||
| // Destructor must be private/protected. | ||
| ~LanguageDetectionModelContainer() = default; | ||
| }; | ||
|
|
||
| namespace { | ||
| void SetLanguageDetectionModelModelFile( | ||
| scoped_refptr<translate::LanguageDetectionModelContainer> | ||
| language_detection_model, | ||
| base::File model_file) { | ||
| language_detection_model->UpdateWithFile(std::move(model_file)); | ||
| } | ||
| } // namespace | ||
|
|
||
| LanguageDetectionModelService::LanguageDetectionModelService( | ||
| TranslateModelService* translate_model_service, | ||
| const scoped_refptr<base::SequencedTaskRunner>& background_task_runner) | ||
| : translate_model_service_(translate_model_service), | ||
| background_task_runner_(background_task_runner), | ||
| language_detection_model_( | ||
| base::MakeRefCounted<LanguageDetectionModelContainer>()) { | ||
| if (translate_model_service_) { | ||
| if (!language_detection_model_->IsAvailable()) { | ||
| translate_model_service_->NotifyOnModelFileAvailable( | ||
| base::BindOnce(&LanguageDetectionModelService:: | ||
| OnLanguageModelFileAvailabilityChanged, | ||
| weak_ptr_factory_.GetWeakPtr())); | ||
| } else { | ||
| OnLanguageModelFileAvailabilityChanged(true); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| LanguageDetectionModelService::~LanguageDetectionModelService() {} | ||
|
|
||
| LanguageDetectionModel* | ||
| LanguageDetectionModelService::GetLanguageDetectionModel() { | ||
| return language_detection_model_.get(); | ||
| } | ||
|
|
||
| bool LanguageDetectionModelService::IsModelAvailable() { | ||
| return language_detection_model_->IsAvailable(); | ||
| } | ||
|
|
||
| void LanguageDetectionModelService::OnLanguageModelFileAvailabilityChanged( | ||
| bool available) { | ||
| if (available) { | ||
| DCHECK(translate_model_service_); | ||
| base::File model_file = | ||
| translate_model_service_->GetLanguageDetectionModelFile(); | ||
| background_task_runner_->PostTask( | ||
| FROM_HERE, | ||
| base::BindOnce(&SetLanguageDetectionModelModelFile, | ||
| language_detection_model_, std::move(model_file))); | ||
| } | ||
| } | ||
|
|
||
| } // namespace translate |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
48 changes: 48 additions & 0 deletions
48
ios/chrome/browser/translate/language_detection_model_service_factory.h
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| // Copyright 2022 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_TRANSLATE_LANGUAGE_DETECTION_MODEL_SERVICE_FACTORY_H_ | ||
| #define IOS_CHROME_BROWSER_TRANSLATE_LANGUAGE_DETECTION_MODEL_SERVICE_FACTORY_H_ | ||
|
|
||
| #include <memory> | ||
|
|
||
| #include "base/no_destructor.h" | ||
| #include "components/keyed_service/ios/browser_state_keyed_service_factory.h" | ||
|
|
||
| class ChromeBrowserState; | ||
|
|
||
| namespace translate { | ||
| class LanguageDetectionModelService; | ||
| } // namespace translate | ||
|
|
||
| // This is a workaround for crbug/1324530 on iOS where it is mandatory to have | ||
| // LanguageDetectionModel scoped by BrowserState. | ||
| // TODO(crbug.com/1324530): remove this class once TranslateModelService does | ||
| // this. | ||
| class LanguageDetectionModelServiceFactory | ||
| : public BrowserStateKeyedServiceFactory { | ||
| public: | ||
| static translate::LanguageDetectionModelService* GetForBrowserState( | ||
| ChromeBrowserState* browser_state); | ||
| static LanguageDetectionModelServiceFactory* GetInstance(); | ||
|
|
||
| LanguageDetectionModelServiceFactory( | ||
| const LanguageDetectionModelServiceFactory&) = delete; | ||
| LanguageDetectionModelServiceFactory& operator=( | ||
| const LanguageDetectionModelServiceFactory&) = delete; | ||
|
|
||
| private: | ||
| friend class base::NoDestructor<LanguageDetectionModelServiceFactory>; | ||
|
|
||
| LanguageDetectionModelServiceFactory(); | ||
| ~LanguageDetectionModelServiceFactory() override; | ||
|
|
||
| // BrowserStateKeyedServiceFactory implementation. | ||
| std::unique_ptr<KeyedService> BuildServiceInstanceFor( | ||
| web::BrowserState* context) const override; | ||
| web::BrowserState* GetBrowserStateToUse( | ||
| web::BrowserState* context) const override; | ||
| }; | ||
|
|
||
| #endif // IOS_CHROME_BROWSER_TRANSLATE_LANGUAGE_DETECTION_MODEL_SERVICE_FACTORY_H_ |
Oops, something went wrong.