Skip to content

Commit

Permalink
feedback: Display localized suggested help contents
Browse files Browse the repository at this point in the history
There is a language field in the search request that indicates what
language to return response in.  Set it to the locale of the
application.

Add OsFeedbackDelegate to allow Feedback UI to access chrome/browser/.

      unit_tests --gtest_filter=ChromeOsFeedbackDelegateTest*

Bug: b:185624798
Test: browser_tests --gtest_filter=OSFeedbackBrowserTest.* \
Change-Id: Iaefaf94ffb0be67333bd493ecccce3430d0ef09a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3540653
Reviewed-by: Gavin Williams <gavinwill@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Xiangdong Kong <xiangdongkong@google.com>
Cr-Commit-Position: refs/heads/main@{#984366}
  • Loading branch information
xiangdong kong authored and Chromium LUCI CQ committed Mar 23, 2022
1 parent 1af111a commit 01ae532
Show file tree
Hide file tree
Showing 13 changed files with 143 additions and 18 deletions.
1 change: 1 addition & 0 deletions ash/webui/os_feedback_ui/BUILD.gn
Expand Up @@ -9,6 +9,7 @@ assert(is_chromeos_ash, "Non-ChromeOS builds cannot depend on //ash")

static_library("os_feedback_ui") {
sources = [
"os_feedback_delegate.h",
"os_feedback_ui.cc",
"os_feedback_ui.h",
"os_feedback_untrusted_ui.cc",
Expand Down
17 changes: 10 additions & 7 deletions ash/webui/os_feedback_ui/backend/help_content_provider.cc
Expand Up @@ -156,18 +156,18 @@ HelpContentPtr GetHelpContent(const base::Value::Dict& data) {
} // namespace

std::string ConvertSearchRequestToJson(
const std::string& app_locale,
const os_feedback_ui::mojom::SearchRequestPtr& request) {
base::Value::Dict request_dict;

request_dict.Set("helpcenter", "chromeos");
request_dict.Set("query", request->query);
// TODO(xiangdongkong): use UI language.
request_dict.Set("language", "en");
request_dict.Set("language", app_locale);
request_dict.Set("max_results", base::NumberToString(request->max_results));

std::string request_content;
base::JSONWriter::Write(request_dict, &request_content);
DVLOG(2) << "HelpContentProvider request body: " << request_content;
VLOG(2) << request_content;
return request_content;
}

Expand Down Expand Up @@ -221,13 +221,16 @@ void PopulateSearchResponse(const base::Value& search_result,
}

HelpContentProvider::HelpContentProvider(
const std::string& app_locale,
content::BrowserContext* browser_context)
: url_loader_factory_(browser_context->GetDefaultStoragePartition()
: app_locale_(app_locale),
url_loader_factory_(browser_context->GetDefaultStoragePartition()
->GetURLLoaderFactoryForBrowserProcess()) {}

HelpContentProvider::HelpContentProvider(
const std::string& app_locale,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory)
: url_loader_factory_(url_loader_factory) {}
: app_locale_(app_locale), url_loader_factory_(url_loader_factory) {}

HelpContentProvider::~HelpContentProvider() = default;

Expand All @@ -238,8 +241,8 @@ void HelpContentProvider::GetHelpContents(

auto url_loader = network::SimpleURLLoader::Create(
std::move(resource_request), kTrafficAnnotation);
url_loader->AttachStringForUpload(ConvertSearchRequestToJson(request),
"application/json");
url_loader->AttachStringForUpload(
ConvertSearchRequestToJson(app_locale_, request), "application/json");
url_loader->DownloadToString(
url_loader_factory_.get(),
base::BindOnce(&HelpContentProvider::OnHelpContentSearchResponse,
Expand Down
8 changes: 6 additions & 2 deletions ash/webui/os_feedback_ui/backend/help_content_provider.h
Expand Up @@ -33,6 +33,7 @@ using GetHelpContentsCallback =
// Convert a search request to a JSON string as the payload to be sent to the
// search API.
std::string ConvertSearchRequestToJson(
const std::string& app_locale,
const os_feedback_ui::mojom::SearchRequestPtr& request);

// Convert the result_type string to HelpContentType.
Expand Down Expand Up @@ -65,8 +66,10 @@ void PopulateSearchResponse(
// GetHelpContents.
class HelpContentProvider : os_feedback_ui::mojom::HelpContentProvider {
public:
explicit HelpContentProvider(content::BrowserContext* browser_context);
explicit HelpContentProvider(
HelpContentProvider(const std::string& app_locale,
content::BrowserContext* browser_context);
HelpContentProvider(
const std::string& app_locale,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory);
HelpContentProvider(const HelpContentProvider&) = delete;
HelpContentProvider& operator=(const HelpContentProvider&) = delete;
Expand All @@ -91,6 +94,7 @@ class HelpContentProvider : os_feedback_ui::mojom::HelpContentProvider {
void OnResponseJsonParsed(GetHelpContentsCallback callback,
data_decoder::DataDecoder::ValueOrError result);

std::string app_locale_;
// Decoder for data decoding service.
data_decoder::DataDecoder data_decoder_;
// URLLoaderFactory used for network requests.
Expand Down
Expand Up @@ -58,8 +58,8 @@ class HelpContentProviderTest : public testing::Test {
test_shared_loader_factory_ =
base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
&test_url_loader_factory_);
provider_ =
std::make_unique<HelpContentProvider>(test_shared_loader_factory_);
provider_ = std::make_unique<HelpContentProvider>(
"en", test_shared_loader_factory_);
}
~HelpContentProviderTest() override = default;

Expand Down Expand Up @@ -121,9 +121,9 @@ TEST_F(HelpContentProviderTest, ConvertToHelpContentType) {
// Test the ConvertSearchRequestToJson utility function.
TEST_F(HelpContentProviderTest, ConvertSearchRequestToJson) {
auto request = SearchRequest::New(u"how do", 10);
EXPECT_EQ(R"({"helpcenter":"chromeos","language":"en",)"
EXPECT_EQ(R"({"helpcenter":"chromeos","language":"zh",)"
R"("max_results":"10","query":"how do"})",
ConvertSearchRequestToJson(request));
ConvertSearchRequestToJson("zh", request));
}

// Test the PopulateSearchResponse utility function with empty json string.
Expand Down
23 changes: 23 additions & 0 deletions ash/webui/os_feedback_ui/os_feedback_delegate.h
@@ -0,0 +1,23 @@
// Copyright 2022 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 ASH_WEBUI_OS_FEEDBACK_UI_OS_FEEDBACK_DELEGATE_H_
#define ASH_WEBUI_OS_FEEDBACK_UI_OS_FEEDBACK_DELEGATE_H_

namespace ash {

// A delegate which exposes browser functionality from //chrome to the OS
// Feedback UI.
class OsFeedbackDelegate {
public:
virtual ~OsFeedbackDelegate() = default;

// Gets the application locale so that suggested help contents can display
// localized titles when available.
virtual std::string GetApplicationLocale() = 0;
};

} // namespace ash

#endif // ASH_WEBUI_OS_FEEDBACK_UI_OS_FEEDBACK_DELEGATE_H_
12 changes: 8 additions & 4 deletions ash/webui/os_feedback_ui/os_feedback_ui.cc
Expand Up @@ -11,6 +11,7 @@
#include "ash/webui/grit/ash_os_feedback_resources_map.h"
#include "ash/webui/os_feedback_ui/backend/help_content_provider.h"
#include "ash/webui/os_feedback_ui/mojom/os_feedback_ui.mojom.h"
#include "ash/webui/os_feedback_ui/os_feedback_delegate.h"
#include "ash/webui/os_feedback_ui/url_constants.h"
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_ui.h"
Expand Down Expand Up @@ -38,8 +39,11 @@ void SetUpWebUIDataSource(content::WebUIDataSource* source,

} // namespace

OSFeedbackUI::OSFeedbackUI(content::WebUI* web_ui)
: MojoWebUIController(web_ui) {
OSFeedbackUI::OSFeedbackUI(
content::WebUI* web_ui,
std::unique_ptr<OsFeedbackDelegate> feedback_delegate)
: MojoWebUIController(web_ui),
feedback_delegate_(std::move(feedback_delegate)) {
auto* browser_context = web_ui->GetWebContents()->GetBrowserContext();
content::WebUIDataSource* source = content::WebUIDataSource::CreateAndAdd(
browser_context, kChromeUIOSFeedbackHost);
Expand Down Expand Up @@ -71,8 +75,8 @@ OSFeedbackUI::OSFeedbackUI(content::WebUI* web_ui)
webui_allowlist->RegisterAutoGrantedPermission(
untrusted_origin, ContentSettingsType::JAVASCRIPT);

helpContentProvider_ =
std::make_unique<feedback::HelpContentProvider>(browser_context);
helpContentProvider_ = std::make_unique<feedback::HelpContentProvider>(
feedback_delegate_->GetApplicationLocale(), browser_context);
}

OSFeedbackUI::~OSFeedbackUI() = default;
Expand Down
6 changes: 5 additions & 1 deletion ash/webui/os_feedback_ui/os_feedback_ui.h
Expand Up @@ -17,9 +17,12 @@ class WebUI;

namespace ash {

class OsFeedbackDelegate;

class OSFeedbackUI : public ui::MojoWebUIController {
public:
explicit OSFeedbackUI(content::WebUI* web_ui);
OSFeedbackUI(content::WebUI* web_ui,
std::unique_ptr<OsFeedbackDelegate> feedback_delegate);
OSFeedbackUI(const OSFeedbackUI&) = delete;
OSFeedbackUI& operator=(const OSFeedbackUI&) = delete;
~OSFeedbackUI() override;
Expand All @@ -29,6 +32,7 @@ class OSFeedbackUI : public ui::MojoWebUIController {
receiver);

private:
std::unique_ptr<OsFeedbackDelegate> feedback_delegate_;
std::unique_ptr<feedback::HelpContentProvider> helpContentProvider_;

WEB_UI_CONTROLLER_TYPE_DECL();
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/ash/os_feedback/OWNERS
@@ -0,0 +1 @@
file://ash/webui/os_feedback_ui/OWNERS
20 changes: 20 additions & 0 deletions chrome/browser/ash/os_feedback/chrome_os_feedback_delegate.cc
@@ -0,0 +1,20 @@
// Copyright 2022 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/ash/os_feedback/chrome_os_feedback_delegate.h"

#include <string>

#include "chrome/browser/browser_process.h"

namespace ash {

ChromeOsFeedbackDelegate::ChromeOsFeedbackDelegate() = default;
ChromeOsFeedbackDelegate::~ChromeOsFeedbackDelegate() = default;

std::string ChromeOsFeedbackDelegate::GetApplicationLocale() {
return g_browser_process->GetApplicationLocale();
}

} // namespace ash
28 changes: 28 additions & 0 deletions chrome/browser/ash/os_feedback/chrome_os_feedback_delegate.h
@@ -0,0 +1,28 @@
// Copyright 2022 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_ASH_OS_FEEDBACK_CHROME_OS_FEEDBACK_DELEGATE_H_
#define CHROME_BROWSER_ASH_OS_FEEDBACK_CHROME_OS_FEEDBACK_DELEGATE_H_

#include <string>

#include "ash/webui/os_feedback_ui/os_feedback_delegate.h"

namespace ash {

class ChromeOsFeedbackDelegate : public OsFeedbackDelegate {
public:
ChromeOsFeedbackDelegate();
~ChromeOsFeedbackDelegate() override;

ChromeOsFeedbackDelegate(const ChromeOsFeedbackDelegate&) = delete;
ChromeOsFeedbackDelegate& operator=(const ChromeOsFeedbackDelegate&) = delete;

// OsFeedbackDelegate:
std::string GetApplicationLocale() override;
};

} // namespace ash

#endif // CHROME_BROWSER_ASH_OS_FEEDBACK_CHROME_OS_FEEDBACK_DELEGATE_H_
@@ -0,0 +1,26 @@
// Copyright 2022 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/ash/os_feedback/chrome_os_feedback_delegate.h"

#include "chrome/browser/browser_process.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace ash {

class ChromeOsFeedbackDelegateTest : public testing::Test {
public:
ChromeOsFeedbackDelegateTest() {}
~ChromeOsFeedbackDelegateTest() override = default;

protected:
ChromeOsFeedbackDelegate feedback_delegate_;
};

// Test GetApplicationLocale returns a valid locale.
TEST_F(ChromeOsFeedbackDelegateTest, GetApplicationLocale) {
EXPECT_EQ(feedback_delegate_.GetApplicationLocale(), "en");
}

} // namespace ash
3 changes: 3 additions & 0 deletions chrome/browser/chromeos/BUILD.gn
Expand Up @@ -2419,6 +2419,8 @@ source_set("chromeos") {
"../ash/notifications/tpm_auto_update_notification.h",
"../ash/notifications/update_required_notification.cc",
"../ash/notifications/update_required_notification.h",
"../ash/os_feedback/chrome_os_feedback_delegate.cc",
"../ash/os_feedback/chrome_os_feedback_delegate.h",
"../ash/ownership/fake_owner_settings_service.cc",
"../ash/ownership/fake_owner_settings_service.h",
"../ash/ownership/owner_settings_service_ash.cc",
Expand Down Expand Up @@ -4478,6 +4480,7 @@ source_set("unit_tests") {
"../ash/notifications/mock_adb_sideloading_policy_change_notification.h",
"../ash/notifications/request_system_proxy_credentials_view_unittest.cc",
"../ash/notifications/update_required_notification_unittest.cc",
"../ash/os_feedback/chrome_os_feedback_delegate_unittest.cc",
"../ash/ownership/owner_settings_service_ash_unittest.cc",
"../ash/pcie_peripheral/ash_usb_detector_unittest.cc",
"../ash/phonehub/browser_tabs_metadata_fetcher_impl_unittest.cc",
Expand Down
8 changes: 8 additions & 0 deletions chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc
Expand Up @@ -189,6 +189,7 @@
#include "ash/webui/media_app_ui/url_constants.h"
#include "ash/webui/multidevice_debug/proximity_auth_ui.h"
#include "ash/webui/multidevice_debug/url_constants.h"
#include "ash/webui/os_feedback_ui/os_feedback_delegate.h"
#include "ash/webui/os_feedback_ui/os_feedback_ui.h"
#include "ash/webui/os_feedback_ui/url_constants.h"
#include "ash/webui/personalization_app/personalization_app_ui.h"
Expand Down Expand Up @@ -216,6 +217,7 @@
#include "chrome/browser/ash/login/login_pref_names.h"
#include "chrome/browser/ash/multidevice_setup/multidevice_setup_service_factory.h"
#include "chrome/browser/ash/net/network_health/network_health_service.h"
#include "chrome/browser/ash/os_feedback/chrome_os_feedback_delegate.h"
#include "chrome/browser/ash/printing/print_management/printing_manager.h"
#include "chrome/browser/ash/printing/print_management/printing_manager_factory.h"
#include "chrome/browser/ash/scanning/chrome_scanning_app_delegate.h"
Expand Down Expand Up @@ -555,6 +557,12 @@ WebUIController* NewWebUI<ash::eche_app::EcheAppUI>(WebUI* web_ui,
base::BindRepeating(&BindEcheDisplayStreamHandler, manager));
}

template <>
WebUIController* NewWebUI<ash::OSFeedbackUI>(WebUI* web_ui, const GURL& url) {
return new ash::OSFeedbackUI(
web_ui, std::make_unique<ash::ChromeOsFeedbackDelegate>());
}

void BindScanService(
Profile* profile,
mojo::PendingReceiver<ash::scanning::mojom::ScanService> pending_receiver) {
Expand Down

0 comments on commit 01ae532

Please sign in to comment.