0

Reland "Reland "Assistant: use equal weight buttons based on settings value""

This is a reland of da5f7720e6

The previous CL breaks linux-chromeos-chrome because I missed to update
chromeos/services/libassistant/settings_controller_unittest.cc with
the updated parameters. I've fixed it in this CL.

Original change's description:
> Reland "Assistant: use equal weight buttons based on settings value"
>
> This is a reland of b5cd3736c6
>
> Original change's description:
> > Assistant: use equal weight buttons based on settings value
> >
> > 1. Add an extra parameter `include_header` in mojom method
> > `GetSettings`. When `include_header` is true, returns a seralized
> > proto of |GetSettingsUiResponse|; otherwise, returns |SettingsUi|.
> > 2. Read the newly-added value `footer_button_layout` from header and
> > use it to determine whether to use equal weight buttons on Assistant
> > UIs (both OOBE and in-session).
> >
> > Bug: 1224850
> > Change-Id: I81c255914a3a8aa31f36325044b11d9272132668
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3015335
> > Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
> > Reviewed-by: Yue Li <updowndota@chromium.org>
> > Reviewed-by: Tao Wu <wutao@chromium.org>
> > Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> > Reviewed-by: Sam McNally <sammc@chromium.org>
> > Commit-Queue: Yunke Zhou <yunkez@google.com>
> > Cr-Commit-Position: refs/heads/master@{#900414}
>
> Bug: 1224850
> Change-Id: I7069870fa815406e90474558707517e723d0bfc8
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3020766
> Reviewed-by: Yue Li <updowndota@chromium.org>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
> Reviewed-by: Tao Wu <wutao@chromium.org>
> Reviewed-by: Sam McNally <sammc@chromium.org>
> Commit-Queue: Yunke Zhou <yunkez@google.com>
> Cr-Commit-Position: refs/heads/master@{#900827}

Bug: 1224850
Change-Id: Ib3905e078b76c156870744b52f148cbfd90e5577
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3021206
Reviewed-by: Satoru Takabayashi <satorux@chromium.org>
Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Commit-Queue: Yunke Zhou <yunkez@google.com>
Cr-Commit-Position: refs/heads/master@{#900950}
This commit is contained in:
Yunke Zhou
2021-07-13 11:39:37 +00:00
committed by Chromium LUCI CQ
parent 075f2ab64c
commit a4d655f14a
26 changed files with 189 additions and 55 deletions

@ -32,6 +32,7 @@
#include "chromeos/services/assistant/public/cpp/assistant_prefs.h"
#include "chromeos/services/assistant/public/cpp/assistant_settings.h"
#include "chromeos/services/assistant/public/cpp/features.h"
#include "chromeos/services/assistant/public/proto/get_settings_ui.pb.h"
#include "chromeos/services/assistant/public/proto/settings_ui.pb.h"
#include "chromeos/services/assistant/service.h"
#include "components/prefs/pref_service.h"
@ -159,6 +160,8 @@ class ScopedAssistantSettings : public chromeos::assistant::AssistantSettings {
void set_setting_zippy_size(int size) { setting_zippy_size_ = size; }
void set_is_minor_user(bool is_minor_user) { is_minor_user_ = is_minor_user; }
const std::set<OptIn>& collected_optins() const { return collected_optins_; }
// Advances speaker ID enrollment to the next state.
@ -203,7 +206,10 @@ class ScopedAssistantSettings : public chromeos::assistant::AssistantSettings {
// chromeos::assistant::AssistantSettings:
void GetSettings(const std::string& selector,
GetSettingsCallback callback) override {
GetSettingsCallback callback) override {}
void GetSettingsWithHeader(const std::string& selector,
GetSettingsCallback callback) override {
chromeos::assistant::SettingsUiSelector selector_proto;
ASSERT_TRUE(selector_proto.ParseFromString(selector));
EXPECT_FALSE(selector_proto.about_me_settings());
@ -212,15 +218,24 @@ class ScopedAssistantSettings : public chromeos::assistant::AssistantSettings {
ASSISTANT_SUW_ONBOARDING_ON_CHROME_OS,
selector_proto.consent_flow_ui_selector().flow_id());
chromeos::assistant::SettingsUi settings_ui;
auto* gaia_user_context_ui = settings_ui.mutable_gaia_user_context_ui();
chromeos::assistant::GetSettingsUiResponse response;
if (is_minor_user_) {
auto* header = response.mutable_header();
header->set_footer_button_layout(
chromeos::assistant::
SettingsResponseHeader_AcceptRejectLayout_EQUAL_WEIGHT);
}
auto* settings_ui = response.mutable_settings();
auto* gaia_user_context_ui = settings_ui->mutable_gaia_user_context_ui();
gaia_user_context_ui->set_is_gaia_user(true);
gaia_user_context_ui->set_waa_disabled_by_dasher_domain(
(consent_ui_flags_ & CONSENT_UI_FLAG_WAA_DISABLED_BY_POLICY));
gaia_user_context_ui->set_assistant_disabled_by_dasher_domain(
(consent_ui_flags_ & CONSENT_UI_FLAG_ASSISTANT_DISABLED_BY_POLICY));
auto* consent_flow_ui = settings_ui.mutable_consent_flow_ui();
auto* consent_flow_ui = settings_ui->mutable_consent_flow_ui();
consent_flow_ui->set_consent_status(
chromeos::assistant::ConsentFlowUi_ConsentStatus_ASK_FOR_CONSENT);
consent_flow_ui->mutable_consent_ui()->set_accept_button_text("OK");
@ -251,7 +266,7 @@ class ScopedAssistantSettings : public chromeos::assistant::AssistantSettings {
if (selector_proto.email_opt_in() &&
(consent_ui_flags_ & CONSENT_UI_FLAG_ASK_EMAIL_OPT_IN)) {
auto* email_opt_in = settings_ui.mutable_email_opt_in_ui();
auto* email_opt_in = settings_ui->mutable_email_opt_in_ui();
email_opt_in->set_title("Receive email upfates");
email_opt_in->set_description("It might be useful");
email_opt_in->set_legal_text("And you can opt out");
@ -261,7 +276,7 @@ class ScopedAssistantSettings : public chromeos::assistant::AssistantSettings {
}
std::string message;
EXPECT_TRUE(settings_ui.SerializeToString(&message));
EXPECT_TRUE(response.SerializeToString(&message));
std::move(callback).Run(message);
}
@ -366,6 +381,7 @@ class ScopedAssistantSettings : public chromeos::assistant::AssistantSettings {
std::set<OptIn> collected_optins_;
int setting_zippy_size_ = 1;
bool is_minor_user_ = false;
DISALLOW_COPY_AND_ASSIGN(ScopedAssistantSettings);
};
@ -530,12 +546,17 @@ IN_PROC_BROWSER_TEST_F(AssistantOptInFlowTest, Basic) {
screen_waiter.Wait();
test::OobeJS().CreateVisibilityWaiter(true, kAssistantValueProp)->Wait();
EXPECT_TRUE(test::OobeJS().GetAttributeBool("inverse", kValuePropNextButton));
TapWhenEnabled(kValuePropNextButton);
test::OobeJS().CreateVisibilityWaiter(true, kAssistantRelatedInfo)->Wait();
EXPECT_TRUE(
test::OobeJS().GetAttributeBool("inverse", kRelatedInfoNextButton));
TapWhenEnabled(kRelatedInfoNextButton);
test::OobeJS().CreateVisibilityWaiter(true, kAssistantVoiceMatch)->Wait();
EXPECT_TRUE(
test::OobeJS().GetAttributeBool("inverse", kVoiceMatchAgreeButton));
TapWhenEnabled(kVoiceMatchAgreeButton);
WaitForScreenExit();
@ -1113,6 +1134,11 @@ class AssistantOptInFlowMinorModeTest : public AssistantOptInFlowTest {
scoped_feature_list_.Reset();
scoped_feature_list_.InitAndEnableFeature(features::kMinorModeRestriction);
}
void SetUpOnMainThread() override {
AssistantOptInFlowTest::SetUpOnMainThread();
assistant_settings_->set_is_minor_user(true);
}
};
IN_PROC_BROWSER_TEST_F(AssistantOptInFlowMinorModeTest,
@ -1131,13 +1157,21 @@ IN_PROC_BROWSER_TEST_F(AssistantOptInFlowMinorModeTest,
screen_waiter.Wait();
test::OobeJS().CreateVisibilityWaiter(true, kAssistantValueProp)->Wait();
EXPECT_FALSE(
test::OobeJS().GetAttributeBool("inverse", kValuePropNextButton));
TapWhenEnabled(kValuePropNextButton);
EXPECT_FALSE(
test::OobeJS().GetAttributeBool("inverse", kValuePropNextButton));
TapWhenEnabled(kValuePropNextButton);
test::OobeJS().CreateVisibilityWaiter(true, kAssistantRelatedInfo)->Wait();
EXPECT_FALSE(
test::OobeJS().GetAttributeBool("inverse", kRelatedInfoNextButton));
TapWhenEnabled(kRelatedInfoNextButton);
test::OobeJS().CreateVisibilityWaiter(true, kAssistantVoiceMatch)->Wait();
EXPECT_FALSE(
test::OobeJS().GetAttributeBool("inverse", kVoiceMatchAgreeButton));
TapWhenEnabled(kVoiceMatchAgreeButton);
WaitForScreenExit();

@ -136,6 +136,7 @@ Polymer({
data['flowType'] = this.flowType;
this.$.valueProp.reloadContent(data);
this.$.relatedInfo.reloadContent(data);
this.$.voiceMatch.reloadContent(data);
this.$.thirdParty.reloadContent(data);
this.$.getMore.reloadContent(data);
},

@ -95,7 +95,7 @@
<oobe-text-button id="skip-button" disabled="[[loading]]"
text-key="assistantOptinNoThanksButton" on-click="onSkipTap_">
</oobe-text-button>
<oobe-text-button id="next-button" inverse="[[!isMinorMode_]]"
<oobe-text-button id="next-button" inverse="[[!equalWeightButtons_]]"
disabled="[[loading]]" text-key="assistantOptinAgreeButton"
on-click="onNextTap_">
</oobe-text-button>

@ -47,14 +47,11 @@ Polymer({
},
/**
* Indicates whether user is minor mode user (e.g. under age of 18).
* Indicates whether to use same design for accept/decline buttons.
*/
isMinorMode_: {
equalWeightButtons_: {
type: Boolean,
value() {
return loadTimeData.valueExists('isMinorMode') &&
loadTimeData.getBoolean('isMinorMode');
}
value: false,
},
},
@ -240,6 +237,8 @@ Polymer({
'https://www.gstatic.com/images/icons/material/system/2x/' +
'info_outline_grey600_24dp.png',
this.i18n('assistantScreenContextTitle'))));
this.equalWeightButtons_ = data['equalWeightButtons'];
this.consentStringLoaded_ = true;
if (this.webViewLoaded_) {
this.onPageLoaded();

@ -51,7 +51,7 @@
disabled="[[buttonsDisabled]]">
<div id="skip-button-text" slot="text"></div>
</oobe-text-button>
<oobe-text-button id="next-button" inverse="[[!isMinorMode_]]"
<oobe-text-button id="next-button" inverse="[[!equalWeightButtons_]]"
on-click="onNextTap_" disabled="[[buttonsDisabled]]">
<div id="next-button-text" slot="text"></div>
</oobe-text-button>

@ -58,10 +58,15 @@ Polymer({
*/
isMinorMode_: {
type: Boolean,
value() {
return loadTimeData.valueExists('isMinorMode') &&
loadTimeData.getBoolean('isMinorMode');
}
value: false,
},
/**
* Indicates whether to use same design for accept/decline buttons.
*/
equalWeightButtons_: {
type: Boolean,
value: false,
},
/**
@ -300,6 +305,7 @@ Polymer({
this.$['skip-button-text'].textContent = data['valuePropSkipButton'];
this.$['footer-text'].innerHTML =
this.sanitizer_.sanitizeHtml(data['valuePropFooter']);
this.equalWeightButtons_ = data['equalWeightButtons'];
this.consentStringLoaded_ = true;
if (this.settingZippyLoaded_) {
@ -326,6 +332,9 @@ Polymer({
// `zippy_data` contains a list of lists, where each list contains the
// setting zippys that should be shown on the same screen.
// `isMinorMode` is the same for all data in `zippy_data`. We could use the
// first one and set `isMinorMode_` flag.
this.isMinorMode_ = zippy_data[0][0]['isMinorMode'];
for (var i in zippy_data) {
for (var j in zippy_data[i]) {
var data = zippy_data[i][j];

@ -48,7 +48,7 @@
disabled="[[buttonsDisabled]]"
text-key="assistantOptinNoThanksButton">
</oobe-text-button>
<oobe-text-button id="agree-button" inverse="[[!isMinorMode_]]"
<oobe-text-button id="agree-button" inverse="[[!equalWeightButtons_]]"
on-click="onAgreeTap_" disabled="[[buttonsDisabled]]"
text-key="assistantOptinAgreeButton">
</oobe-text-button>

@ -30,14 +30,11 @@ Polymer({
properties: {
/**
* Indicates whether user is minor mode user (e.g. under age of 18).
* Indicates whether to use same design for accept/decline buttons.
*/
isMinorMode_: {
equalWeightButtons_: {
type: Boolean,
value() {
return loadTimeData.valueExists('isMinorMode') &&
loadTimeData.getBoolean('isMinorMode');
}
value: false,
},
},
@ -123,6 +120,13 @@ Polymer({
this.browserProxy_ = assistant.BrowserProxyImpl.getInstance();
},
/**
* Reload the page with the given settings data.
*/
reloadContent(data) {
this.equalWeightButtons_ = data['equalWeightButtons'];
},
/**
* Reloads voice match flow.
*/

@ -96,7 +96,8 @@ assistant::SettingsUiUpdate GetEmailOptInUpdate(bool opted_in) {
}
// Helper method to create zippy data.
base::Value CreateZippyData(const SettingZippyList& zippy_list) {
base::Value CreateZippyData(const SettingZippyList& zippy_list,
bool is_minor_mode) {
base::Value zippy_data(base::Value::Type::LIST);
for (auto& setting_zippy : zippy_list) {
base::Value data(base::Value::Type::DICTIONARY);
@ -112,6 +113,7 @@ base::Value CreateZippyData(const SettingZippyList& zippy_list) {
data.SetKey("iconUri", base::Value(setting_zippy.icon_uri()));
data.SetKey("popupLink", base::Value(l10n_util::GetStringUTF16(
IDS_ASSISTANT_ACTIVITY_CONTROL_POPUP_LINK)));
data.SetKey("isMinorMode", base::Value(is_minor_mode));
zippy_data.Append(std::move(data));
}
return zippy_data;
@ -178,7 +180,8 @@ base::Value CreateGetMoreData(bool email_optin_needed,
// Get string constants for settings ui.
base::Value GetSettingsUiStrings(const assistant::SettingsUi& settings_ui,
bool activity_control_needed) {
bool activity_control_needed,
bool equal_weight_buttons) {
auto consent_ui = settings_ui.consent_flow_ui().consent_ui();
auto activity_control_ui = consent_ui.activity_control_ui();
auto third_party_disclosure_ui = consent_ui.third_party_disclosure_ui();
@ -186,6 +189,7 @@ base::Value GetSettingsUiStrings(const assistant::SettingsUi& settings_ui,
dictionary.SetKey("activityControlNeeded",
base::Value(activity_control_needed));
dictionary.SetKey("equalWeightButtons", base::Value(equal_weight_buttons));
// Add activity control string constants.
if (activity_control_needed) {

@ -59,7 +59,8 @@ assistant::SettingsUiUpdate GetEmailOptInUpdate(bool opted_in);
using SettingZippyList = google::protobuf::RepeatedPtrField<
assistant::ClassicActivityControlUiTexts::SettingZippy>;
// Helper method to create zippy data.
base::Value CreateZippyData(const SettingZippyList& zippy_list);
base::Value CreateZippyData(const SettingZippyList& zippy_list,
bool is_minor_mode);
// Helper method to create disclosure data.
base::Value CreateDisclosureData(const SettingZippyList& disclosure_list);
@ -71,7 +72,8 @@ base::Value CreateGetMoreData(bool email_optin_needed,
// Get string constants for settings ui.
base::Value GetSettingsUiStrings(const assistant::SettingsUi& settings_ui,
bool activity_control_needed);
bool activity_control_needed,
bool equal_weight_buttons);
void RecordActivityControlConsent(Profile* profile,
std::string ui_audit_key,

@ -22,6 +22,7 @@
#include "chromeos/services/assistant/public/cpp/assistant_prefs.h"
#include "chromeos/services/assistant/public/cpp/assistant_service.h"
#include "chromeos/services/assistant/public/cpp/features.h"
#include "chromeos/services/assistant/public/proto/get_settings_ui.pb.h"
#include "chromeos/services/assistant/public/proto/settings_ui.pb.h"
#include "components/login/localized_values_builder.h"
#include "components/prefs/pref_service.h"
@ -171,9 +172,6 @@ void AssistantOptInFlowScreenHandler::GetAdditionalParameters(
chromeos::assistant::features::IsVoiceMatchDisabled());
dict->SetBoolean("betterAssistantEnabled",
chromeos::assistant::features::IsBetterAssistantEnabled());
// TODO(https://crbug.com/1224850): read actual minor mode signal from account
// capability or get this info from consent server.
dict->SetBoolean("isMinorMode", features::IsMinorModeRestrictionEnabled());
BaseScreenHandler::GetAdditionalParameters(dict);
}
@ -352,7 +350,7 @@ void AssistantOptInFlowScreenHandler::SendGetSettingsRequest() {
}
assistant::SettingsUiSelector selector = GetSettingsUiSelector();
assistant::AssistantSettings::Get()->GetSettings(
assistant::AssistantSettings::Get()->GetSettingsWithHeader(
selector.SerializeAsString(),
base::BindOnce(&AssistantOptInFlowScreenHandler::OnGetSettingsResponse,
weak_factory_.GetWeakPtr()));
@ -379,7 +377,7 @@ void AssistantOptInFlowScreenHandler::UpdateValuePropScreen() {
}
void AssistantOptInFlowScreenHandler::OnGetSettingsResponse(
const std::string& settings) {
const std::string& response) {
const base::TimeDelta time_since_request_sent =
base::TimeTicks::Now() - send_request_time_;
UMA_HISTOGRAM_TIMES("Assistant.OptInFlow.GetSettingsRequestTime",
@ -393,13 +391,21 @@ void AssistantOptInFlowScreenHandler::OnGetSettingsResponse(
return;
}
assistant::SettingsUi settings_ui;
if (!settings_ui.ParseFromString(settings)) {
assistant::GetSettingsUiResponse settings_ui_response;
if (!settings_ui_response.ParseFromString(response)) {
LOG(ERROR) << "Failed to parse get settings response.";
HandleFlowFinished();
return;
}
auto settings_ui = settings_ui_response.settings();
auto header = settings_ui_response.header();
bool equal_weight_buttons =
features::IsMinorModeRestrictionEnabled() &&
header.footer_button_layout() ==
assistant::SettingsResponseHeader_AcceptRejectLayout_EQUAL_WEIGHT;
if (settings_ui.has_gaia_user_context_ui()) {
auto gaia_user_context_ui = settings_ui.gaia_user_context_ui();
if (gaia_user_context_ui.assistant_disabled_by_dasher_domain()) {
@ -429,6 +435,8 @@ void AssistantOptInFlowScreenHandler::OnGetSettingsResponse(
base::Value zippy_data(base::Value::Type::LIST);
bool skip_activity_control = true;
pending_consent_data_.clear();
// We read from `multi_consent_ui` field if it is populated and we assume user
// is a minor user; otherwise, we read from `consent_ui` field.
if (features::IsMinorModeRestrictionEnabled() &&
settings_ui.consent_flow_ui().multi_consent_ui().size()) {
auto multi_consent_ui = settings_ui.consent_flow_ui().multi_consent_ui();
@ -440,7 +448,8 @@ void AssistantOptInFlowScreenHandler::OnGetSettingsResponse(
data.consent_token = activity_control_ui.consent_token();
data.ui_audit_key = activity_control_ui.ui_audit_key();
pending_consent_data_.push_back(data);
zippy_data.Append(CreateZippyData(activity_control_ui.setting_zippy()));
zippy_data.Append(CreateZippyData(activity_control_ui.setting_zippy(),
/*is_minor_mode=*/true));
}
}
} else {
@ -451,7 +460,8 @@ void AssistantOptInFlowScreenHandler::OnGetSettingsResponse(
data.consent_token = activity_control_ui.consent_token();
data.ui_audit_key = activity_control_ui.ui_audit_key();
pending_consent_data_.push_back(data);
zippy_data.Append(CreateZippyData(activity_control_ui.setting_zippy()));
zippy_data.Append(CreateZippyData(activity_control_ui.setting_zippy(),
/*is_minor_mode=*/false));
}
}
@ -507,7 +517,8 @@ void AssistantOptInFlowScreenHandler::OnGetSettingsResponse(
}
// Pass string constants dictionary.
auto dictionary = GetSettingsUiStrings(settings_ui, activity_control_needed_);
auto dictionary = GetSettingsUiStrings(settings_ui, activity_control_needed_,
equal_weight_buttons);
PrefService* prefs = ProfileManager::GetActiveUserProfile()->GetPrefs();
dictionary.SetKey("voiceMatchEnforcedOff",
base::Value(IsVoiceMatchEnforcedOff(prefs)));

@ -35,7 +35,15 @@ void AssistantSettingsImpl::Initialize(
void AssistantSettingsImpl::GetSettings(const std::string& selector,
GetSettingsCallback callback) {
settings_controller().GetSettings(selector, std::move(callback));
settings_controller().GetSettings(selector, /*include_header=*/false,
std::move(callback));
}
void AssistantSettingsImpl::GetSettingsWithHeader(
const std::string& selector,
GetSettingsCallback callback) {
settings_controller().GetSettings(selector, /*include_header=*/true,
std::move(callback));
}
void AssistantSettingsImpl::UpdateSettings(const std::string& update,

@ -40,6 +40,8 @@ class AssistantSettingsImpl : public AssistantSettings {
// AssistantSettings overrides:
void GetSettings(const std::string& selector,
GetSettingsCallback callback) override;
void GetSettingsWithHeader(const std::string& selector,
GetSettingsCallback callback) override;
void UpdateSettings(const std::string& update,
UpdateSettingsCallback callback) override;
void StartSpeakerIdEnrollment(

@ -41,12 +41,17 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC) AssistantSettings {
// |selector| is a serialized proto of SettingsUiSelector, indicating which
// settings sub-pages should be requested to the server.
// Return value is a serialized proto of SettingsUi, containing the settings
// sub-pages requested.
// For `GetSettings`, a serialized proto of SettingsUi containing the settings
// sub-pages requested is paased to `GetSettingsCallback`.
// For `GetSettingsWithHeader`, a serialized proto of GetSettingsUiResponse
// containing the settings and the header information is passed to
// `GetSettingsCallback`.
// Send a request for the settings ui sub-pages indicated by the |selector|.
using GetSettingsCallback = base::OnceCallback<void(const std::string&)>;
virtual void GetSettings(const std::string& selector,
GetSettingsCallback callback) = 0;
virtual void GetSettingsWithHeader(const std::string& selector,
GetSettingsCallback callback) = 0;
// |update| is a serialized proto of SettingsUiUpdate, indicating what kind
// of updates should be applied to the settings.

@ -14,6 +14,7 @@ proto_library("proto") {
"email_opt_in_ui.proto",
"gaia_user_context_ui.proto",
"get_settings_ui.proto",
"header.proto",
"settings_ui.proto",
]
}

@ -8,6 +8,7 @@ option optimize_for = LITE_RUNTIME;
package chromeos.assistant;
import "header.proto";
import "settings_ui.proto";
// GetSettingsUi.
@ -16,3 +17,9 @@ message GetSettingsUiRequest {
// Required.
optional SettingsUiSelector selector = 4;
}
message GetSettingsUiResponse {
optional SettingsUi settings = 1;
optional SettingsResponseHeader header = 2;
}

@ -0,0 +1,21 @@
// Copyright 2021 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.
syntax = "proto2";
option optimize_for = LITE_RUNTIME;
package chromeos.assistant;
// Metadata for the response back to the user.
message SettingsResponseHeader {
enum AcceptRejectLayout {
// Unspecified layout for buttons.
UNSPECIFIED = 0;
// Accept is filled on RHS, Reject is filled on LHS.
EQUAL_WEIGHT = 1;
}
// Controls the way the accept and reject buttons are rendered.
optional AcceptRejectLayout footer_button_layout = 2;
}

@ -7,6 +7,7 @@
#include <utility>
#include "base/callback.h"
#include "chromeos/services/assistant/public/proto/get_settings_ui.pb.h"
#include "chromeos/services/assistant/public/proto/settings_ui.pb.h"
namespace chromeos {
@ -25,6 +26,16 @@ void FakeAssistantSettingsImpl::GetSettings(const std::string& selector,
std::move(callback).Run(settings_ui.SerializeAsString());
}
void FakeAssistantSettingsImpl::GetSettingsWithHeader(
const std::string& selector,
GetSettingsCallback callback) {
// Create a fake response
assistant::GetSettingsUiResponse response;
response.mutable_settings()->mutable_consent_flow_ui()->set_consent_status(
ConsentFlowUi_ConsentStatus_ALREADY_CONSENTED);
std::move(callback).Run(response.SerializeAsString());
}
void FakeAssistantSettingsImpl::UpdateSettings(
const std::string& update,
UpdateSettingsCallback callback) {

@ -22,6 +22,8 @@ class FakeAssistantSettingsImpl : public AssistantSettings {
// AssistantSettings overrides:
void GetSettings(const std::string& selector,
GetSettingsCallback callback) override;
void GetSettingsWithHeader(const std::string& selector,
GetSettingsCallback callback) override;
void UpdateSettings(const std::string& update,
UpdateSettingsCallback callback) override;
void StartSpeakerIdEnrollment(

@ -122,6 +122,7 @@ void FakeServiceController::UpdateSettings(const std::string& settings,
}
void FakeServiceController::GetSettings(const std::string& selector,
bool include_header,
GetSettingsCallback callback) {
// Callback must be called to satisfy the mojom contract.
std::move(callback).Run(std::string());

@ -98,6 +98,7 @@ class FakeServiceController
void UpdateSettings(const std::string& settings,
UpdateSettingsCallback callback) override;
void GetSettings(const std::string& selector,
bool include_header,
GetSettingsCallback callback) override;
void SetHotwordEnabled(bool value) override {}

@ -31,10 +31,12 @@ interface SettingsController {
// Retrieve settings. |selector| is a serialized proto of
// |SettingsUiSelector|, indicating which settings sub-pages should be
// requested to the server.
// On success, the return value is a serialized proto of |SettingsUi|,
// containing the requested sub-pages.
// On success, the return value is a serialized proto of
// |GetSettingsUiResponse| when `include_header` is true; otherwisse, the
// return value is a serialized proto of |SettingsUi|, containing the
// requested sub-pages.
// On failure, the return value is the empty string.
GetSettings(string selector) => (string result);
GetSettings(string selector, bool include_header) => (string result);
// Update settings. |update| is a serialized proto of |SettingsUiUpdate|,
// indicating the newly updated values.
@ -49,4 +51,3 @@ struct AuthenticationToken {
string gaia_id;
string access_token;
};

@ -112,7 +112,9 @@ class SettingsControllerMock : public mojom::SettingsController {
MOCK_METHOD(void, SetHotwordEnabled, (bool value));
MOCK_METHOD(void,
GetSettings,
(const std::string& selector, GetSettingsCallback callback));
(const std::string& selector,
bool include_header,
GetSettingsCallback callback));
MOCK_METHOD(void,
UpdateSettings,
(const std::string& settings, UpdateSettingsCallback callback));

@ -117,9 +117,9 @@ class SettingsController::DeviceSettingsUpdater
// running or stopped.
class GetSettingsResponseWaiter : public AbortableTask {
public:
explicit GetSettingsResponseWaiter(
SettingsController::GetSettingsCallback callback)
: callback_(std::move(callback)) {}
GetSettingsResponseWaiter(SettingsController::GetSettingsCallback callback,
bool include_header)
: callback_(std::move(callback)), include_header_(include_header) {}
GetSettingsResponseWaiter(const GetSettingsResponseWaiter&) = delete;
GetSettingsResponseWaiter& operator=(const GetSettingsResponseWaiter&) =
@ -154,11 +154,16 @@ class GetSettingsResponseWaiter : public AbortableTask {
private:
void OnResponse(const assistant_client::VoicelessResponse& response) {
std::string result = assistant::UnwrapGetSettingsUiResponse(response);
std::string result =
assistant::UnwrapGetSettingsUiResponse(response, include_header_);
std::move(callback_).Run(result);
}
SettingsController::GetSettingsCallback callback_;
// Whether to include header in response. If this is true, a serialized proto
// of GetSettingsUiResponse is passed to the callback; otherwise, a serialized
// proto of SettingsUi is passed to the callback.
bool include_header_;
base::WeakPtrFactory<GetSettingsResponseWaiter> weak_factory_{this};
};
@ -247,9 +252,11 @@ void SettingsController::SetHotwordEnabled(bool value) {
}
void SettingsController::GetSettings(const std::string& selector,
bool include_header,
GetSettingsCallback callback) {
auto* waiter = pending_response_waiters_.Add(
std::make_unique<GetSettingsResponseWaiter>(std::move(callback)));
auto* waiter =
pending_response_waiters_.Add(std::make_unique<GetSettingsResponseWaiter>(
std::move(callback), include_header));
waiter->SendRequest(assistant_manager_internal_, selector);
}

@ -40,6 +40,7 @@ class SettingsController : public AssistantClientObserver,
void SetSpokenFeedbackEnabled(bool value) override;
void SetHotwordEnabled(bool value) override;
void GetSettings(const std::string& selector,
bool include_header,
GetSettingsCallback callback) override;
void UpdateSettings(const std::string& settings,
UpdateSettingsCallback callback) override;

@ -349,7 +349,7 @@ TEST_F(AssistantSettingsControllerTest,
EXPECT_CALL(callback, Run(std::string{}));
controller().GetSettings("selector", callback.Get());
controller().GetSettings("selector", /*include_header=*/false, callback.Get());
}
TEST_F(AssistantSettingsControllerTest,
@ -357,7 +357,7 @@ TEST_F(AssistantSettingsControllerTest,
CreateAndStartLibassistant();
base::MockCallback<SettingsController::GetSettingsCallback> callback;
controller().GetSettings("selector", callback.Get());
controller().GetSettings("selector", /*include_header=*/false, callback.Get());
EXPECT_CALL(callback, Run(std::string{}));
DestroyLibassistant();