Commit 1125fe1a authored by Li Lin's avatar Li Lin Committed by Commit Bot

Improve language detection by sending surrounding text to CLD3.

CLD3 is the on-device language detector. The surrounding text is not
sent to server.

Bug: 1084381e
Test: manual test
Change-Id: I9f12151847122c30352883607ac724c45d5001b7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2207859Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Commit-Queue: Li Lin <llin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#770759}
parent ec2b409f
...@@ -14,6 +14,7 @@ namespace chromeos { ...@@ -14,6 +14,7 @@ namespace chromeos {
namespace quick_answers { namespace quick_answers {
class QuickAnswersClient; class QuickAnswersClient;
class QuickAnswersDelegate; class QuickAnswersDelegate;
struct Context;
} // namespace quick_answers } // namespace quick_answers
} // namespace chromeos } // namespace chromeos
...@@ -36,10 +37,13 @@ class ASH_PUBLIC_EXPORT QuickAnswersController { ...@@ -36,10 +37,13 @@ class ASH_PUBLIC_EXPORT QuickAnswersController {
// Show the quick-answers view (and/or any accompanying/associated views like // Show the quick-answers view (and/or any accompanying/associated views like
// user-consent view instead, if consent is not yet granted). |anchor_bounds| // user-consent view instead, if consent is not yet granted). |anchor_bounds|
// is the bounds of the anchor view (which is the context menu for browser). // is the bounds of the anchor view (which is the context menu for browser).
// |title| is the text selected by the user. // |title| is the text selected by the user. |context| is the context
virtual void MaybeShowQuickAnswers(const gfx::Rect& anchor_bounds, // information which will be used as part of the request for getting more
const std::string& title, // relevant result.
const std::string& device_language) = 0; virtual void MaybeShowQuickAnswers(
const gfx::Rect& anchor_bounds,
const std::string& title,
const chromeos::quick_answers::Context& context) = 0;
// Dismiss the quick-answers view (and/or any associated views like // Dismiss the quick-answers view (and/or any associated views like
// user-consent view) currently shown. |is_active| is true if the quick-answer // user-consent view) currently shown. |is_active| is true if the quick-answer
......
...@@ -10,12 +10,12 @@ ...@@ -10,12 +10,12 @@
#include "ash/shell.h" #include "ash/shell.h"
#include "ash/strings/grit/ash_strings.h" #include "ash/strings/grit/ash_strings.h"
#include "chromeos/components/quick_answers/quick_answers_consents.h" #include "chromeos/components/quick_answers/quick_answers_consents.h"
#include "chromeos/components/quick_answers/quick_answers_model.h"
#include "chromeos/constants/chromeos_features.h" #include "chromeos/constants/chromeos_features.h"
#include "url/gurl.h" #include "url/gurl.h"
// TODO(yanxiao):Add a unit test for QuickAnswersControllerImpl. // TODO(yanxiao):Add a unit test for QuickAnswersControllerImpl.
namespace { namespace {
using chromeos::quick_answers::Context;
using chromeos::quick_answers::QuickAnswer; using chromeos::quick_answers::QuickAnswer;
using chromeos::quick_answers::QuickAnswersClient; using chromeos::quick_answers::QuickAnswersClient;
using chromeos::quick_answers::QuickAnswersRequest; using chromeos::quick_answers::QuickAnswersRequest;
...@@ -48,15 +48,14 @@ void QuickAnswersControllerImpl::SetClient( ...@@ -48,15 +48,14 @@ void QuickAnswersControllerImpl::SetClient(
void QuickAnswersControllerImpl::MaybeShowQuickAnswers( void QuickAnswersControllerImpl::MaybeShowQuickAnswers(
const gfx::Rect& anchor_bounds, const gfx::Rect& anchor_bounds,
const std::string& title, const std::string& title,
const std::string& device_language) { const Context& context) {
// Cache anchor-bounds and query. // Cache anchor-bounds and query.
anchor_bounds_ = anchor_bounds; anchor_bounds_ = anchor_bounds;
// Initially, title is same as query. Title and query can be overridden based // Initially, title is same as query. Title and query can be overridden based
// on text annotation result at |OnRequestPreprocessFinish|. // on text annotation result at |OnRequestPreprocessFinish|.
title_ = title; title_ = title;
query_ = title; query_ = title;
context_ = context;
device_language_ = device_language;
// Show user-consent notice informing user about the feature if required. // Show user-consent notice informing user about the feature if required.
if (consent_controller_->ShouldShowConsent()) { if (consent_controller_->ShouldShowConsent()) {
...@@ -80,7 +79,7 @@ void QuickAnswersControllerImpl::MaybeShowQuickAnswers( ...@@ -80,7 +79,7 @@ void QuickAnswersControllerImpl::MaybeShowQuickAnswers(
QuickAnswersRequest request; QuickAnswersRequest request;
request.selected_text = title; request.selected_text = title;
request.device_properties.language = device_language; request.context = context;
quick_answers_client_->SendRequest(request); quick_answers_client_->SendRequest(request);
} }
...@@ -172,7 +171,7 @@ void QuickAnswersControllerImpl::OnUserConsentGranted() { ...@@ -172,7 +171,7 @@ void QuickAnswersControllerImpl::OnUserConsentGranted() {
chromeos::quick_answers::ConsentInteractionType::kAccept); chromeos::quick_answers::ConsentInteractionType::kAccept);
// Display Quick-Answer for the cached query when user consents. // Display Quick-Answer for the cached query when user consents.
MaybeShowQuickAnswers(anchor_bounds_, title_, device_language_); MaybeShowQuickAnswers(anchor_bounds_, title_, context_);
} }
void QuickAnswersControllerImpl::OnConsentSettingsRequestedByUser() { void QuickAnswersControllerImpl::OnConsentSettingsRequestedByUser() {
......
...@@ -11,12 +11,11 @@ ...@@ -11,12 +11,11 @@
#include "ash/ash_export.h" #include "ash/ash_export.h"
#include "ash/public/cpp/quick_answers_controller.h" #include "ash/public/cpp/quick_answers_controller.h"
#include "chromeos/components/quick_answers/quick_answers_client.h" #include "chromeos/components/quick_answers/quick_answers_client.h"
#include "chromeos/components/quick_answers/quick_answers_model.h"
#include "ui/gfx/geometry/rect.h" #include "ui/gfx/geometry/rect.h"
namespace chromeos { namespace chromeos {
namespace quick_answers { namespace quick_answers {
struct QuickAnswer;
struct QuickAnswersRequest;
class QuickAnswersConsent; class QuickAnswersConsent;
} // namespace quick_answers } // namespace quick_answers
} // namespace chromeos } // namespace chromeos
...@@ -42,9 +41,10 @@ class ASH_EXPORT QuickAnswersControllerImpl ...@@ -42,9 +41,10 @@ class ASH_EXPORT QuickAnswersControllerImpl
// SetClient is required to be called before using these methods. // SetClient is required to be called before using these methods.
// TODO(yanxiao): refactor to delegate to browser. // TODO(yanxiao): refactor to delegate to browser.
void MaybeShowQuickAnswers(const gfx::Rect& anchor_bounds, void MaybeShowQuickAnswers(
const std::string& title, const gfx::Rect& anchor_bounds,
const std::string& device_language) override; const std::string& title,
const chromeos::quick_answers::Context& context) override;
void DismissQuickAnswers(bool is_active) override; void DismissQuickAnswers(bool is_active) override;
...@@ -92,8 +92,8 @@ class ASH_EXPORT QuickAnswersControllerImpl ...@@ -92,8 +92,8 @@ class ASH_EXPORT QuickAnswersControllerImpl
// Title to be shown on the QuickAnswers view. // Title to be shown on the QuickAnswers view.
std::string title_; std::string title_;
// Device language. // Context information, including surrounding text and device properties.
std::string device_language_; chromeos::quick_answers::Context context_;
std::unique_ptr<chromeos::quick_answers::QuickAnswersClient> std::unique_ptr<chromeos::quick_answers::QuickAnswersClient>
quick_answers_client_; quick_answers_client_;
......
...@@ -24,13 +24,16 @@ ...@@ -24,13 +24,16 @@
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "components/renderer_context_menu/render_view_context_menu_proxy.h" #include "components/renderer_context_menu/render_view_context_menu_proxy.h"
#include "content/public/browser/context_menu_params.h" #include "content/public/browser/context_menu_params.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/storage_partition.h" #include "content/public/browser/storage_partition.h"
#include "content/public/browser/web_contents.h"
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
#include "ui/gfx/text_constants.h" #include "ui/gfx/text_constants.h"
#include "ui/gfx/text_elider.h" #include "ui/gfx/text_elider.h"
namespace { namespace {
using chromeos::quick_answers::Context;
using chromeos::quick_answers::QuickAnswer; using chromeos::quick_answers::QuickAnswer;
using chromeos::quick_answers::QuickAnswersClient; using chromeos::quick_answers::QuickAnswersClient;
using chromeos::quick_answers::QuickAnswersRequest; using chromeos::quick_answers::QuickAnswersRequest;
...@@ -42,6 +45,7 @@ constexpr char kNoResult[] = "See result in Assistant"; ...@@ -42,6 +45,7 @@ constexpr char kNoResult[] = "See result in Assistant";
constexpr char kNetworkError[] = "Cannot connect to internet."; constexpr char kNetworkError[] = "Cannot connect to internet.";
constexpr size_t kMaxDisplayTextLength = 70; constexpr size_t kMaxDisplayTextLength = 70;
constexpr int kMaxSurroundingTextLength = 300;
base::string16 TruncateString(const std::string& text) { base::string16 TruncateString(const std::string& text) {
return gfx::TruncateString(base::UTF8ToUTF16(text), kMaxDisplayTextLength, return gfx::TruncateString(base::UTF8ToUTF16(text), kMaxDisplayTextLength,
...@@ -123,7 +127,7 @@ void QuickAnswersMenuObserver::InitMenu( ...@@ -123,7 +127,7 @@ void QuickAnswersMenuObserver::InitMenu(
// Fetch Quick Answer. // Fetch Quick Answer.
QuickAnswersRequest request; QuickAnswersRequest request;
request.selected_text = selected_text; request.selected_text = selected_text;
request.device_properties.language = GetDeviceLanguage(); request.context.device_properties.language = GetDeviceLanguage();
query_ = request.selected_text; query_ = request.selected_text;
quick_answers_client_->SendRequest(request); quick_answers_client_->SendRequest(request);
} }
...@@ -145,12 +149,22 @@ void QuickAnswersMenuObserver::OnContextMenuShown( ...@@ -145,12 +149,22 @@ void QuickAnswersMenuObserver::OnContextMenuShown(
if (selected_text.empty()) if (selected_text.empty())
return; return;
quick_answers_controller_->MaybeShowQuickAnswers( bounds_in_screen_ = bounds_in_screen;
bounds_in_screen, selected_text, GetDeviceLanguage());
content::RenderFrameHost* focused_frame =
proxy_->GetWebContents()->GetFocusedFrame();
if (focused_frame) {
focused_frame->RequestTextSurroundingSelection(
base::BindOnce(
&QuickAnswersMenuObserver::OnTextSurroundingSelectionAvailable,
weak_factory_.GetWeakPtr(), selected_text),
kMaxSurroundingTextLength);
}
} }
void QuickAnswersMenuObserver::OnContextMenuViewBoundsChanged( void QuickAnswersMenuObserver::OnContextMenuViewBoundsChanged(
const gfx::Rect& bounds_in_screen) { const gfx::Rect& bounds_in_screen) {
bounds_in_screen_ = bounds_in_screen;
if (!quick_answers_controller_) if (!quick_answers_controller_)
return; return;
quick_answers_controller_->UpdateQuickAnswersAnchorBounds(bounds_in_screen); quick_answers_controller_->UpdateQuickAnswersAnchorBounds(bounds_in_screen);
...@@ -250,3 +264,15 @@ void QuickAnswersMenuObserver::SendAssistantQuery(const std::string& query) { ...@@ -250,3 +264,15 @@ void QuickAnswersMenuObserver::SendAssistantQuery(const std::string& query) {
std::string QuickAnswersMenuObserver::GetDeviceLanguage() { std::string QuickAnswersMenuObserver::GetDeviceLanguage() {
return l10n_util::GetLanguage(g_browser_process->GetApplicationLocale()); return l10n_util::GetLanguage(g_browser_process->GetApplicationLocale());
} }
void QuickAnswersMenuObserver::OnTextSurroundingSelectionAvailable(
const std::string& selected_text,
const base::string16& surrounding_text,
uint32_t start_offset,
uint32_t end_offset) {
Context context;
context.surrounding_text = base::UTF16ToUTF8(surrounding_text);
context.device_properties.language = GetDeviceLanguage();
quick_answers_controller_->MaybeShowQuickAnswers(bounds_in_screen_,
selected_text, context);
}
...@@ -56,6 +56,11 @@ class QuickAnswersMenuObserver ...@@ -56,6 +56,11 @@ class QuickAnswersMenuObserver
bool IsRichUiEnabled(); bool IsRichUiEnabled();
void SendAssistantQuery(const std::string& query); void SendAssistantQuery(const std::string& query);
std::string GetDeviceLanguage(); std::string GetDeviceLanguage();
void OnTextSurroundingSelectionAvailable(
const std::string& selected_text,
const base::string16& surrounding_text,
uint32_t start_offset,
uint32_t end_offset);
// The interface to add a context-menu item and update it. // The interface to add a context-menu item and update it.
RenderViewContextMenuProxy* proxy_; RenderViewContextMenuProxy* proxy_;
...@@ -70,12 +75,16 @@ class QuickAnswersMenuObserver ...@@ -70,12 +75,16 @@ class QuickAnswersMenuObserver
// Query used to retrieve quick answer. // Query used to retrieve quick answer.
std::string query_; std::string query_;
gfx::Rect bounds_in_screen_;
std::unique_ptr<chromeos::quick_answers::QuickAnswer> quick_answer_; std::unique_ptr<chromeos::quick_answers::QuickAnswer> quick_answer_;
ash::QuickAnswersController* quick_answers_controller_ = nullptr; ash::QuickAnswersController* quick_answers_controller_ = nullptr;
// Whether commands other than quick answers is executed. // Whether commands other than quick answers is executed.
bool is_other_command_executed_ = false; bool is_other_command_executed_ = false;
base::WeakPtrFactory<QuickAnswersMenuObserver> weak_factory_{this};
}; };
#endif // CHROME_BROWSER_RENDERER_CONTEXT_MENU_QUICK_ANSWERS_MENU_OBSERVER_H_ #endif // CHROME_BROWSER_RENDERER_CONTEXT_MENU_QUICK_ANSWERS_MENU_OBSERVER_H_
...@@ -13,5 +13,10 @@ PreprocessedOutput::PreprocessedOutput() = default; ...@@ -13,5 +13,10 @@ PreprocessedOutput::PreprocessedOutput() = default;
PreprocessedOutput::PreprocessedOutput(const PreprocessedOutput& other) = PreprocessedOutput::PreprocessedOutput(const PreprocessedOutput& other) =
default; default;
PreprocessedOutput::~PreprocessedOutput() = default; PreprocessedOutput::~PreprocessedOutput() = default;
QuickAnswersRequest::QuickAnswersRequest() = default;
QuickAnswersRequest::QuickAnswersRequest(const QuickAnswersRequest& other) =
default;
QuickAnswersRequest::~QuickAnswersRequest() = default;
} // namespace quick_answers } // namespace quick_answers
} // namespace chromeos } // namespace chromeos
...@@ -138,18 +138,31 @@ struct PreprocessedOutput { ...@@ -138,18 +138,31 @@ struct PreprocessedOutput {
std::string query; std::string query;
}; };
// Structure of quick answers request context, including device properties and
// surrounding text.
struct Context {
// Device specific properties.
DeviceProperties device_properties;
std::string surrounding_text;
};
// Structure to describe an quick answer request including selected content and // Structure to describe an quick answer request including selected content and
// context. // context.
struct QuickAnswersRequest { struct QuickAnswersRequest {
QuickAnswersRequest();
QuickAnswersRequest(const QuickAnswersRequest& other);
~QuickAnswersRequest();
// The selected Text. // The selected Text.
std::string selected_text; std::string selected_text;
// Device specific properties.
DeviceProperties device_properties;
// Output of processed result. // Output of processed result.
PreprocessedOutput preprocessed_output; PreprocessedOutput preprocessed_output;
// Context information.
Context context;
// TODO(llin): Add context and other targeted objects (e.g: images, links, // TODO(llin): Add context and other targeted objects (e.g: images, links,
// etc). // etc).
}; };
......
...@@ -115,7 +115,7 @@ void IntentGenerator::LoadModelCallback(const QuickAnswersRequest& request, ...@@ -115,7 +115,7 @@ void IntentGenerator::LoadModelCallback(const QuickAnswersRequest& request,
machine_learning::mojom::TextAnnotationRequest::New(); machine_learning::mojom::TextAnnotationRequest::New();
text_annotation_request->text = request.selected_text; text_annotation_request->text = request.selected_text;
text_annotation_request->default_locales = text_annotation_request->default_locales =
request.device_properties.language; request.context.device_properties.language;
text_classifier_->Annotate( text_classifier_->Annotate(
std::move(text_annotation_request), std::move(text_annotation_request),
...@@ -155,25 +155,23 @@ void IntentGenerator::MaybeGenerateTranslationIntent( ...@@ -155,25 +155,23 @@ void IntentGenerator::MaybeGenerateTranslationIntent(
// Don't do language detection if no device language is provided or the length // Don't do language detection if no device language is provided or the length
// of selected text is above the threshold. Returns unknown intent type. // of selected text is above the threshold. Returns unknown intent type.
if (request.device_properties.language.empty() || if (request.context.device_properties.language.empty() ||
request.selected_text.length() > kTranslationTextLengthThreshold) { request.selected_text.length() > kTranslationTextLengthThreshold) {
std::move(complete_callback_) std::move(complete_callback_)
.Run(request.selected_text, IntentType::kUnknown); .Run(request.selected_text, IntentType::kUnknown);
return; return;
} }
auto detected_language = language_detector_->DetectLanguage(
// TODO(b/150974962): Investigate improving language detection accuracy using !request.context.surrounding_text.empty()
// surrounding text or page content. ? request.context.surrounding_text
auto detected_language = : request.selected_text);
language_detector_->DetectLanguage(request.selected_text);
auto intent_type = IntentType::kUnknown; auto intent_type = IntentType::kUnknown;
if (!detected_language.empty() && if (!detected_language.empty() &&
detected_language != request.device_properties.language) { detected_language != request.context.device_properties.language) {
intent_type = IntentType::kTranslation; intent_type = IntentType::kTranslation;
} }
std::move(complete_callback_).Run(request.selected_text, intent_type); std::move(complete_callback_).Run(request.selected_text, intent_type);
} }
} // namespace quick_answers } // namespace quick_answers
} // namespace chromeos } // namespace chromeos
\ No newline at end of file
...@@ -99,7 +99,7 @@ TEST_F(IntentGeneratorTest, TranslationIntent) { ...@@ -99,7 +99,7 @@ TEST_F(IntentGeneratorTest, TranslationIntent) {
QuickAnswersRequest request; QuickAnswersRequest request;
request.selected_text = "quick answers"; request.selected_text = "quick answers";
request.device_properties.language = "es"; request.context.device_properties.language = "es";
intent_generator_->GenerateIntent(request); intent_generator_->GenerateIntent(request);
task_environment_.RunUntilIdle(); task_environment_.RunUntilIdle();
...@@ -113,7 +113,7 @@ TEST_F(IntentGeneratorTest, TranslationIntentSameLanguage) { ...@@ -113,7 +113,7 @@ TEST_F(IntentGeneratorTest, TranslationIntentSameLanguage) {
QuickAnswersRequest request; QuickAnswersRequest request;
request.selected_text = "quick answers"; request.selected_text = "quick answers";
request.device_properties.language = "en"; request.context.device_properties.language = "en";
intent_generator_->GenerateIntent(request); intent_generator_->GenerateIntent(request);
task_environment_.RunUntilIdle(); task_environment_.RunUntilIdle();
...@@ -129,7 +129,7 @@ TEST_F(IntentGeneratorTest, TranslationIntentTextLengthAboveThreshold) { ...@@ -129,7 +129,7 @@ TEST_F(IntentGeneratorTest, TranslationIntentTextLengthAboveThreshold) {
request.selected_text = request.selected_text =
"Search the world's information, including webpages, images, videos and " "Search the world's information, including webpages, images, videos and "
"more."; "more.";
request.device_properties.language = "es"; request.context.device_properties.language = "es";
intent_generator_->GenerateIntent(request); intent_generator_->GenerateIntent(request);
task_environment_.RunUntilIdle(); task_environment_.RunUntilIdle();
......
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