From c7e7fb8b2be5d555d4d90cbf20cc9362705b5b78 Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Fri, 1 Oct 2021 18:12:10 +0700 Subject: [PATCH 01/16] Draft to support separate lists for origin and target languages --- ...nslate_redirect_network_delegate_helper.cc | 1 + browser/translate/buildflags/buildflags.gni | 2 +- .../translate/translate_bubble_model_impl.cc | 124 ++++++++++++++++++ .../translate/translate_bubble_model_impl.h | 37 ++++++ .../core/browser/translate_manager.cc | 69 +++++++++- .../core/browser/translate_manager.h | 39 ++++++ 6 files changed, 270 insertions(+), 2 deletions(-) create mode 100644 chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc create mode 100644 chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.h create mode 100644 chromium_src/components/translate/core/browser/translate_manager.h diff --git a/browser/net/brave_translate_redirect_network_delegate_helper.cc b/browser/net/brave_translate_redirect_network_delegate_helper.cc index 09d135bde310b..354a1cc035937 100644 --- a/browser/net/brave_translate_redirect_network_delegate_helper.cc +++ b/browser/net/brave_translate_redirect_network_delegate_helper.cc @@ -10,6 +10,7 @@ #include #include "brave/common/translate_network_constants.h" #include "extensions/common/url_pattern.h" +#include "net/base/net_errors.h" namespace { const char kTranslateElementLibQuery[] = "client=te_lib"; diff --git a/browser/translate/buildflags/buildflags.gni b/browser/translate/buildflags/buildflags.gni index 8158a0136f7eb..a7dcb77633470 100644 --- a/browser/translate/buildflags/buildflags.gni +++ b/browser/translate/buildflags/buildflags.gni @@ -2,7 +2,7 @@ import("//brave/build/config.gni") import("//extensions/buildflags/buildflags.gni") declare_args() { - enable_brave_translate_go = false + enable_brave_translate_go = true } declare_args() { diff --git a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc new file mode 100644 index 0000000000000..c1d21c2b6ce74 --- /dev/null +++ b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc @@ -0,0 +1,124 @@ +/* Copyright (c) 2021 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "chrome/browser/ui/translate/translate_bubble_model_impl.h" + +#define TranslateBubbleModelImpl ChromiumTranslateBubbleModelImpl +#include "../../../../../../../chrome/browser/ui/translate/translate_bubble_model_impl.cc" +#undef TranslateBubbleModelImpl + +#include "base/containers/contains.h" + +namespace { +const int kNoIndex = static_cast(translate::TranslateUIDelegate::kNoIndex); +} // namespace + +bool IsSourceLanguageSupported(const std::string& lang) { + return lang == "fr" || lang == "en" || lang == "de" || lang == "und"; +} + +bool IsTargetLanguageSupported(const std::string& lang) { + return lang == "en" || lang == "ru"; +} + +class BraveLanguageMap { + public: + BraveLanguageMap(const translate::TranslateUIDelegate* ui_delegate, + base::RepeatingCallback filter) { + int ui_index = 0; + std::string langs; + for (size_t core_index = 0; + core_index < ui_delegate->GetNumberOfLanguages(); ++core_index) { + const auto lang = ui_delegate->GetLanguageCodeAt(core_index); + langs += " " + lang; + if (!filter.Run(lang)) + continue; + to_core_index_[ui_index] = core_index; + to_ui_index_[core_index] = ui_index; + ++ui_index; + } + DCHECK_EQ(to_ui_index_.size(), to_core_index_.size()); +#if DCHECK_IS_ON() + for (int i = 0; i < static_cast(to_core_index_.size()); ++i) { + const int core_ind = to_core_index_[i]; + DCHECK_EQ(to_ui_index_[to_core_index_[i]], i); + DCHECK_EQ(to_core_index_[to_ui_index_[core_ind]], core_ind); + } +#endif // DCHECK_IS_ON() + } + size_t ToCoreIndex(int index) const { + if (index == kNoIndex || !base::Contains(to_core_index_, index)) + return translate::TranslateUIDelegate::kNoIndex; + return to_core_index_.at(index); + } + size_t ToUiIndex(int index) const { + if (index == kNoIndex || !base::Contains(to_ui_index_, index)) + return translate::TranslateUIDelegate::kNoIndex; + return to_ui_index_.at(index); + } + + int GetSize() const { return to_core_index_.size(); } + + private: + std::map to_core_index_, to_ui_index_; +}; + +BraveTranslateBubbleModelImpl::BraveTranslateBubbleModelImpl( + translate::TranslateStep step, + std::unique_ptr ui_delegate) + : ChromiumTranslateBubbleModelImpl(step, std::move(ui_delegate)) { + source_language_map_ = std::make_unique( + ui_delegate_.get(), base::BindRepeating(&IsSourceLanguageSupported)); + target_language_map_ = std::make_unique( + ui_delegate_.get(), base::BindRepeating(&IsTargetLanguageSupported)); + + // If the source language is unsupported the drop it to unknown. + // TODO(atuchin): is it good place to call this? + if (!IsSourceLanguageSupported(ui_delegate_->GetSourceLanguageCode())) { + ui_delegate_->UpdateSourceLanguageIndex(0u); + } +} + +BraveTranslateBubbleModelImpl::~BraveTranslateBubbleModelImpl() = default; + +int BraveTranslateBubbleModelImpl::GetNumberOfSourceLanguages() const { + return source_language_map_->GetSize(); +} + +int BraveTranslateBubbleModelImpl::GetNumberOfTargetLanguages() const { + return target_language_map_->GetSize(); +} + +std::u16string BraveTranslateBubbleModelImpl::GetSourceLanguageNameAt( + int index) const { + return ui_delegate_->GetLanguageNameAt( + source_language_map_->ToCoreIndex(index)); +} + +std::u16string BraveTranslateBubbleModelImpl::GetTargetLanguageNameAt( + int index) const { + return ui_delegate_->GetLanguageNameAt( + target_language_map_->ToCoreIndex(index)); +} + +int BraveTranslateBubbleModelImpl::GetSourceLanguageIndex() const { + return source_language_map_->ToUiIndex( + ui_delegate_->GetSourceLanguageIndex()); +} + +int BraveTranslateBubbleModelImpl::GetTargetLanguageIndex() const { + return target_language_map_->ToUiIndex( + ui_delegate_->GetTargetLanguageIndex()); +} + +void BraveTranslateBubbleModelImpl::UpdateSourceLanguageIndex(int index) { + ui_delegate_->UpdateSourceLanguageIndex( + source_language_map_->ToCoreIndex(index)); +} + +void BraveTranslateBubbleModelImpl::UpdateTargetLanguageIndex(int index) { + ui_delegate_->UpdateTargetLanguageIndex( + target_language_map_->ToCoreIndex(index)); +} diff --git a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.h b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.h new file mode 100644 index 0000000000000..a04a8706eb7cc --- /dev/null +++ b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.h @@ -0,0 +1,37 @@ +/* Copyright (c) 2021 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_TRANSLATE_TRANSLATE_BUBBLE_MODEL_IMPL_H_ +#define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_TRANSLATE_TRANSLATE_BUBBLE_MODEL_IMPL_H_ + +#define TranslateBubbleModelImpl ChromiumTranslateBubbleModelImpl +#include "../../../../../../../chrome/browser/ui/translate/translate_bubble_model_impl.h" +#undef TranslateBubbleModelImpl + +class BraveLanguageMap; +class BraveTranslateBubbleModelImpl : public ChromiumTranslateBubbleModelImpl { + public: + BraveTranslateBubbleModelImpl( + translate::TranslateStep step, + std::unique_ptr ui_delegate); + + ~BraveTranslateBubbleModelImpl() override; + + int GetNumberOfSourceLanguages() const override; + int GetNumberOfTargetLanguages() const override; + std::u16string GetSourceLanguageNameAt(int index) const override; + std::u16string GetTargetLanguageNameAt(int index) const override; + int GetSourceLanguageIndex() const override; + int GetTargetLanguageIndex() const override; + void UpdateSourceLanguageIndex(int index) override; + void UpdateTargetLanguageIndex(int index) override; + + private: + std::unique_ptr source_language_map_, target_language_map_; +}; + +using TranslateBubbleModelImpl = BraveTranslateBubbleModelImpl; + +#endif // BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_TRANSLATE_TRANSLATE_BUBBLE_MODEL_IMPL_H_ diff --git a/chromium_src/components/translate/core/browser/translate_manager.cc b/chromium_src/components/translate/core/browser/translate_manager.cc index e30a9989f14c0..b49825bc5b052 100644 --- a/chromium_src/components/translate/core/browser/translate_manager.cc +++ b/chromium_src/components/translate/core/browser/translate_manager.cc @@ -4,10 +4,77 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ #include "components/translate/core/browser/translate_manager.h" +#include "components/translate/core/browser/translate_download_manager.h" +#include "components/translate/core/browser/translate_prefs.h" +namespace { +bool IsSourceLanguageSupported(const std::string& lang) { + return lang == "fr" || lang == "en" || lang == "de" || lang == "und"; +} + +bool IsTargetLanguageSupported(const std::string& lang) { + return lang == "en" || lang == "ru"; +} +} // namespace + +namespace translate { + +class BraveIsSupportedTargetLanguageProxy : public TranslateDownloadManager { + public: + static bool IsSupportedLanguage(const std::string& lang) { + return IsTargetLanguageSupported(lang); + } + ~BraveIsSupportedTargetLanguageProxy() override; +}; + +} // namespace translate + +#define GetRecentTargetLanguage \ + GetRecentTargetLanguage(); \ + using TranslateDownloadManager = BraveIsSupportedTargetLanguageProxy; \ + void #define HasAPIKeyConfigured BraveHasAPIKeyConfigured -#include "../../../../../../components/translate/core/browser/translate_manager.cc" // NOLINT +#define TranslateManager ChromiumTranslateManager +#include "../../../../../../components/translate/core/browser/translate_manager.cc" // NOLINT #undef HasAPIKeyConfigured +#undef TranslateManager + +namespace translate { + +void TranslateManager::FilterIsTranslatePossible( + TranslateTriggerDecision* decision, + TranslatePrefs* translate_prefs, + const std::string& page_language_code, + const std::string& target_lang) { + ChromiumTranslateManager::FilterIsTranslatePossible( + decision, translate_prefs, page_language_code, target_lang); + if (!IsSourceLanguageSupported(page_language_code)) { + decision->PreventAutoTranslate(); + decision->PreventShowingUI(); + decision->initiation_statuses.push_back( + TranslateBrowserMetrics::INITIATION_STATUS_LANGUAGE_IS_NOT_SUPPORTED); + decision->ranker_events.push_back( + metrics::TranslateEventProto::UNSUPPORTED_LANGUAGE); + GetActiveTranslateMetricsLogger()->LogTriggerDecision( + TriggerDecision::kDisabledUnsupportedLanguage); + } + + if (!IsTargetLanguageSupported(target_lang)) { + decision->PreventAllTriggering(); + decision->initiation_statuses.push_back( + TranslateBrowserMetrics::INITIATION_STATUS_LANGUAGE_IS_NOT_SUPPORTED); + decision->ranker_events.push_back( + metrics::TranslateEventProto::UNSUPPORTED_LANGUAGE); + GetActiveTranslateMetricsLogger()->LogTriggerDecision( + TriggerDecision::kDisabledUnsupportedLanguage); + } +} + +base::WeakPtr TranslateManager::GetWeakPtr() { + return AsWeakPtr(); +} + +} // namespace translate namespace google_apis { diff --git a/chromium_src/components/translate/core/browser/translate_manager.h b/chromium_src/components/translate/core/browser/translate_manager.h new file mode 100644 index 0000000000000..18f38d81b078c --- /dev/null +++ b/chromium_src/components/translate/core/browser/translate_manager.h @@ -0,0 +1,39 @@ +/* Copyright (c) 2021 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_CHROMIUM_SRC_COMPONENTS_TRANSLATE_CORE_BROWSER_TRANSLATE_MANAGER_H_ +#define BRAVE_CHROMIUM_SRC_COMPONENTS_TRANSLATE_CORE_BROWSER_TRANSLATE_MANAGER_H_ + +namespace translate { +class TranslateManager; +using BraveTranslateManager = TranslateManager; +} // namespace translate + +#define TranslateManager ChromiumTranslateManager +#define ignore_missing_key_for_testing_ \ + ignore_missing_key_for_testing_; \ + friend BraveTranslateManager +#define FilterIsTranslatePossible virtual FilterIsTranslatePossible +#include "../../../../../../../components/translate/core/browser/translate_manager.h" +#undef FilterIsTranslatePossible +#undef ignore_missing_key_for_testing_ +#undef TranslateManager + +namespace translate { + +class TranslateManager : public ChromiumTranslateManager, + public base::SupportsWeakPtr { + public: + using ChromiumTranslateManager::ChromiumTranslateManager; + void FilterIsTranslatePossible(TranslateTriggerDecision* decision, + TranslatePrefs* translate_prefs, + const std::string& page_language_code, + const std::string& target_lang) override; + base::WeakPtr GetWeakPtr(); +}; + +} // namespace translate + +#endif // BRAVE_CHROMIUM_SRC_COMPONENTS_TRANSLATE_CORE_BROWSER_TRANSLATE_MANAGER_H_ From 03ff1958dca4459f05529975d6d3c0b3c1ed823f Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Fri, 1 Oct 2021 19:45:11 +0700 Subject: [PATCH 02/16] Remove some debug info --- .../chrome/browser/ui/translate/translate_bubble_model_impl.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc index c1d21c2b6ce74..3fa119da4d091 100644 --- a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc +++ b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc @@ -28,11 +28,9 @@ class BraveLanguageMap { BraveLanguageMap(const translate::TranslateUIDelegate* ui_delegate, base::RepeatingCallback filter) { int ui_index = 0; - std::string langs; for (size_t core_index = 0; core_index < ui_delegate->GetNumberOfLanguages(); ++core_index) { const auto lang = ui_delegate->GetLanguageCodeAt(core_index); - langs += " " + lang; if (!filter.Run(lang)) continue; to_core_index_[ui_index] = core_index; From 18a0be38c8a5ee4b6d30c02921ad1ec50c20a05a Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Fri, 1 Oct 2021 20:21:46 +0700 Subject: [PATCH 03/16] Simplify --- .../translate/translate_bubble_model_impl.cc | 23 ++++++++++--------- .../translate/translate_bubble_model_impl.h | 16 ++++++++----- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc index 3fa119da4d091..71557ff632c7d 100644 --- a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc +++ b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc @@ -60,10 +60,11 @@ class BraveLanguageMap { int GetSize() const { return to_core_index_.size(); } private: - std::map to_core_index_, to_ui_index_; + std::map to_core_index_; + std::map to_ui_index_; }; -BraveTranslateBubbleModelImpl::BraveTranslateBubbleModelImpl( +TranslateBubbleModelImpl::TranslateBubbleModelImpl( translate::TranslateStep step, std::unique_ptr ui_delegate) : ChromiumTranslateBubbleModelImpl(step, std::move(ui_delegate)) { @@ -79,44 +80,44 @@ BraveTranslateBubbleModelImpl::BraveTranslateBubbleModelImpl( } } -BraveTranslateBubbleModelImpl::~BraveTranslateBubbleModelImpl() = default; +TranslateBubbleModelImpl::~TranslateBubbleModelImpl() = default; -int BraveTranslateBubbleModelImpl::GetNumberOfSourceLanguages() const { +int TranslateBubbleModelImpl::GetNumberOfSourceLanguages() const { return source_language_map_->GetSize(); } -int BraveTranslateBubbleModelImpl::GetNumberOfTargetLanguages() const { +int TranslateBubbleModelImpl::GetNumberOfTargetLanguages() const { return target_language_map_->GetSize(); } -std::u16string BraveTranslateBubbleModelImpl::GetSourceLanguageNameAt( +std::u16string TranslateBubbleModelImpl::GetSourceLanguageNameAt( int index) const { return ui_delegate_->GetLanguageNameAt( source_language_map_->ToCoreIndex(index)); } -std::u16string BraveTranslateBubbleModelImpl::GetTargetLanguageNameAt( +std::u16string TranslateBubbleModelImpl::GetTargetLanguageNameAt( int index) const { return ui_delegate_->GetLanguageNameAt( target_language_map_->ToCoreIndex(index)); } -int BraveTranslateBubbleModelImpl::GetSourceLanguageIndex() const { +int TranslateBubbleModelImpl::GetSourceLanguageIndex() const { return source_language_map_->ToUiIndex( ui_delegate_->GetSourceLanguageIndex()); } -int BraveTranslateBubbleModelImpl::GetTargetLanguageIndex() const { +int TranslateBubbleModelImpl::GetTargetLanguageIndex() const { return target_language_map_->ToUiIndex( ui_delegate_->GetTargetLanguageIndex()); } -void BraveTranslateBubbleModelImpl::UpdateSourceLanguageIndex(int index) { +void TranslateBubbleModelImpl::UpdateSourceLanguageIndex(int index) { ui_delegate_->UpdateSourceLanguageIndex( source_language_map_->ToCoreIndex(index)); } -void BraveTranslateBubbleModelImpl::UpdateTargetLanguageIndex(int index) { +void TranslateBubbleModelImpl::UpdateTargetLanguageIndex(int index) { ui_delegate_->UpdateTargetLanguageIndex( target_language_map_->ToCoreIndex(index)); } diff --git a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.h b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.h index a04a8706eb7cc..cef1adc2924b0 100644 --- a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.h +++ b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.h @@ -6,18 +6,23 @@ #ifndef BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_TRANSLATE_TRANSLATE_BUBBLE_MODEL_IMPL_H_ #define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_TRANSLATE_TRANSLATE_BUBBLE_MODEL_IMPL_H_ +class TranslateBubbleModelImpl; +using BraveTranslateBubbleModelImpl = TranslateBubbleModelImpl; + #define TranslateBubbleModelImpl ChromiumTranslateBubbleModelImpl +#define translate_executed_ translate_executed_; friend BraveTranslateBubbleModelImpl #include "../../../../../../../chrome/browser/ui/translate/translate_bubble_model_impl.h" +#undef translate_executed_ #undef TranslateBubbleModelImpl class BraveLanguageMap; -class BraveTranslateBubbleModelImpl : public ChromiumTranslateBubbleModelImpl { +class TranslateBubbleModelImpl : public ChromiumTranslateBubbleModelImpl { public: - BraveTranslateBubbleModelImpl( + TranslateBubbleModelImpl( translate::TranslateStep step, std::unique_ptr ui_delegate); - ~BraveTranslateBubbleModelImpl() override; + ~TranslateBubbleModelImpl() override; int GetNumberOfSourceLanguages() const override; int GetNumberOfTargetLanguages() const override; @@ -29,9 +34,8 @@ class BraveTranslateBubbleModelImpl : public ChromiumTranslateBubbleModelImpl { void UpdateTargetLanguageIndex(int index) override; private: - std::unique_ptr source_language_map_, target_language_map_; + std::unique_ptr source_language_map_; + std::unique_ptr target_language_map_; }; -using TranslateBubbleModelImpl = BraveTranslateBubbleModelImpl; - #endif // BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_TRANSLATE_TRANSLATE_BUBBLE_MODEL_IMPL_H_ From 00a0d3817dcbd4a3fb3d00cbb8b810d7e6fc17c7 Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Tue, 5 Oct 2021 17:37:21 +0700 Subject: [PATCH 04/16] Move lists to a proper place --- .../translate/translate_bubble_model_impl.cc | 15 ++++----------- .../core/browser/translate_manager.cc | 18 +++++------------- components/translate/core/browser/BUILD.gn | 4 ++++ .../brave_translate_language_filter.cc | 19 +++++++++++++++++++ .../browser/brave_translate_language_filter.h | 19 +++++++++++++++++++ 5 files changed, 51 insertions(+), 24 deletions(-) create mode 100644 components/translate/core/browser/brave_translate_language_filter.cc create mode 100644 components/translate/core/browser/brave_translate_language_filter.h diff --git a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc index 71557ff632c7d..a5a60a5d6c2fa 100644 --- a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc +++ b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc @@ -10,19 +10,12 @@ #undef TranslateBubbleModelImpl #include "base/containers/contains.h" +#include "brave/components/translate/core/browser/brave_translate_language_filter.h" namespace { const int kNoIndex = static_cast(translate::TranslateUIDelegate::kNoIndex); } // namespace -bool IsSourceLanguageSupported(const std::string& lang) { - return lang == "fr" || lang == "en" || lang == "de" || lang == "und"; -} - -bool IsTargetLanguageSupported(const std::string& lang) { - return lang == "en" || lang == "ru"; -} - class BraveLanguageMap { public: BraveLanguageMap(const translate::TranslateUIDelegate* ui_delegate, @@ -69,13 +62,13 @@ TranslateBubbleModelImpl::TranslateBubbleModelImpl( std::unique_ptr ui_delegate) : ChromiumTranslateBubbleModelImpl(step, std::move(ui_delegate)) { source_language_map_ = std::make_unique( - ui_delegate_.get(), base::BindRepeating(&IsSourceLanguageSupported)); + ui_delegate_.get(), base::BindRepeating(&translate::IsSourceLanguageCodeSupported)); target_language_map_ = std::make_unique( - ui_delegate_.get(), base::BindRepeating(&IsTargetLanguageSupported)); + ui_delegate_.get(), base::BindRepeating(&translate::IsTargetLanguageCodeSupported)); // If the source language is unsupported the drop it to unknown. // TODO(atuchin): is it good place to call this? - if (!IsSourceLanguageSupported(ui_delegate_->GetSourceLanguageCode())) { + if (!translate::IsSourceLanguageCodeSupported(ui_delegate_->GetSourceLanguageCode())) { ui_delegate_->UpdateSourceLanguageIndex(0u); } } diff --git a/chromium_src/components/translate/core/browser/translate_manager.cc b/chromium_src/components/translate/core/browser/translate_manager.cc index b49825bc5b052..5f101f55233bc 100644 --- a/chromium_src/components/translate/core/browser/translate_manager.cc +++ b/chromium_src/components/translate/core/browser/translate_manager.cc @@ -4,25 +4,17 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ #include "components/translate/core/browser/translate_manager.h" + +#include "brave/components/translate/core/browser/brave_translate_language_filter.h" #include "components/translate/core/browser/translate_download_manager.h" #include "components/translate/core/browser/translate_prefs.h" -namespace { -bool IsSourceLanguageSupported(const std::string& lang) { - return lang == "fr" || lang == "en" || lang == "de" || lang == "und"; -} - -bool IsTargetLanguageSupported(const std::string& lang) { - return lang == "en" || lang == "ru"; -} -} // namespace - namespace translate { class BraveIsSupportedTargetLanguageProxy : public TranslateDownloadManager { public: static bool IsSupportedLanguage(const std::string& lang) { - return IsTargetLanguageSupported(lang); + return IsTargetLanguageCodeSupported(lang); } ~BraveIsSupportedTargetLanguageProxy() override; }; @@ -48,7 +40,7 @@ void TranslateManager::FilterIsTranslatePossible( const std::string& target_lang) { ChromiumTranslateManager::FilterIsTranslatePossible( decision, translate_prefs, page_language_code, target_lang); - if (!IsSourceLanguageSupported(page_language_code)) { + if (!IsSourceLanguageCodeSupported(page_language_code)) { decision->PreventAutoTranslate(); decision->PreventShowingUI(); decision->initiation_statuses.push_back( @@ -59,7 +51,7 @@ void TranslateManager::FilterIsTranslatePossible( TriggerDecision::kDisabledUnsupportedLanguage); } - if (!IsTargetLanguageSupported(target_lang)) { + if (!IsTargetLanguageCodeSupported(target_lang)) { decision->PreventAllTriggering(); decision->initiation_statuses.push_back( TranslateBrowserMetrics::INITIATION_STATUS_LANGUAGE_IS_NOT_SUPPORTED); diff --git a/components/translate/core/browser/BUILD.gn b/components/translate/core/browser/BUILD.gn index 80017fb6cb8e1..1715ff6948ed9 100644 --- a/components/translate/core/browser/BUILD.gn +++ b/components/translate/core/browser/BUILD.gn @@ -1,4 +1,8 @@ source_set("browser") { + sources = [ + "brave_translate_language_filter.cc", + "brave_translate_language_filter.h", + ] public_deps = [ # TODO(jocelyn): move buildflags to brave/components/translate/core/browser "//brave/browser/translate/buildflags", diff --git a/components/translate/core/browser/brave_translate_language_filter.cc b/components/translate/core/browser/brave_translate_language_filter.cc new file mode 100644 index 0000000000000..b97d21e4661b8 --- /dev/null +++ b/components/translate/core/browser/brave_translate_language_filter.cc @@ -0,0 +1,19 @@ +/* Copyright (c) 2021 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "brave/components/translate/core/browser/brave_translate_language_filter.h" + +namespace translate { + +bool IsSourceLanguageCodeSupported(const std::string& lang_code) { + return lang_code == "fr" || lang_code == "en" || lang_code == "de" || + lang_code == "und"; +} + +bool IsTargetLanguageCodeSupported(const std::string& lang_code) { + return lang_code == "en" || lang_code == "ru"; +} + +} // namespace translate diff --git a/components/translate/core/browser/brave_translate_language_filter.h b/components/translate/core/browser/brave_translate_language_filter.h new file mode 100644 index 0000000000000..66acd1bac215c --- /dev/null +++ b/components/translate/core/browser/brave_translate_language_filter.h @@ -0,0 +1,19 @@ +/* Copyright (c) 2021 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_COMPONENTS_TRANSLATE_CORE_BROWSER_BRAVE_TRANSLATE_LANGUAGE_FILTER_H_ +#define BRAVE_COMPONENTS_TRANSLATE_CORE_BROWSER_BRAVE_TRANSLATE_LANGUAGE_FILTER_H_ + +#include + +namespace translate { + +bool IsSourceLanguageCodeSupported(const std::string& lang_code); + +bool IsTargetLanguageCodeSupported(const std::string& lang_code); + +} // namespace translate + +#endif // BRAVE_COMPONENTS_TRANSLATE_CORE_BROWSER_BRAVE_TRANSLATE_LANGUAGE_FILTER_H_ From 891c660793708689c041d54f298252ab45e15b13 Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Tue, 5 Oct 2021 17:46:32 +0700 Subject: [PATCH 05/16] Fix gn check --- chromium_src/chrome/browser/ui/translate/DEPS | 3 +++ chromium_src/components/translate/core/browser/DEPS | 1 + test/BUILD.gn | 1 + 3 files changed, 5 insertions(+) create mode 100644 chromium_src/chrome/browser/ui/translate/DEPS diff --git a/chromium_src/chrome/browser/ui/translate/DEPS b/chromium_src/chrome/browser/ui/translate/DEPS new file mode 100644 index 0000000000000..bcc8bcd864d09 --- /dev/null +++ b/chromium_src/chrome/browser/ui/translate/DEPS @@ -0,0 +1,3 @@ +include_rules = [ + "+../../../../../../../chrome/browser/ui/translate", +] \ No newline at end of file diff --git a/chromium_src/components/translate/core/browser/DEPS b/chromium_src/components/translate/core/browser/DEPS index daa8c8b0b84db..4d7f7cfd55b15 100644 --- a/chromium_src/components/translate/core/browser/DEPS +++ b/chromium_src/components/translate/core/browser/DEPS @@ -1,5 +1,6 @@ include_rules = [ "+../../../../../../components/translate/core/browser", + "+../../../../../../../components/translate/core/browser", "+components/translate/core/browser", "+components/translate/core/common", "+components/language/core/browser", diff --git a/test/BUILD.gn b/test/BUILD.gn index d701c94d18d3c..3feca9d9913de 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -233,6 +233,7 @@ test("brave_unit_tests") { "//components/country_codes", "//components/domain_reliability:domain_reliability", "//components/language/core/browser", + "//components/language/core/common", "//components/os_crypt:test_support", "//components/page_load_metrics/common", "//components/permissions", From 88d356b55bdb587497da15af222f491544ed47e0 Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Tue, 5 Oct 2021 18:45:20 +0700 Subject: [PATCH 06/16] Add static_assert() --- .../translate/core/browser/translate_manager.cc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/chromium_src/components/translate/core/browser/translate_manager.cc b/chromium_src/components/translate/core/browser/translate_manager.cc index 5f101f55233bc..c19dfa5e0ac5e 100644 --- a/chromium_src/components/translate/core/browser/translate_manager.cc +++ b/chromium_src/components/translate/core/browser/translate_manager.cc @@ -21,9 +21,12 @@ class BraveIsSupportedTargetLanguageProxy : public TranslateDownloadManager { } // namespace translate -#define GetRecentTargetLanguage \ - GetRecentTargetLanguage(); \ - using TranslateDownloadManager = BraveIsSupportedTargetLanguageProxy; \ +#define GetRecentTargetLanguage \ + GetRecentTargetLanguage(); \ + static_assert(base::StringPiece(__FUNCTION__).find("GetTargetLanguage") != \ + base::StringPiece::npos, \ + "bad override, should work only for GetTargetLanguage"); \ + using TranslateDownloadManager = BraveIsSupportedTargetLanguageProxy; \ void #define HasAPIKeyConfigured BraveHasAPIKeyConfigured #define TranslateManager ChromiumTranslateManager From c73fbac9e50c47af22825fa3ea188147595f19d4 Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Tue, 5 Oct 2021 20:00:17 +0700 Subject: [PATCH 07/16] Fix network redirection unit tests --- ...direct_network_delegate_helper_unittest.cc | 28 ++++++++----------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/browser/net/brave_translate_redirect_network_delegate_helper_unittest.cc b/browser/net/brave_translate_redirect_network_delegate_helper_unittest.cc index c57feb02a5590..a502d9e838d3f 100644 --- a/browser/net/brave_translate_redirect_network_delegate_helper_unittest.cc +++ b/browser/net/brave_translate_redirect_network_delegate_helper_unittest.cc @@ -15,7 +15,6 @@ #include "net/traffic_annotation/network_traffic_annotation_test_helper.h" #include "net/url_request/url_request_test_util.h" #include "url/gurl.h" -// #include "url/url_constants.h" namespace { @@ -23,14 +22,14 @@ class BraveTranslateRedirectNetworkDelegateHelperTest : public testing::Test { public: BraveTranslateRedirectNetworkDelegateHelperTest() - : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP), + : task_environment_(content::BrowserTaskEnvironment::IO_MAINLOOP), context_(new net::TestURLRequestContext(true)) {} ~BraveTranslateRedirectNetworkDelegateHelperTest() override {} void SetUp() override { context_->Init(); } net::TestURLRequestContext* context() { return context_.get(); } private: - content::TestBrowserThreadBundle thread_bundle_; + content::BrowserTaskEnvironment task_environment_; std::unique_ptr context_; }; @@ -52,8 +51,8 @@ TEST_F(BraveTranslateRedirectNetworkDelegateHelperTest, std::shared_ptr before_url_context( new brave::BraveRequestInfo()); - brave::BraveRequestInfo::FillCTXFromRequest(request.get(), - before_url_context); + before_url_context->initiator_url = GURL(kTranslateInitiatorURL); + before_url_context->request_url = url; brave::ResponseCallback callback; GURL expected_url(kBraveTranslateServer + path_string); @@ -81,8 +80,7 @@ TEST_F(BraveTranslateRedirectNetworkDelegateHelperTest, std::shared_ptr before_url_context( new brave::BraveRequestInfo()); - brave::BraveRequestInfo::FillCTXFromRequest(request.get(), - before_url_context); + before_url_context->request_url = url; brave::ResponseCallback callback; int ret = OnBeforeURLRequest_TranslateRedirectWork(callback, @@ -110,8 +108,7 @@ TEST_F(BraveTranslateRedirectNetworkDelegateHelperTest, std::shared_ptr before_url_context( new brave::BraveRequestInfo()); - brave::BraveRequestInfo::FillCTXFromRequest(request.get(), - before_url_context); + before_url_context->request_url = url; brave::ResponseCallback callback; GURL expected_url(kBraveTranslateServer + url.path()); @@ -137,8 +134,8 @@ TEST_F(BraveTranslateRedirectNetworkDelegateHelperTest, std::shared_ptr before_url_context( new brave::BraveRequestInfo()); - brave::BraveRequestInfo::FillCTXFromRequest(request.get(), - before_url_context); + before_url_context->initiator_url = GURL(kTranslateInitiatorURL); + before_url_context->request_url = url; brave::ResponseCallback callback; GURL expected_url(kBraveTranslateEndpoint + std::string("?") + url.query()); @@ -162,8 +159,7 @@ TEST_F(BraveTranslateRedirectNetworkDelegateHelperTest, std::shared_ptr before_url_context( new brave::BraveRequestInfo()); - brave::BraveRequestInfo::FillCTXFromRequest(request.get(), - before_url_context); + before_url_context->request_url = url; brave::ResponseCallback callback; GURL expected_url(kBraveTranslateServer + url.path()); @@ -186,8 +182,7 @@ TEST_F(BraveTranslateRedirectNetworkDelegateHelperTest, std::shared_ptr before_url_context( new brave::BraveRequestInfo()); - brave::BraveRequestInfo::FillCTXFromRequest(request.get(), - before_url_context); + before_url_context->request_url = url; brave::ResponseCallback callback; int ret = OnBeforeURLRequest_TranslateRedirectWork(callback, @@ -210,8 +205,7 @@ TEST_F(BraveTranslateRedirectNetworkDelegateHelperTest, std::shared_ptr before_url_context( new brave::BraveRequestInfo()); - brave::BraveRequestInfo::FillCTXFromRequest(request.get(), - before_url_context); + before_url_context->request_url = url; brave::ResponseCallback callback; int ret = OnBeforeURLRequest_TranslateRedirectWork(callback, From 5ae95c06eb31f6dc18f23540a4fc933fecc4056f Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Wed, 6 Oct 2021 14:32:02 +0700 Subject: [PATCH 08/16] Fix whitespaces --- chromium_src/chrome/browser/ui/translate/DEPS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chromium_src/chrome/browser/ui/translate/DEPS b/chromium_src/chrome/browser/ui/translate/DEPS index bcc8bcd864d09..22f0697b05814 100644 --- a/chromium_src/chrome/browser/ui/translate/DEPS +++ b/chromium_src/chrome/browser/ui/translate/DEPS @@ -1,3 +1,3 @@ include_rules = [ "+../../../../../../../chrome/browser/ui/translate", -] \ No newline at end of file +] From 5b3cb9042c8813f3412a0c0e2b56094f257c5fe5 Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Wed, 6 Oct 2021 16:05:38 +0700 Subject: [PATCH 09/16] Formating --- .../browser/ui/translate/translate_bubble_model_impl.cc | 9 ++++++--- .../browser/ui/translate/translate_bubble_model_impl.h | 4 +++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc index a5a60a5d6c2fa..4959d5636e3e5 100644 --- a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc +++ b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc @@ -62,13 +62,16 @@ TranslateBubbleModelImpl::TranslateBubbleModelImpl( std::unique_ptr ui_delegate) : ChromiumTranslateBubbleModelImpl(step, std::move(ui_delegate)) { source_language_map_ = std::make_unique( - ui_delegate_.get(), base::BindRepeating(&translate::IsSourceLanguageCodeSupported)); + ui_delegate_.get(), + base::BindRepeating(&translate::IsSourceLanguageCodeSupported)); target_language_map_ = std::make_unique( - ui_delegate_.get(), base::BindRepeating(&translate::IsTargetLanguageCodeSupported)); + ui_delegate_.get(), + base::BindRepeating(&translate::IsTargetLanguageCodeSupported)); // If the source language is unsupported the drop it to unknown. // TODO(atuchin): is it good place to call this? - if (!translate::IsSourceLanguageCodeSupported(ui_delegate_->GetSourceLanguageCode())) { + if (!translate::IsSourceLanguageCodeSupported( + ui_delegate_->GetSourceLanguageCode())) { ui_delegate_->UpdateSourceLanguageIndex(0u); } } diff --git a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.h b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.h index cef1adc2924b0..584f96f2e5820 100644 --- a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.h +++ b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.h @@ -10,7 +10,9 @@ class TranslateBubbleModelImpl; using BraveTranslateBubbleModelImpl = TranslateBubbleModelImpl; #define TranslateBubbleModelImpl ChromiumTranslateBubbleModelImpl -#define translate_executed_ translate_executed_; friend BraveTranslateBubbleModelImpl +#define translate_executed_ \ + translate_executed_; \ + friend BraveTranslateBubbleModelImpl #include "../../../../../../../chrome/browser/ui/translate/translate_bubble_model_impl.h" #undef translate_executed_ #undef TranslateBubbleModelImpl From e04eae19515d577cc821ffe4fa5b5fcb4fd73c61 Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Thu, 7 Oct 2021 13:59:08 +0700 Subject: [PATCH 10/16] Update languages lists --- .../core/browser/brave_translate_language_filter.cc | 10 +++++++--- .../core/browser/brave_translate_language_filter.h | 4 ++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/components/translate/core/browser/brave_translate_language_filter.cc b/components/translate/core/browser/brave_translate_language_filter.cc index b97d21e4661b8..1b5b0296aa2da 100644 --- a/components/translate/core/browser/brave_translate_language_filter.cc +++ b/components/translate/core/browser/brave_translate_language_filter.cc @@ -8,12 +8,16 @@ namespace translate { bool IsSourceLanguageCodeSupported(const std::string& lang_code) { - return lang_code == "fr" || lang_code == "en" || lang_code == "de" || - lang_code == "und"; + // Note: keep sync with language/language.go (brave/go-translate repo) + return lang_code == "und" || lang_code == "en" || lang_code == "es" || + lang_code == "et" || lang_code == "it" || lang_code == "pt" || + lang_code == "ru"; } bool IsTargetLanguageCodeSupported(const std::string& lang_code) { - return lang_code == "en" || lang_code == "ru"; + // Note: keep sync with language/language.go (brave/go-translate repo) + return lang_code == "de" || lang_code == "en" || lang_code == "es" || + lang_code == "et" || lang_code == "ru"; } } // namespace translate diff --git a/components/translate/core/browser/brave_translate_language_filter.h b/components/translate/core/browser/brave_translate_language_filter.h index 66acd1bac215c..87e43b36c0524 100644 --- a/components/translate/core/browser/brave_translate_language_filter.h +++ b/components/translate/core/browser/brave_translate_language_filter.h @@ -10,8 +10,12 @@ namespace translate { +// Returns true if the source language |lang_code| is supported by Brave +// backend. bool IsSourceLanguageCodeSupported(const std::string& lang_code); +// Returns true if the target language |lang_code| is supported by Brave +// backend. bool IsTargetLanguageCodeSupported(const std::string& lang_code); } // namespace translate From b797b43476c8165081a5de493367859b5822c21b Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Thu, 7 Oct 2021 14:35:01 +0700 Subject: [PATCH 11/16] Minor improvements --- ...direct_network_delegate_helper_unittest.cc | 3 +- .../translate/translate_bubble_model_impl.cc | 38 +++++++++++++------ .../translate/translate_bubble_model_impl.h | 4 ++ 3 files changed, 31 insertions(+), 14 deletions(-) diff --git a/browser/net/brave_translate_redirect_network_delegate_helper_unittest.cc b/browser/net/brave_translate_redirect_network_delegate_helper_unittest.cc index a502d9e838d3f..fee7da292c840 100644 --- a/browser/net/brave_translate_redirect_network_delegate_helper_unittest.cc +++ b/browser/net/brave_translate_redirect_network_delegate_helper_unittest.cc @@ -22,8 +22,7 @@ class BraveTranslateRedirectNetworkDelegateHelperTest : public testing::Test { public: BraveTranslateRedirectNetworkDelegateHelperTest() - : task_environment_(content::BrowserTaskEnvironment::IO_MAINLOOP), - context_(new net::TestURLRequestContext(true)) {} + : context_(new net::TestURLRequestContext(true)) {} ~BraveTranslateRedirectNetworkDelegateHelperTest() override {} void SetUp() override { context_->Init(); } net::TestURLRequestContext* context() { return context_.get(); } diff --git a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc index 4959d5636e3e5..8d83d61c7c111 100644 --- a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc +++ b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc @@ -16,6 +16,9 @@ namespace { const int kNoIndex = static_cast(translate::TranslateUIDelegate::kNoIndex); } // namespace +// A mapping between a chromium languages list in |ui_delegate| and a brave +// languages list (a limited subset of chromium list preassigned by |filter|). +// Note: mixin of int/size_t types is used because of chromium implementation. class BraveLanguageMap { public: BraveLanguageMap(const translate::TranslateUIDelegate* ui_delegate, @@ -33,28 +36,39 @@ class BraveLanguageMap { DCHECK_EQ(to_ui_index_.size(), to_core_index_.size()); #if DCHECK_IS_ON() for (int i = 0; i < static_cast(to_core_index_.size()); ++i) { - const int core_ind = to_core_index_[i]; + const auto core_ind = to_core_index_[i]; DCHECK_EQ(to_ui_index_[to_core_index_[i]], i); DCHECK_EQ(to_core_index_[to_ui_index_[core_ind]], core_ind); } #endif // DCHECK_IS_ON() } + + // Coverts index in brave list (index in [0, GetSize() - 1]) to a correspoding + // chromium index. size_t ToCoreIndex(int index) const { - if (index == kNoIndex || !base::Contains(to_core_index_, index)) + if (index == kNoIndex) return translate::TranslateUIDelegate::kNoIndex; - return to_core_index_.at(index); + const auto it = to_core_index_.find(index); + return it != to_core_index_.end() + ? it->second + : translate::TranslateUIDelegate::kNoIndex; } - size_t ToUiIndex(int index) const { - if (index == kNoIndex || !base::Contains(to_ui_index_, index)) - return translate::TranslateUIDelegate::kNoIndex; - return to_ui_index_.at(index); + + // An inverse function to ToCoreIndex(). Coverts chromium index + // (form [0, ui_delegate->GetNumberOfLanguages()] to a + // correspoding index in brave list. + int FromCoreIndex(size_t index) const { + if (index == translate::TranslateUIDelegate::kNoIndex) + return kNoIndex; + const auto it = to_ui_index_.find(index); + return it != to_ui_index_.end() ? it->second : kNoIndex; } int GetSize() const { return to_core_index_.size(); } private: - std::map to_core_index_; - std::map to_ui_index_; + std::map to_core_index_; + std::map to_ui_index_; }; TranslateBubbleModelImpl::TranslateBubbleModelImpl( @@ -68,7 +82,7 @@ TranslateBubbleModelImpl::TranslateBubbleModelImpl( ui_delegate_.get(), base::BindRepeating(&translate::IsTargetLanguageCodeSupported)); - // If the source language is unsupported the drop it to unknown. + // If the source language is unsupported then drop it to unknown. // TODO(atuchin): is it good place to call this? if (!translate::IsSourceLanguageCodeSupported( ui_delegate_->GetSourceLanguageCode())) { @@ -99,12 +113,12 @@ std::u16string TranslateBubbleModelImpl::GetTargetLanguageNameAt( } int TranslateBubbleModelImpl::GetSourceLanguageIndex() const { - return source_language_map_->ToUiIndex( + return source_language_map_->FromCoreIndex( ui_delegate_->GetSourceLanguageIndex()); } int TranslateBubbleModelImpl::GetTargetLanguageIndex() const { - return target_language_map_->ToUiIndex( + return target_language_map_->FromCoreIndex( ui_delegate_->GetTargetLanguageIndex()); } diff --git a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.h b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.h index 584f96f2e5820..32c1e74dd1dfb 100644 --- a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.h +++ b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.h @@ -18,6 +18,10 @@ using BraveTranslateBubbleModelImpl = TranslateBubbleModelImpl; #undef TranslateBubbleModelImpl class BraveLanguageMap; + +// Brave customization of TranslateBubbleModelImpl to uses separated lists for +// source and target languages. Holds two mappings between chromium list in +// |ui_delegate| and brave lists. class TranslateBubbleModelImpl : public ChromiumTranslateBubbleModelImpl { public: TranslateBubbleModelImpl( From ecddf82a8024797db34824cb48ef6fa53d2951ab Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Thu, 7 Oct 2021 15:05:23 +0700 Subject: [PATCH 12/16] Fix comment --- .../browser/ui/translate/translate_bubble_model_impl.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.h b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.h index 32c1e74dd1dfb..9f696faae1e0b 100644 --- a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.h +++ b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.h @@ -19,9 +19,9 @@ using BraveTranslateBubbleModelImpl = TranslateBubbleModelImpl; class BraveLanguageMap; -// Brave customization of TranslateBubbleModelImpl to uses separated lists for -// source and target languages. Holds two mappings between chromium list in -// |ui_delegate| and brave lists. +// Brave customization of TranslateBubbleModelImpl that uses separated lists for +// source and target languages. Holds two mappings between chromium languages +// list in |ui_delegate| and brave lists. class TranslateBubbleModelImpl : public ChromiumTranslateBubbleModelImpl { public: TranslateBubbleModelImpl( From 2eb78ed1c90e4560c2b5aa4a7124a89714882155 Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Thu, 7 Oct 2021 19:59:39 +0700 Subject: [PATCH 13/16] Review fixes --- chromium_src/chrome/browser/ui/translate/DEPS | 2 +- .../translate/translate_bubble_model_impl.cc | 8 ++++--- .../translate/translate_bubble_model_impl.h | 2 +- .../components/translate/core/browser/DEPS | 1 - .../core/browser/translate_manager.cc | 22 +++++++++++++------ .../core/browser/translate_manager.h | 5 ++++- components/translate/core/browser/BUILD.gn | 1 + .../brave_translate_language_filter.cc | 21 ++++++++++++------ 8 files changed, 41 insertions(+), 21 deletions(-) diff --git a/chromium_src/chrome/browser/ui/translate/DEPS b/chromium_src/chrome/browser/ui/translate/DEPS index 22f0697b05814..c4f1c33d02487 100644 --- a/chromium_src/chrome/browser/ui/translate/DEPS +++ b/chromium_src/chrome/browser/ui/translate/DEPS @@ -1,3 +1,3 @@ include_rules = [ - "+../../../../../../../chrome/browser/ui/translate", + "+../../../../../../chrome/browser/ui/translate", ] diff --git a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc index 8d83d61c7c111..4fe77aeac7292 100644 --- a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc +++ b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc @@ -6,7 +6,7 @@ #include "chrome/browser/ui/translate/translate_bubble_model_impl.h" #define TranslateBubbleModelImpl ChromiumTranslateBubbleModelImpl -#include "../../../../../../../chrome/browser/ui/translate/translate_bubble_model_impl.cc" +#include "../../../../../../chrome/browser/ui/translate/translate_bubble_model_impl.cc" #undef TranslateBubbleModelImpl #include "base/containers/contains.h" @@ -82,8 +82,10 @@ TranslateBubbleModelImpl::TranslateBubbleModelImpl( ui_delegate_.get(), base::BindRepeating(&translate::IsTargetLanguageCodeSupported)); - // If the source language is unsupported then drop it to unknown. - // TODO(atuchin): is it good place to call this? + // If the source language is unsupported then drop it to "und". + // Theoretically isn't the same as creating ui_delegate with source_lang == + // und, because |initial_source_language_index_| hasn't been updated. But in + // fact chromium doesn't use |initial_source_language_index_| at all. if (!translate::IsSourceLanguageCodeSupported( ui_delegate_->GetSourceLanguageCode())) { ui_delegate_->UpdateSourceLanguageIndex(0u); diff --git a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.h b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.h index 9f696faae1e0b..42f78b3893f6f 100644 --- a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.h +++ b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.h @@ -13,7 +13,7 @@ using BraveTranslateBubbleModelImpl = TranslateBubbleModelImpl; #define translate_executed_ \ translate_executed_; \ friend BraveTranslateBubbleModelImpl -#include "../../../../../../../chrome/browser/ui/translate/translate_bubble_model_impl.h" +#include "../../../../../../chrome/browser/ui/translate/translate_bubble_model_impl.h" #undef translate_executed_ #undef TranslateBubbleModelImpl diff --git a/chromium_src/components/translate/core/browser/DEPS b/chromium_src/components/translate/core/browser/DEPS index 4d7f7cfd55b15..daa8c8b0b84db 100644 --- a/chromium_src/components/translate/core/browser/DEPS +++ b/chromium_src/components/translate/core/browser/DEPS @@ -1,6 +1,5 @@ include_rules = [ "+../../../../../../components/translate/core/browser", - "+../../../../../../../components/translate/core/browser", "+components/translate/core/browser", "+components/translate/core/common", "+components/language/core/browser", diff --git a/chromium_src/components/translate/core/browser/translate_manager.cc b/chromium_src/components/translate/core/browser/translate_manager.cc index c19dfa5e0ac5e..6185673b8aef0 100644 --- a/chromium_src/components/translate/core/browser/translate_manager.cc +++ b/chromium_src/components/translate/core/browser/translate_manager.cc @@ -20,19 +20,21 @@ class BraveIsSupportedTargetLanguageProxy : public TranslateDownloadManager { }; } // namespace translate +#define ASSERT_INSIDE_FUNCTION(fname) \ + static_assert(base::StringPiece(__FUNCTION__) == fname, \ + "should work only within " fname " function"); -#define GetRecentTargetLanguage \ - GetRecentTargetLanguage(); \ - static_assert(base::StringPiece(__FUNCTION__).find("GetTargetLanguage") != \ - base::StringPiece::npos, \ - "bad override, should work only for GetTargetLanguage"); \ - using TranslateDownloadManager = BraveIsSupportedTargetLanguageProxy; \ +#define GetRecentTargetLanguage \ + GetRecentTargetLanguage(); \ + ASSERT_INSIDE_FUNCTION("GetTargetLanguage")\ + using TranslateDownloadManager = BraveIsSupportedTargetLanguageProxy; \ void #define HasAPIKeyConfigured BraveHasAPIKeyConfigured #define TranslateManager ChromiumTranslateManager -#include "../../../../../../components/translate/core/browser/translate_manager.cc" // NOLINT +#include "../../../../../../components/translate/core/browser/translate_manager.cc" #undef HasAPIKeyConfigured #undef TranslateManager +#undef ASSERT_INSIDE_FUNCTION namespace translate { @@ -43,6 +45,9 @@ void TranslateManager::FilterIsTranslatePossible( const std::string& target_lang) { ChromiumTranslateManager::FilterIsTranslatePossible( decision, translate_prefs, page_language_code, target_lang); + // The source language is not supported by Brave backend. Current we allow a + // user to trigger a manual translation to have a chance to fix if it wasn't + // recognized correctly (in that case it will shown as "Detect Language"). if (!IsSourceLanguageCodeSupported(page_language_code)) { decision->PreventAutoTranslate(); decision->PreventShowingUI(); @@ -54,6 +59,9 @@ void TranslateManager::FilterIsTranslatePossible( TriggerDecision::kDisabledUnsupportedLanguage); } + // Just in case. In general a user can't trigger a translation to an + // unsupported target language. But some new entry points can be added so + // block translation in that case. if (!IsTargetLanguageCodeSupported(target_lang)) { decision->PreventAllTriggering(); decision->initiation_statuses.push_back( diff --git a/chromium_src/components/translate/core/browser/translate_manager.h b/chromium_src/components/translate/core/browser/translate_manager.h index 18f38d81b078c..55bba3e6920e1 100644 --- a/chromium_src/components/translate/core/browser/translate_manager.h +++ b/chromium_src/components/translate/core/browser/translate_manager.h @@ -16,13 +16,16 @@ using BraveTranslateManager = TranslateManager; ignore_missing_key_for_testing_; \ friend BraveTranslateManager #define FilterIsTranslatePossible virtual FilterIsTranslatePossible -#include "../../../../../../../components/translate/core/browser/translate_manager.h" +#include "../../../../../../components/translate/core/browser/translate_manager.h" #undef FilterIsTranslatePossible #undef ignore_missing_key_for_testing_ #undef TranslateManager namespace translate { +// Brave customization of TranslateManager that limit the number of supported +// languages. Also two independet lists are used for source and target +// languages. class TranslateManager : public ChromiumTranslateManager, public base::SupportsWeakPtr { public: diff --git a/components/translate/core/browser/BUILD.gn b/components/translate/core/browser/BUILD.gn index 1715ff6948ed9..522b8ec82862e 100644 --- a/components/translate/core/browser/BUILD.gn +++ b/components/translate/core/browser/BUILD.gn @@ -3,6 +3,7 @@ source_set("browser") { "brave_translate_language_filter.cc", "brave_translate_language_filter.h", ] + deps = [ "//base" ] public_deps = [ # TODO(jocelyn): move buildflags to brave/components/translate/core/browser "//brave/browser/translate/buildflags", diff --git a/components/translate/core/browser/brave_translate_language_filter.cc b/components/translate/core/browser/brave_translate_language_filter.cc index 1b5b0296aa2da..20ed04aabc631 100644 --- a/components/translate/core/browser/brave_translate_language_filter.cc +++ b/components/translate/core/browser/brave_translate_language_filter.cc @@ -5,19 +5,26 @@ #include "brave/components/translate/core/browser/brave_translate_language_filter.h" +#include "base/containers/contains.h" +#include "base/strings/string_piece.h" + namespace translate { +namespace { +// Note: keep sync with language/language.go (brave/go-translate repo) +constexpr base::StringPiece kSourceLanguages[] = {"und", "en", "es", "et", + "it", "pt", "ru"}; + +// Note: keep sync with language/language.go (brave/go-translate repo) +constexpr base::StringPiece kTargetLanguages[] = {"de", "en", "es", "et", "ru"}; + +} // namespace bool IsSourceLanguageCodeSupported(const std::string& lang_code) { - // Note: keep sync with language/language.go (brave/go-translate repo) - return lang_code == "und" || lang_code == "en" || lang_code == "es" || - lang_code == "et" || lang_code == "it" || lang_code == "pt" || - lang_code == "ru"; + return base::Contains(kSourceLanguages, lang_code); } bool IsTargetLanguageCodeSupported(const std::string& lang_code) { - // Note: keep sync with language/language.go (brave/go-translate repo) - return lang_code == "de" || lang_code == "en" || lang_code == "es" || - lang_code == "et" || lang_code == "ru"; + return base::Contains(kTargetLanguages, lang_code); } } // namespace translate From 3e26365e748647f7032a9a8ae2590c112a348994 Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Thu, 7 Oct 2021 20:31:06 +0700 Subject: [PATCH 14/16] Fix more review issues --- .../brave_translate_redirect_network_delegate_helper.cc | 1 + .../browser/ui/translate/translate_bubble_model_impl.cc | 2 +- .../translate/core/browser/translate_manager.cc | 8 ++++---- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/browser/net/brave_translate_redirect_network_delegate_helper.cc b/browser/net/brave_translate_redirect_network_delegate_helper.cc index 354a1cc035937..1d98e722ee543 100644 --- a/browser/net/brave_translate_redirect_network_delegate_helper.cc +++ b/browser/net/brave_translate_redirect_network_delegate_helper.cc @@ -8,6 +8,7 @@ #include #include #include + #include "brave/common/translate_network_constants.h" #include "extensions/common/url_pattern.h" #include "net/base/net_errors.h" diff --git a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc index 4fe77aeac7292..b1a13ee762bcc 100644 --- a/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc +++ b/chromium_src/chrome/browser/ui/translate/translate_bubble_model_impl.cc @@ -83,7 +83,7 @@ TranslateBubbleModelImpl::TranslateBubbleModelImpl( base::BindRepeating(&translate::IsTargetLanguageCodeSupported)); // If the source language is unsupported then drop it to "und". - // Theoretically isn't the same as creating ui_delegate with source_lang == + // Theoretically it isn't the same as creating ui_delegate with source_lang == // und, because |initial_source_language_index_| hasn't been updated. But in // fact chromium doesn't use |initial_source_language_index_| at all. if (!translate::IsSourceLanguageCodeSupported( diff --git a/chromium_src/components/translate/core/browser/translate_manager.cc b/chromium_src/components/translate/core/browser/translate_manager.cc index 6185673b8aef0..47b7e6d9cb6d3 100644 --- a/chromium_src/components/translate/core/browser/translate_manager.cc +++ b/chromium_src/components/translate/core/browser/translate_manager.cc @@ -26,7 +26,7 @@ class BraveIsSupportedTargetLanguageProxy : public TranslateDownloadManager { #define GetRecentTargetLanguage \ GetRecentTargetLanguage(); \ - ASSERT_INSIDE_FUNCTION("GetTargetLanguage")\ + ASSERT_INSIDE_FUNCTION("GetTargetLanguage") \ using TranslateDownloadManager = BraveIsSupportedTargetLanguageProxy; \ void #define HasAPIKeyConfigured BraveHasAPIKeyConfigured @@ -45,9 +45,9 @@ void TranslateManager::FilterIsTranslatePossible( const std::string& target_lang) { ChromiumTranslateManager::FilterIsTranslatePossible( decision, translate_prefs, page_language_code, target_lang); - // The source language is not supported by Brave backend. Current we allow a - // user to trigger a manual translation to have a chance to fix if it wasn't - // recognized correctly (in that case it will shown as "Detect Language"). + // The source language is not supported by Brave backend. Currently we allow a + // user to trigger a manual translation to have a chance to change the + // incorrectly recognized source language to the correct one. if (!IsSourceLanguageCodeSupported(page_language_code)) { decision->PreventAutoTranslate(); decision->PreventShowingUI(); From 3461bbdeef7fc722866916f673475fcfb6d04229 Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Fri, 8 Oct 2021 15:20:24 +0700 Subject: [PATCH 15/16] switch off the buildflag --- browser/translate/buildflags/buildflags.gni | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/browser/translate/buildflags/buildflags.gni b/browser/translate/buildflags/buildflags.gni index a7dcb77633470..8158a0136f7eb 100644 --- a/browser/translate/buildflags/buildflags.gni +++ b/browser/translate/buildflags/buildflags.gni @@ -2,7 +2,7 @@ import("//brave/build/config.gni") import("//extensions/buildflags/buildflags.gni") declare_args() { - enable_brave_translate_go = true + enable_brave_translate_go = false } declare_args() { From 877fef495faac973a4b5cb74de9259bd3d0e53c3 Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Fri, 8 Oct 2021 21:19:31 +0700 Subject: [PATCH 16/16] Hide language filtering in manager under the buildflag --- chromium_src/components/translate/core/browser/DEPS | 3 +++ .../components/translate/core/browser/translate_manager.cc | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/chromium_src/components/translate/core/browser/DEPS b/chromium_src/components/translate/core/browser/DEPS index daa8c8b0b84db..97dba98cebc03 100644 --- a/chromium_src/components/translate/core/browser/DEPS +++ b/chromium_src/components/translate/core/browser/DEPS @@ -14,4 +14,7 @@ specific_include_rules = { "translate_url_fetcher(\.cc|\.h)": [ "+brave/browser/translate/buildflags/buildflags.h", ], + "translate_manager(\.cc|\.h)": [ + "+brave/browser/translate/buildflags/buildflags.h", + ], } diff --git a/chromium_src/components/translate/core/browser/translate_manager.cc b/chromium_src/components/translate/core/browser/translate_manager.cc index 47b7e6d9cb6d3..80ec9fdb58b61 100644 --- a/chromium_src/components/translate/core/browser/translate_manager.cc +++ b/chromium_src/components/translate/core/browser/translate_manager.cc @@ -5,6 +5,7 @@ #include "components/translate/core/browser/translate_manager.h" +#include "brave/browser/translate/buildflags/buildflags.h" #include "brave/components/translate/core/browser/brave_translate_language_filter.h" #include "components/translate/core/browser/translate_download_manager.h" #include "components/translate/core/browser/translate_prefs.h" @@ -13,9 +14,11 @@ namespace translate { class BraveIsSupportedTargetLanguageProxy : public TranslateDownloadManager { public: +#if BUILDFLAG(ENABLE_BRAVE_TRANSLATE_GO) static bool IsSupportedLanguage(const std::string& lang) { return IsTargetLanguageCodeSupported(lang); } +#endif // BUILDFLAG(ENABLE_BRAVE_TRANSLATE_GO) ~BraveIsSupportedTargetLanguageProxy() override; }; @@ -45,6 +48,7 @@ void TranslateManager::FilterIsTranslatePossible( const std::string& target_lang) { ChromiumTranslateManager::FilterIsTranslatePossible( decision, translate_prefs, page_language_code, target_lang); +#if BUILDFLAG(ENABLE_BRAVE_TRANSLATE_GO) // The source language is not supported by Brave backend. Currently we allow a // user to trigger a manual translation to have a chance to change the // incorrectly recognized source language to the correct one. @@ -71,6 +75,7 @@ void TranslateManager::FilterIsTranslatePossible( GetActiveTranslateMetricsLogger()->LogTriggerDecision( TriggerDecision::kDisabledUnsupportedLanguage); } +#endif // BUILDFLAG(ENABLE_BRAVE_TRANSLATE_GO) } base::WeakPtr TranslateManager::GetWeakPtr() {