Skip to content

Commit

Permalink
Compose Triggering - determine the right time to allow a nudge or menu
Browse files Browse the repository at this point in the history
Check a variety of conditions before deciding to show a nudge bubble
or context menu, including whether the feature is enabled, the language
is supported, and autocomplete is allowed for this field.

Bug: b:300941076
Change-Id: I4b749bb9cd5d9ac84ee3e073ed943237fd5a51d8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4959114
Commit-Queue: Peter Williamson <petewil@chromium.org>
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Reviewed-by: Sophey Dong <sophey@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1215690}
  • Loading branch information
Pete Williamson authored and Chromium LUCI CQ committed Oct 26, 2023
1 parent 8f1b0d2 commit de39d17
Show file tree
Hide file tree
Showing 12 changed files with 419 additions and 36 deletions.
2 changes: 2 additions & 0 deletions chrome/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -7977,6 +7977,8 @@ static_library("browser") {
"compose/compose_enabling.h",
"compose/compose_session.cc",
"compose/compose_session.h",
"compose/translate_language_provider.cc",
"compose/translate_language_provider.h",
]

deps += [
Expand Down
30 changes: 28 additions & 2 deletions chrome/browser/compose/chrome_compose_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "chrome/browser/optimization_guide/optimization_guide_keyed_service_factory.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/translate/chrome_translate_client.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_dialogs.h"
#include "chrome/browser/ui/browser_finder.h"
Expand All @@ -43,14 +44,16 @@ const char kComposeURL[] = "chrome://compose/";

ChromeComposeClient::ChromeComposeClient(content::WebContents* web_contents)
: content::WebContentsUserData<ChromeComposeClient>(*web_contents),
translate_language_provider_(new TranslateLanguageProvider()),
compose_enabling_(translate_language_provider_.get()),
manager_(this),
close_page_receiver_(this) {
profile_ = Profile::FromBrowserContext(GetWebContents().GetBrowserContext());
opt_guide_ = OptimizationGuideKeyedServiceFactory::GetForProfile(profile_);

if (GetOptimizationGuide()) {
std::vector<optimization_guide::proto::OptimizationType> types;
if (ComposeEnabling::IsEnabledForProfile(profile_)) {
if (compose_enabling_.IsEnabledForProfile(profile_)) {
types.push_back(optimization_guide::proto::OptimizationType::COMPOSE);
}

Expand Down Expand Up @@ -165,6 +168,29 @@ compose::ComposeManager& ChromeComposeClient::GetManager() {
return manager_;
}

ComposeEnabling& ChromeComposeClient::GetComposeEnabling() {
return compose_enabling_;
}

bool ChromeComposeClient::ShouldTriggerPopup(std::string autocomplete_attribute,
autofill::FieldGlobalId field_id) {
// TODO(b/303502029): When we make an enum for return state, check to see if
// we have saved state for the current field, and offer the saved state
// bubble.
bool saved_state = !sessions_.empty();
translate::TranslateManager* translate_manager =
ChromeTranslateClient::GetManagerFromWebContents(&GetWebContents());
return compose_enabling_.ShouldTriggerPopup(autocomplete_attribute, profile_,
translate_manager, saved_state);
}

bool ChromeComposeClient::ShouldTriggerContextMenu() {
translate::TranslateManager* translate_manager =
ChromeTranslateClient::GetManagerFromWebContents(&GetWebContents());
return compose_enabling_.ShouldTriggerContextMenu(profile_,
translate_manager);
}

optimization_guide::OptimizationGuideModelExecutor*
ChromeComposeClient::GetModelExecutor() {
return model_executor_for_test_.value_or(
Expand Down Expand Up @@ -197,7 +223,7 @@ compose::ComposeHintDecision ChromeComposeClient::GetOptimizationGuidanceForUrl(
return compose::ComposeHintDecision::COMPOSE_HINT_DECISION_UNSPECIFIED;
}

if (!ComposeEnabling::IsEnabledForProfile(profile_)) {
if (!compose_enabling_.IsEnabledForProfile(profile_)) {
return compose::ComposeHintDecision::COMPOSE_HINT_DECISION_COMPOSE_DISABLED;
}

Expand Down
9 changes: 9 additions & 0 deletions chrome/browser/compose/chrome_compose_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <string>

#include "base/containers/flat_map.h"
#include "chrome/browser/compose/compose_enabling.h"
#include "chrome/browser/compose/compose_session.h"
#include "chrome/browser/compose/proto/compose_optimization_guide.pb.h"
#include "chrome/browser/optimization_guide/optimization_guide_keyed_service.h"
Expand Down Expand Up @@ -56,6 +57,10 @@ class ChromeComposeClient
// triggered the close.
void CloseUI(compose::mojom::CloseReason reason) override;

bool ShouldTriggerPopup(std::string autocomplete_attribute,
autofill::FieldGlobalId field_id) override;
bool ShouldTriggerContextMenu() override;

void BindComposeDialog(
mojo::PendingReceiver<compose::mojom::ComposeDialogClosePageHandler>
close_handler,
Expand All @@ -73,9 +78,13 @@ class ChromeComposeClient
// to guide our decision to enable the feature and trigger the nudge.
compose::ComposeHintDecision GetOptimizationGuidanceForUrl(const GURL& url);

ComposeEnabling& GetComposeEnabling();

protected:
optimization_guide::OptimizationGuideModelExecutor* GetModelExecutor();
optimization_guide::OptimizationGuideDecider* GetOptimizationGuide();
std::unique_ptr<TranslateLanguageProvider> translate_language_provider_;
ComposeEnabling compose_enabling_;

private:
friend class content::WebContentsUserData<ChromeComposeClient>;
Expand Down
17 changes: 11 additions & 6 deletions chrome/browser/compose/chrome_compose_client_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -161,13 +161,22 @@ class ChromeComposeClientTest : public BrowserWithTestWindowTest {
GURL GetPageUrl() { return GURL("http://foo/1"); }

void TearDown() override {
ClearEnabled();
client_ = nullptr;
BrowserWithTestWindowTest::TearDown();
}

void SetEnabled() { ComposeEnabling::SetEnabledForTesting(); }
void SetEnabled() {
if (client_ != nullptr) {
client_->GetComposeEnabling().SetEnabledForTesting();
}
}

void ClearEnabled() { ComposeEnabling::ClearEnabledForTesting(); }
void ClearEnabled() {
if (client_ != nullptr) {
client_->GetComposeEnabling().ClearEnabledForTesting();
}
}

protected:
compose_proto::ComposeRequest ComposeRequest(std::string user_input) {
Expand Down Expand Up @@ -480,7 +489,6 @@ TEST_F(ChromeComposeClientTest, GetOptimizationGuidanceShowNudgeTest) {
GURL example(kExampleURL);
compose::ComposeHintDecision decision =
client().GetOptimizationGuidanceForUrl(example);
ClearEnabled();

// Verify response from CanApplyOptimization is as we expect.
EXPECT_EQ(compose::ComposeHintDecision::COMPOSE_HINT_DECISION_ENABLED,
Expand Down Expand Up @@ -512,7 +520,6 @@ TEST_F(ChromeComposeClientTest, GetOptimizationGuidanceFeatureOffTest) {
GURL example(kExampleURL);
compose::ComposeHintDecision decision =
client().GetOptimizationGuidanceForUrl(example);
ClearEnabled();

// Verify response from CanApplyOptimization is as we expect.
EXPECT_EQ(
Expand Down Expand Up @@ -545,7 +552,6 @@ TEST_F(ChromeComposeClientTest, GetOptimizationGuidanceNoFeedbackTest) {
GURL example(kExampleURL);
compose::ComposeHintDecision decision =
client().GetOptimizationGuidanceForUrl(example);
ClearEnabled();

// Verify response from CanApplyOptimization is as we expect.
EXPECT_EQ(compose::ComposeHintDecision::COMPOSE_HINT_DECISION_UNSPECIFIED,
Expand Down Expand Up @@ -575,7 +581,6 @@ TEST_F(ChromeComposeClientTest, GetOptimizationGuidanceNoComposeMetadataTest) {
GURL example(kExampleURL);
compose::ComposeHintDecision decision =
client().GetOptimizationGuidanceForUrl(example);
ClearEnabled();

// Verify response from CanApplyOptimization is as we expect.
EXPECT_EQ(compose::ComposeHintDecision::COMPOSE_HINT_DECISION_UNSPECIFIED,
Expand Down
81 changes: 75 additions & 6 deletions chrome/browser/compose/compose_enabling.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,39 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include <type_traits>

#include "chrome/browser/compose/compose_enabling.h"

#include "chrome/browser/signin/identity_manager_factory.h"
#include "components/compose/buildflags.h"
#include "components/compose/core/browser/compose_features.h"
#include "components/unified_consent/url_keyed_data_collection_consent_helper.h"
#include "compose_enabling.h"

namespace {

bool AutocompleteAllowed(std::string_view autocomplete_attribute) {
// Check autocomplete is not turned off.
return autocomplete_attribute != std::string("off");
}

bool ComposeEnabling::enabled_for_testing_ = false;
} // namespace

ComposeEnabling::ComposeEnabling(
TranslateLanguageProvider* translate_language_provider) {
enabled_for_testing_ = false;
translate_language_provider_ = translate_language_provider;
}

ComposeEnabling::~ComposeEnabling() = default;

void ComposeEnabling::SetEnabledForTesting() {
ComposeEnabling::enabled_for_testing_ = true;
enabled_for_testing_ = true;
}

void ComposeEnabling::ClearEnabledForTesting() {
ComposeEnabling::enabled_for_testing_ = false;
enabled_for_testing_ = false;
}

bool ComposeEnabling::IsEnabledForProfile(Profile* profile) {
Expand All @@ -41,8 +59,7 @@ bool ComposeEnabling::IsEnabled(Profile* profile,

// Check that the feature flag is enabled.
if (!base::FeatureList::IsEnabled(compose::features::kEnableCompose)) {
DVLOG(2) << "feature not enabled "
<< "ComposeEnabling::IsEnabled";
DVLOG(2) << "feature not enabled ";
return false;
}

Expand All @@ -63,7 +80,59 @@ bool ComposeEnabling::IsEnabled(Profile* profile,
return false;
}

// TODO(b/300974056): Check with optization guide (age, labs).
// TODO(b/305245821): Check age.
// TODO(b/305246349): Check finch (or maybe labs).
// TODO(b/305245736): Check consent once it is available to check.

return true;
}

// TODO(b/303502029): make return state an enum instead of a bool so we
// can return a different value when we have saved state for this field.
bool ComposeEnabling::ShouldTriggerPopup(
std::string_view autocomplete_attribute,
Profile* profile,
translate::TranslateManager* translate_manager,
bool has_saved_state) {
if (!PageLevelChecks(profile, translate_manager)) {
return false;
}

// Check autocomplete attribute.
if (!AutocompleteAllowed(autocomplete_attribute)) {
DVLOG(2) << "autocomplete=off";
return false;
}

if (has_saved_state) {
DVLOG(2) << "has saved state";
return false;
}

return true;
}

bool ComposeEnabling::ShouldTriggerContextMenu(
Profile* profile,
translate::TranslateManager* translate_manager) {
return PageLevelChecks(profile, translate_manager);
}

bool ComposeEnabling::PageLevelChecks(
Profile* profile,
translate::TranslateManager* translate_manager) {
if (!IsEnabledForProfile(profile)) {
DVLOG(2) << "not enabled";
return false;
}

if (!translate_language_provider_->IsLanguageSupported(translate_manager)) {
DVLOG(2) << "language not supported";
return false;
}

// TODO(b/301609046): Check with the optimization guide.
// TODO(b/301609046): Check that we have enough space to show the dialog.

return true;
}
28 changes: 21 additions & 7 deletions chrome/browser/compose/compose_enabling.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,34 @@
#ifndef CHROME_BROWSER_COMPOSE_COMPOSE_ENABLING_H_
#define CHROME_BROWSER_COMPOSE_COMPOSE_ENABLING_H_

#include "chrome/browser/compose/translate_language_provider.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#include "components/translate/core/browser/translate_manager.h"

class ComposeEnabling {
public:
static bool IsEnabledForProfile(Profile* profile);
static void SetEnabledForTesting();
static void ClearEnabledForTesting();
explicit ComposeEnabling(
TranslateLanguageProvider* translate_language_provider);
~ComposeEnabling();
bool IsEnabledForProfile(Profile* profile);
bool IsEnabled(Profile* profile, signin::IdentityManager* identity_manager);
void SetEnabledForTesting();
void ClearEnabledForTesting();
std::string GetLanguage();
bool ShouldTriggerPopup(std::string_view autocomplete_attribute,
Profile* profile,
translate::TranslateManager* translate_manager,
bool has_saved_state);
bool ShouldTriggerContextMenu(Profile* profile,
translate::TranslateManager* translate_manager);

private:
friend class ComposeEnablingTest;
static bool enabled_for_testing_;
static bool IsEnabled(Profile* profile,
signin::IdentityManager* identity_manager);
raw_ptr<TranslateLanguageProvider> translate_language_provider_;
bool enabled_for_testing_;

bool PageLevelChecks(Profile* profile,
translate::TranslateManager* translate_manager);
};

#endif // CHROME_BROWSER_COMPOSE_COMPOSE_ENABLING_H_

0 comments on commit de39d17

Please sign in to comment.