Commit 923926a4 authored by Alexandre Frechette's avatar Alexandre Frechette Committed by Commit Bot

[Translate Manual Trigger] Manual translation implementation that waits for source language.

The translate UI should not appear before the necessary information (i.e. source language) has been computed.
This changes manual translations to block before the state is correct.

Bug: 902803
Change-Id: I0d0549b5093774ae5b60dd0e18fd72fe7349f895
Reviewed-on: https://chromium-review.googlesource.com/c/1331688Reviewed-by: default avatarTheresa <twellington@chromium.org>
Reviewed-by: default avataranthonyvd <anthonyvd@chromium.org>
Commit-Queue: Alexandre Frechette <frechette@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608352}
parent 19f0ea3b
......@@ -2141,7 +2141,7 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
RecordUserAction.record("MobileMenuTranslate");
Tracker tracker = TrackerFactory.getTrackerForProfile(getActivityTab().getProfile());
tracker.notifyEvent(EventConstants.TRANSLATE_MENU_BUTTON_CLICKED);
TranslateBridge.translateTab(getActivityTab());
TranslateBridge.translateTabWhenReady(getActivityTab());
} else if (id == R.id.share_menu_id || id == R.id.direct_share_menu_id) {
onShareMenuItemSelected(id == R.id.direct_share_menu_id,
getCurrentTabModel().isIncognito());
......
......@@ -12,10 +12,10 @@ import org.chromium.content_public.browser.WebContents;
*/
public class TranslateBridge {
/**
* Translates the given tab.
* Translates the given tab when the necessary state has been computed (e.g. source language).
*/
public static void translateTab(Tab tab) {
nativeTranslate(tab.getWebContents());
public static void translateTabWhenReady(Tab tab) {
nativeManualTranslateWhenReady(tab.getWebContents());
}
/**
......@@ -32,7 +32,7 @@ public class TranslateBridge {
return nativeShouldShowManualTranslateIPH(tab.getWebContents());
}
private static native void nativeTranslate(WebContents webContents);
private static native void nativeManualTranslateWhenReady(WebContents webContents);
private static native boolean nativeCanManuallyTranslate(WebContents webContents);
private static native boolean nativeShouldShowManualTranslateIPH(WebContents webContents);
}
......@@ -5,37 +5,41 @@
#include "base/android/jni_string.h"
#include "chrome/browser/translate/chrome_translate_client.h"
#include "components/language/core/common/language_experiments.h"
#include "components/translate/content/browser/content_translate_driver.h"
#include "components/translate/core/browser/translate_manager.h"
#include "components/translate/core/browser/translate_prefs.h"
#include "content/public/browser/web_contents.h"
#include "jni/TranslateBridge_jni.h"
static translate::TranslateManager* GetTranslateManager(
static ChromeTranslateClient* GetTranslateClient(
const base::android::JavaParamRef<jobject>& j_web_contents) {
content::WebContents* web_contents =
content::WebContents::FromJavaWebContents(j_web_contents);
ChromeTranslateClient* chrome_translate_client =
ChromeTranslateClient* client =
ChromeTranslateClient::FromWebContents(web_contents);
DCHECK(chrome_translate_client);
translate::TranslateManager* manager =
chrome_translate_client->GetTranslateManager();
DCHECK(manager);
return manager;
DCHECK(client);
return client;
}
static void JNI_TranslateBridge_Translate(
static void JNI_TranslateBridge_ManualTranslateWhenReady(
JNIEnv* env,
const base::android::JavaParamRef<jclass>& jcaller,
const base::android::JavaParamRef<jobject>& j_web_contents) {
translate::TranslateManager* manager = GetTranslateManager(j_web_contents);
manager->InitiateManualTranslation();
content::WebContents* web_contents =
content::WebContents::FromJavaWebContents(j_web_contents);
ChromeTranslateClient* client =
ChromeTranslateClient::FromWebContents(web_contents);
DCHECK(client);
client->ManualTranslateWhenReady();
}
static jboolean JNI_TranslateBridge_CanManuallyTranslate(
JNIEnv* env,
const base::android::JavaParamRef<jclass>& jcaller,
const base::android::JavaParamRef<jobject>& j_web_contents) {
translate::TranslateManager* manager = GetTranslateManager(j_web_contents);
ChromeTranslateClient* client = GetTranslateClient(j_web_contents);
translate::TranslateManager* manager = client->GetTranslateManager();
DCHECK(manager);
return manager->CanManuallyTranslate();
}
......@@ -43,18 +47,13 @@ static jboolean JNI_TranslateBridge_ShouldShowManualTranslateIPH(
JNIEnv* env,
const base::android::JavaParamRef<jclass>& jcaller,
const base::android::JavaParamRef<jobject>& j_web_contents) {
content::WebContents* web_contents =
content::WebContents::FromJavaWebContents(j_web_contents);
ChromeTranslateClient* chrome_translate_client =
ChromeTranslateClient::FromWebContents(web_contents);
DCHECK(chrome_translate_client);
translate::TranslateManager* manager =
chrome_translate_client->GetTranslateManager();
ChromeTranslateClient* client = GetTranslateClient(j_web_contents);
translate::TranslateManager* manager = client->GetTranslateManager();
DCHECK(manager);
const std::string page_lang = manager->GetLanguageState().original_language();
std::unique_ptr<translate::TranslatePrefs> translate_prefs(
chrome_translate_client->GetTranslatePrefs());
client->GetTranslatePrefs());
return base::StartsWith(page_lang, "en",
base::CompareCase::INSENSITIVE_ASCII) &&
......
......@@ -293,6 +293,15 @@ ChromeTranslateClient::GetTranslateAcceptLanguages() {
int ChromeTranslateClient::GetInfobarIconID() const {
return IDR_ANDROID_INFOBAR_TRANSLATE;
}
void ChromeTranslateClient::ManualTranslateWhenReady() {
if (GetLanguageState().original_language().empty()) {
manual_translate_on_ready_ = true;
} else {
translate::TranslateManager* manager = GetTranslateManager();
manager->InitiateManualTranslation();
}
}
#endif
void ChromeTranslateClient::RecordLanguageDetectionEvent(
......@@ -362,6 +371,14 @@ void ChromeTranslateClient::OnLanguageDetermined(
content::Details<const translate::LanguageDetectionDetails>(&details));
RecordLanguageDetectionEvent(details);
#if defined(OS_ANDROID)
// See ChromeTranslateClient::ManualTranslateOnReady
if (manual_translate_on_ready_) {
GetTranslateManager()->InitiateManualTranslation();
manual_translate_on_ready_ = false;
}
#endif
}
void ChromeTranslateClient::OnPageTranslated(
......
......@@ -92,6 +92,10 @@ class ChromeTranslateClient
std::unique_ptr<translate::TranslateInfoBarDelegate> delegate)
const override;
int GetInfobarIconID() const override;
// Trigger a manual translation when the necessary state (e.g. source
// language) is ready.
void ManualTranslateWhenReady();
#endif
void RecordLanguageDetectionEvent(
......@@ -134,6 +138,12 @@ class ChromeTranslateClient
translate::ContentTranslateDriver translate_driver_;
std::unique_ptr<translate::TranslateManager> translate_manager_;
#if defined(OS_ANDROID)
// Whether to trigger a manual translation when ready.
// See ChromeTranslateClient::ManualTranslateOnReady
bool manual_translate_on_ready_ = false;
#endif
DISALLOW_COPY_AND_ASSIGN(ChromeTranslateClient);
};
......
// Copyright 2014 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_fake_page.h"
#include <stddef.h>
#include <memory>
#include <set>
#include <tuple>
#include <utility>
#include <vector>
#include "build/build_config.h"
#include "chrome/browser/infobars/infobar_service.h"
#include "chrome/browser/renderer_context_menu/render_view_context_menu_test_util.h"
#include "chrome/browser/translate/chrome_translate_client.h"
#include "chrome/browser/translate/translate_service.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/test/base/chrome_render_view_host_test_harness.h"
#include "chrome/test/base/testing_browser_process.h"
#include "chrome/test/base/testing_profile.h"
#include "components/infobars/core/infobar.h"
#include "components/infobars/core/infobar_manager.h"
#include "components/translate/content/browser/content_translate_driver.h"
#include "components/translate/content/common/translate.mojom.h"
#include "components/translate/core/browser/translate_infobar_delegate.h"
#include "components/translate/core/browser/translate_manager.h"
#include "components/translate/core/browser/translate_ui_delegate.h"
#include "components/translate/core/common/language_detection_details.h"
#include "content/public/browser/navigation_details.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/url_constants.h"
#include "content/public/test/navigation_simulator.h"
#include "content/public/test/test_renderer_host.h"
#include "mojo/public/cpp/bindings/binding.h"
#include "url/gurl.h"
FakePageImpl::FakePageImpl()
: called_translate_(false),
called_revert_translation_(false),
binding_(this) {}
FakePageImpl::~FakePageImpl(){};
translate::mojom::PagePtr FakePageImpl::BindToNewPagePtr() {
binding_.Close();
translate_callback_pending_.Reset();
translate::mojom::PagePtr page;
binding_.Bind(mojo::MakeRequest(&page));
return page;
}
// translate::mojom::Page implementation.
void FakePageImpl::Translate(const std::string& translate_script,
const std::string& source_lang,
const std::string& target_lang,
TranslateCallback callback) {
// Ensure pending callback gets called.
if (translate_callback_pending_) {
std::move(translate_callback_pending_)
.Run(true, "", "", translate::TranslateErrors::NONE);
}
called_translate_ = true;
source_lang_ = source_lang;
target_lang_ = target_lang;
translate_callback_pending_ = std::move(callback);
}
void FakePageImpl::RevertTranslation() {
called_revert_translation_ = true;
}
void FakePageImpl::PageTranslated(bool cancelled,
const std::string& source_lang,
const std::string& target_lang,
translate::TranslateErrors::Type error) {
std::move(translate_callback_pending_)
.Run(cancelled, source_lang, target_lang, error);
}
// Copyright 2014 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_FAKE_PAGE_H_
#define CHROME_BROWSER_TRANSLATE_TRANSLATE_FAKE_PAGE_H_
#include <stddef.h>
#include <memory>
#include <set>
#include <tuple>
#include <utility>
#include <vector>
#include "build/build_config.h"
#include "chrome/browser/infobars/infobar_service.h"
#include "chrome/browser/renderer_context_menu/render_view_context_menu_test_util.h"
#include "chrome/browser/translate/chrome_translate_client.h"
#include "chrome/browser/translate/translate_service.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/test/base/chrome_render_view_host_test_harness.h"
#include "chrome/test/base/testing_browser_process.h"
#include "chrome/test/base/testing_profile.h"
#include "components/infobars/core/infobar.h"
#include "components/infobars/core/infobar_manager.h"
#include "components/translate/content/browser/content_translate_driver.h"
#include "components/translate/content/common/translate.mojom.h"
#include "components/translate/core/browser/translate_infobar_delegate.h"
#include "components/translate/core/browser/translate_manager.h"
#include "components/translate/core/browser/translate_ui_delegate.h"
#include "components/translate/core/common/language_detection_details.h"
#include "content/public/browser/navigation_details.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/url_constants.h"
#include "content/public/test/navigation_simulator.h"
#include "content/public/test/test_renderer_host.h"
#include "mojo/public/cpp/bindings/binding.h"
#include "url/gurl.h"
class FakePageImpl : public translate::mojom::Page {
public:
FakePageImpl();
~FakePageImpl() override;
translate::mojom::PagePtr BindToNewPagePtr();
// translate::mojom::Page implementation.
void Translate(const std::string& translate_script,
const std::string& source_lang,
const std::string& target_lang,
TranslateCallback callback) override;
void RevertTranslation() override;
void PageTranslated(bool cancelled,
const std::string& source_lang,
const std::string& target_lang,
translate::TranslateErrors::Type error);
bool called_translate_;
base::Optional<std::string> source_lang_;
base::Optional<std::string> target_lang_;
bool called_revert_translation_;
private:
TranslateCallback translate_callback_pending_;
mojo::Binding<translate::mojom::Page> binding_;
DISALLOW_COPY_AND_ASSIGN(FakePageImpl);
};
#endif // CHROME_BROWSER_TRANSLATE_TRANSLATE_FAKE_PAGE_H_
// Copyright 2014 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 <stddef.h>
#include <memory>
#include <set>
#include <tuple>
#include <utility>
#include <vector>
#include "build/build_config.h"
#include "chrome/browser/infobars/infobar_service.h"
#include "chrome/browser/renderer_context_menu/render_view_context_menu_test_util.h"
#include "chrome/browser/translate/chrome_translate_client.h"
#include "chrome/browser/translate/translate_fake_page.h"
#include "chrome/browser/translate/translate_service.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/test/base/chrome_render_view_host_test_harness.h"
#include "chrome/test/base/testing_browser_process.h"
#include "chrome/test/base/testing_profile.h"
#include "components/infobars/core/infobar.h"
#include "components/infobars/core/infobar_manager.h"
#include "components/translate/content/browser/content_translate_driver.h"
#include "components/translate/content/common/translate.mojom.h"
#include "components/translate/core/browser/translate_infobar_delegate.h"
#include "components/translate/core/browser/translate_manager.h"
#include "components/translate/core/browser/translate_ui_delegate.h"
#include "components/translate/core/common/language_detection_details.h"
#include "content/public/browser/navigation_details.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/url_constants.h"
#include "content/public/test/navigation_simulator.h"
#include "content/public/test/test_renderer_host.h"
#include "mojo/public/cpp/bindings/binding.h"
#include "url/gurl.h"
class TranslateManagerRenderViewHostAndroidTest
: public ChromeRenderViewHostTestHarness {
public:
TranslateManagerRenderViewHostAndroidTest() {}
// Simulates navigating to a page and getting the page contents and language
// for that navigation.
void SimulateNavigation(const GURL& url,
const std::string& lang,
bool page_translatable) {
content::NavigationSimulator::NavigateAndCommitFromBrowser(web_contents(),
url);
SimulateOnTranslateLanguageDetermined(lang, page_translatable);
}
void SimulateOnTranslateLanguageDetermined(const std::string& lang,
bool page_translatable) {
translate::LanguageDetectionDetails details;
details.adopted_language = lang;
ChromeTranslateClient::FromWebContents(web_contents())
->translate_driver()
.OnPageReady(fake_page_.BindToNewPagePtr(), details, page_translatable);
}
InfoBarService* infobar_service() {
return InfoBarService::FromWebContents(web_contents());
}
// Returns the translate infobar if there is 1 infobar and it is a translate
// infobar.
translate::TranslateInfoBarDelegate* GetTranslateInfoBar() {
return (infobar_service()->infobar_count() == 1)
? infobar_service()
->infobar_at(0)
->delegate()
->AsTranslateInfoBarDelegate()
: NULL;
}
#if !defined(USE_AURA) && !defined(OS_MACOSX)
// If there is 1 infobar and it is a translate infobar, closes it and returns
// true. Returns false otherwise.
bool CloseTranslateInfoBar() {
infobars::InfoBarDelegate* infobar = GetTranslateInfoBar();
if (!infobar)
return false;
infobar->InfoBarDismissed(); // Simulates closing the infobar.
infobar_service()->RemoveInfoBar(infobar_service()->infobar_at(0));
return true;
}
#endif
protected:
void SetUp() override {
// Setup the test environment, including the threads and message loops. This
// must be done before base::ThreadTaskRunnerHandle::Get() is called when
// setting up the net::TestURLRequestContextGetter below.
ChromeRenderViewHostTestHarness::SetUp();
// Clears the translate script so it is fetched every time and sets the
// expiration delay to a large value by default (in case it was zeroed in a
// previous test).
TranslateService::InitializeForTesting(
network::mojom::ConnectionType::CONNECTION_WIFI);
InfoBarService::CreateForWebContents(web_contents());
ChromeTranslateClient::CreateForWebContents(web_contents());
ChromeTranslateClient::FromWebContents(web_contents())
->translate_driver()
.set_translate_max_reload_attempts(0);
}
void TearDown() override {
ChromeRenderViewHostTestHarness::TearDown();
TranslateService::ShutdownForTesting();
}
private:
// The infobars that have been removed.
// WARNING: the pointers point to deleted objects, use only for comparison.
std::set<infobars::InfoBarDelegate*> removed_infobars_;
FakePageImpl fake_page_;
DISALLOW_COPY_AND_ASSIGN(TranslateManagerRenderViewHostAndroidTest);
};
TEST_F(TranslateManagerRenderViewHostAndroidTest,
ManualTranslateOnReadyBeforeLanguageDetermined) {
// This only makes sense for infobars, because the check for supported
// languages moved out of the Infobar into the TranslateManager.
if (TranslateService::IsTranslateBubbleEnabled())
return;
GURL url("http://www.google.com");
// We should not have a translate infobar.
ASSERT_TRUE(GetTranslateInfoBar() == NULL);
content::NavigationSimulator::NavigateAndCommitFromBrowser(web_contents(),
url);
ChromeTranslateClient::FromWebContents(web_contents())
->ManualTranslateWhenReady();
ASSERT_TRUE(GetTranslateInfoBar() == NULL);
SimulateOnTranslateLanguageDetermined("fr", true);
EXPECT_TRUE(GetTranslateInfoBar() != NULL);
EXPECT_TRUE(CloseTranslateInfoBar());
}
TEST_F(TranslateManagerRenderViewHostAndroidTest,
ManualTranslateOnReadyAfterLanguageDetermined) {
// This only makes sense for infobars, because the check for supported
// languages moved out of the Infobar into the TranslateManager.
if (TranslateService::IsTranslateBubbleEnabled())
return;
GURL url("http://www.google.com");
// We should not have a translate infobar.
ASSERT_TRUE(GetTranslateInfoBar() == NULL);
content::NavigationSimulator::NavigateAndCommitFromBrowser(web_contents(),
url);
ASSERT_TRUE(GetTranslateInfoBar() == NULL);
SimulateOnTranslateLanguageDetermined("fr", true);
ChromeTranslateClient::FromWebContents(web_contents())
->ManualTranslateWhenReady();
EXPECT_TRUE(GetTranslateInfoBar() != NULL);
EXPECT_TRUE(CloseTranslateInfoBar());
}
......@@ -22,6 +22,7 @@
#include "chrome/browser/infobars/infobar_service.h"
#include "chrome/browser/renderer_context_menu/render_view_context_menu_test_util.h"
#include "chrome/browser/translate/chrome_translate_client.h"
#include "chrome/browser/translate/translate_fake_page.h"
#include "chrome/browser/translate/translate_service.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/translate/translate_bubble_factory.h"
......@@ -121,63 +122,6 @@ class MockTranslateBubbleFactory : public TranslateBubbleFactory {
DISALLOW_COPY_AND_ASSIGN(MockTranslateBubbleFactory);
};
class FakePageImpl : public translate::mojom::Page {
public:
FakePageImpl()
: called_translate_(false),
called_revert_translation_(false),
binding_(this) {}
~FakePageImpl() override {}
translate::mojom::PagePtr BindToNewPagePtr() {
binding_.Close();
translate_callback_pending_.Reset();
translate::mojom::PagePtr page;
binding_.Bind(mojo::MakeRequest(&page));
return page;
}
// translate::mojom::Page implementation.
void Translate(const std::string& translate_script,
const std::string& source_lang,
const std::string& target_lang,
TranslateCallback callback) override {
// Ensure pending callback gets called.
if (translate_callback_pending_) {
std::move(translate_callback_pending_)
.Run(true, "", "", translate::TranslateErrors::NONE);
}
called_translate_ = true;
source_lang_ = source_lang;
target_lang_ = target_lang;
translate_callback_pending_ = std::move(callback);
}
void RevertTranslation() override { called_revert_translation_ = true; }
void PageTranslated(bool cancelled,
const std::string& source_lang,
const std::string& target_lang,
translate::TranslateErrors::Type error) {
std::move(translate_callback_pending_)
.Run(cancelled, source_lang, target_lang, error);
}
bool called_translate_;
base::Optional<std::string> source_lang_;
base::Optional<std::string> target_lang_;
bool called_revert_translation_;
private:
TranslateCallback translate_callback_pending_;
mojo::Binding<translate::mojom::Page> binding_;
DISALLOW_COPY_AND_ASSIGN(FakePageImpl);
};
} // namespace
// An observer that keeps track of whether a navigation entry was committed.
......
......@@ -2703,6 +2703,7 @@ test("unit_tests") {
"../browser/thumbnails/thumbnail_utils_unittest.cc",
"../browser/tracing/background_tracing_field_trial_unittest.cc",
"../browser/translate/chrome_translate_client_unittest.cc",
"../browser/translate/translate_fake_page.cc",
"../browser/translate/translate_service_unittest.cc",
"../browser/ui/android/tab_model/tab_model_list_unittest.cc",
"../browser/ui/android/tab_model/tab_model_unittest.cc",
......@@ -3016,6 +3017,7 @@ test("unit_tests") {
"../browser/password_manager/password_accessory_controller_unittest.cc",
"../browser/password_manager/save_password_infobar_delegate_android_unittest.cc",
"../browser/password_manager/update_password_infobar_delegate_android_unittest.cc",
"../browser/translate/translate_manager_render_view_host_android_unittest.cc",
]
deps += [
"//base:base_java",
......
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