Send lacros feedback from ash via mojo.

When user invokes feedback from lacros-chrome, we will send
the request via crosapi mojo call to ash to show Feedback ui,
so that the feedback report will be sent from ash, which includes
both ash and lacros user log.

The chrome version string in feedback report will contain both ash
and lacros versions if Lacros is allowed/supported and launched
in the session.

Bug:1119925
TEST:
1. Open Lacros browser, send a feedback report by clicking
"Report an issue" under Help menu.
2. Verify the report can be found at
http://listnr site. The chrome version string should have
both lacros and ash version strings, for example, "Lacros 87.0.4266.0 dev, Ash 87.0.4266.0 dev".
3. Verify the report contains both system_logs.zip and histograms.zip.
The system log should contain ash log data and lacros user log from
/home/chronos/user/lacros/lacros.log under "lacros_user_log" section.

Change-Id: Iecc94ef9c50d6993159aebd9d3ac305e3f9a5896
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2408883Reviewed-by: Greg Kerr <kerrnel@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Miriam Zimmerman <mutexlox@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Jenny Zhang <jennyz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811454}

chrome/browser/BUILD.gn

@@ -4354,6 +4354,7 @@ static_library("browser") {
   if (chromeos_is_browser_only) {
     assert(enable_native_notifications)
     sources += [
+      "feedback/show_feedback_page_lacros.cc",
       "first_run/first_run_internal_lacros.cc",
       "lacros/lacros_chrome_service_delegate_impl.cc",
       "lacros/lacros_chrome_service_delegate_impl.h",

chrome/browser/chromeos/BUILD.gn

@@ -959,6 +959,8 @@ source_set("chromeos") {
     "crosapi/browser_manager.h",
     "crosapi/browser_util.cc",
     "crosapi/browser_util.h",
+    "crosapi/feedback_ash.cc",
+    "crosapi/feedback_ash.h",
     "crosapi/keystore_service_ash.cc",
     "crosapi/keystore_service_ash.h",
     "crosapi/message_center_ash.cc",

chrome/browser/chromeos/crosapi/ash_chrome_service_impl.cc

@@ -9,10 +9,13 @@
 #include <vector>
 
 #include "base/logging.h"
+#include "chrome/browser/chromeos/crosapi/browser_manager.h"
+#include "chrome/browser/chromeos/crosapi/feedback_ash.h"
 #include "chrome/browser/chromeos/crosapi/keystore_service_ash.h"
 #include "chrome/browser/chromeos/crosapi/message_center_ash.h"
 #include "chrome/browser/chromeos/crosapi/screen_manager_ash.h"
 #include "chrome/browser/chromeos/crosapi/select_file_ash.h"
+#include "chromeos/crosapi/mojom/feedback.mojom.h"
 #include "chromeos/crosapi/mojom/keystore_service.mojom.h"
 #include "chromeos/crosapi/mojom/message_center.mojom.h"
 #include "chromeos/crosapi/mojom/screen_manager.mojom.h"
@@ -58,4 +61,13 @@ void AshChromeServiceImpl::BindHidManager(
   content::GetDeviceService().BindHidManager(std::move(receiver));
 }
 
+void AshChromeServiceImpl::BindFeedback(
+    mojo::PendingReceiver<mojom::Feedback> receiver) {
+  feedback_ash_ = std::make_unique<FeedbackAsh>(std::move(receiver));
+}
+
+void AshChromeServiceImpl::OnLacrosStartup(mojom::LacrosInfoPtr lacros_info) {
+  BrowserManager::Get()->set_lacros_version(lacros_info->lacros_version);
+}
+
 }  // namespace crosapi

chrome/browser/chromeos/crosapi/ash_chrome_service_impl.h

@@ -13,6 +13,7 @@
 
 namespace crosapi {
 
+class FeedbackAsh;
 class KeystoreServiceAsh;
 class MessageCenterAsh;
 class ScreenManagerAsh;
@@ -37,6 +38,8 @@ class AshChromeServiceImpl : public mojom::AshChromeService {
       mojo::PendingReceiver<mojom::SelectFile> receiver) override;
   void BindHidManager(
       mojo::PendingReceiver<device::mojom::HidManager> receiver) override;
+  void BindFeedback(mojo::PendingReceiver<mojom::Feedback> receiver) override;
+  void OnLacrosStartup(mojom::LacrosInfoPtr lacros_info) override;
 
  private:
   mojo::Receiver<mojom::AshChromeService> receiver_;
@@ -45,6 +48,7 @@ class AshChromeServiceImpl : public mojom::AshChromeService {
   std::unique_ptr<MessageCenterAsh> message_center_ash_;
   std::unique_ptr<ScreenManagerAsh> screen_manager_ash_;
   std::unique_ptr<SelectFileAsh> select_file_ash_;
+  std::unique_ptr<FeedbackAsh> feedback_ash_;
 };
 
 }  // namespace crosapi

chrome/browser/chromeos/crosapi/browser_manager.h

@@ -67,6 +67,11 @@ class BrowserManager : public session_manager::SessionManagerObserver {
   // so should be avoided.
   void NewWindow();
 
+  const std::string& lacros_version() const { return lacros_version_; }
+  void set_lacros_version(const std::string& version) {
+    lacros_version_ = version;
+  }
+
  private:
   enum class State {
     // Lacros is not initialized yet.
@@ -136,6 +141,12 @@ class BrowserManager : public session_manager::SessionManagerObserver {
   // Path to the lacros-chrome disk image directory.
   base::FilePath lacros_path_;
 
+  // Version of lacros-chrome displayed to user in feedback report, etc.
+  // It includes both browser version and channel in the format of:
+  // {browser version} {channel}
+  // For example, "87.0.0.1 dev", "86.0.4240.38 beta".
+  std::string lacros_version_;
+
   // Called when the binary download completes.
   LoadCompleteCallback load_complete_callback_;
 

chrome/browser/chromeos/crosapi/feedback_ash.cc

+// Copyright 2020 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/chromeos/crosapi/feedback_ash.h"
+
+#include "chrome/browser/chromeos/login/users/chrome_user_manager.h"
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
+#include "chrome/browser/ui/chrome_pages.h"
+
+namespace crosapi {
+
+namespace {
+
+chrome::FeedbackSource FromMojo(mojom::LacrosFeedbackSource source) {
+  switch (source) {
+    case mojom::LacrosFeedbackSource::kLacrosBrowserCommand:
+      return chrome::kFeedbackSourceBrowserCommand;
+    case mojom::LacrosFeedbackSource::kLacrosSettingsAboutPage:
+      return chrome::kFeedbackSourceMdSettingsAboutPage;
+  }
+}
+
+}  // namespace
+
+FeedbackAsh::FeedbackAsh(mojo::PendingReceiver<mojom::Feedback> receiver)
+    : receiver_(this, std::move(receiver)) {}
+
+FeedbackAsh::~FeedbackAsh() = default;
+
+void FeedbackAsh::ShowFeedbackPage(mojom::FeedbackInfoPtr feedback_info) {
+  const user_manager::User* user =
+      user_manager::UserManager::Get()->GetPrimaryUser();
+  if (!user) {
+    LOG(ERROR) << "Cannot invoke feedback for lacros: No primary user found!";
+    return;
+  }
+  Profile* profile = chromeos::ProfileHelper::Get()->GetProfileByUser(user);
+  if (!profile) {
+    LOG(ERROR)
+        << "Cannot invoke feedback for lacros: No primary profile found!";
+    return;
+  }
+  chrome::ShowFeedbackPage(
+      feedback_info->page_url, profile, FromMojo(feedback_info->source),
+      feedback_info->description_template,
+      feedback_info->description_placeholder_text, feedback_info->category_tag,
+      feedback_info->extra_diagnostics);
+}
+
+}  // namespace crosapi

chrome/browser/chromeos/crosapi/feedback_ash.h

+// Copyright 2020 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_CHROMEOS_CROSAPI_FEEDBACK_ASH_H_
+#define CHROME_BROWSER_CHROMEOS_CROSAPI_FEEDBACK_ASH_H_
+
+#include "chromeos/crosapi/mojom/feedback.mojom.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "mojo/public/cpp/bindings/receiver.h"
+
+namespace crosapi {
+
+// Implements the crosapi feedback interface. Lives in ash-chrome on the
+// UI thread. Shows feedback page in response to mojo IPCs from lacros-chrome.
+class FeedbackAsh : public mojom::Feedback {
+ public:
+  explicit FeedbackAsh(mojo::PendingReceiver<mojom::Feedback> receiver);
+  FeedbackAsh(const FeedbackAsh&) = delete;
+  FeedbackAsh& operator=(const FeedbackAsh&) = delete;
+  ~FeedbackAsh() override;
+
+  // crosapi::mojom::Feedback:
+  void ShowFeedbackPage(mojom::FeedbackInfoPtr feedback_info) override;
+
+ private:
+  mojo::Receiver<mojom::Feedback> receiver_;
+};
+
+}  // namespace crosapi
+
+#endif  // CHROME_BROWSER_CHROMEOS_CROSAPI_FEEDBACK_ASH_H_

chrome/browser/feedback/show_feedback_page.cc

@@ -6,6 +6,7 @@
 
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string_util.h"
+#include "build/build_config.h"
 #include "chrome/browser/feedback/feedback_dialog_utils.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
@@ -23,6 +24,10 @@
 #include "google_apis/gaia/gaia_auth_util.h"
 #endif
 
+#if BUILDFLAG(IS_LACROS)
+#include "chromeos/lacros/lacros_chrome_service_impl.h"
+#endif
+
 namespace feedback_private = extensions::api::feedback_private;
 
 namespace chrome {
@@ -59,8 +64,54 @@ bool IsFromUserInteraction(FeedbackSource source) {
   }
 }
 #endif
+
+// TODO(http://crbug.com/1132106): Include the following code only in
+// non-lacros builds after M87 btea when Feedback crosapi is available in all
+// ash versions.
+// Calls feedback private api to show Feedback ui.
+void RequestFeedbackFlow(const GURL& page_url,
+                         Profile* profile,
+                         FeedbackSource source,
+                         const std::string& description_template,
+                         const std::string& description_placeholder_text,
+                         const std::string& category_tag,
+                         const std::string& extra_diagnostics) {
+  extensions::FeedbackPrivateAPI* api =
+      extensions::FeedbackPrivateAPI::GetFactoryInstance()->Get(profile);
+
+  feedback_private::FeedbackFlow flow =
+      source == kFeedbackSourceSadTabPage
+          ? feedback_private::FeedbackFlow::FEEDBACK_FLOW_SADTABCRASH
+          : feedback_private::FeedbackFlow::FEEDBACK_FLOW_REGULAR;
+
+  bool include_bluetooth_logs = false;
+#if defined(OS_CHROMEOS)
+  if (IsGoogleInternalAccount(profile)) {
+    flow = feedback_private::FeedbackFlow::FEEDBACK_FLOW_GOOGLEINTERNAL;
+    include_bluetooth_logs = IsFromUserInteraction(source);
+  }
+#endif
+
+  api->RequestFeedbackForFlow(
+      description_template, description_placeholder_text, category_tag,
+      extra_diagnostics, page_url, flow, source == kFeedbackSourceAssistant,
+      include_bluetooth_logs, source == kFeedbackSourceKaleidoscope);
+}
+
 }  // namespace
 
+#if BUILDFLAG(IS_LACROS)
+namespace internal {
+// Requests to show Feedback ui remotely in ash via crosapi mojo call.
+void ShowFeedbackPageLacros(const GURL& page_url,
+                            FeedbackSource source,
+                            const std::string& description_template,
+                            const std::string& description_placeholder_text,
+                            const std::string& category_tag,
+                            const std::string& extra_diagnostics);
+}  // namespace internal
+#endif
+
 void ShowFeedbackPage(const Browser* browser,
                       FeedbackSource source,
                       const std::string& description_template,
@@ -97,26 +148,28 @@ void ShowFeedbackPage(const GURL& page_url,
   UMA_HISTOGRAM_ENUMERATION("Feedback.RequestSource", source,
                             kFeedbackSourceCount);
 
-  extensions::FeedbackPrivateAPI* api =
-      extensions::FeedbackPrivateAPI::GetFactoryInstance()->Get(profile);
-
-  feedback_private::FeedbackFlow flow =
-      source == kFeedbackSourceSadTabPage
-          ? feedback_private::FeedbackFlow::FEEDBACK_FLOW_SADTABCRASH
-          : feedback_private::FeedbackFlow::FEEDBACK_FLOW_REGULAR;
-
-  bool include_bluetooth_logs = false;
-#if defined(OS_CHROMEOS)
-  if (IsGoogleInternalAccount(profile)) {
-    flow = feedback_private::FeedbackFlow::FEEDBACK_FLOW_GOOGLEINTERNAL;
-    include_bluetooth_logs = IsFromUserInteraction(source);
+#if BUILDFLAG(IS_LACROS)
+  if (chromeos::LacrosChromeServiceImpl::Get()->IsFeedbackAvailable()) {
+    // Send request to ash via crosapi mojo to show Feedback ui from ash.
+    internal::ShowFeedbackPageLacros(page_url, source, description_template,
+                                     description_placeholder_text, category_tag,
+                                     extra_diagnostics);
+  } else {
+    // If ash version is too old, which does not support Feedback crosapi,
+    // invoke the Feedback ui from feedback extension in lacros and send
+    // a simple lacros feedback report for backward compatibility support.
+    // TODO(http://crbug.com/1132106): Remove this code after M87 beta
+    // when Feedback should be available in crosapi for all ash versions.
+    RequestFeedbackFlow(page_url, profile, source, description_template,
+                        description_placeholder_text, category_tag,
+                        extra_diagnostics);
   }
-#endif
-
-  api->RequestFeedbackForFlow(
-      description_template, description_placeholder_text, category_tag,
-      extra_diagnostics, page_url, flow, source == kFeedbackSourceAssistant,
-      include_bluetooth_logs, source == kFeedbackSourceKaleidoscope);
+#else
+  // Show feedback dialog using feedback extension API.
+  RequestFeedbackFlow(page_url, profile, source, description_template,
+                      description_placeholder_text, category_tag,
+                      extra_diagnostics);
+#endif  //  #if BUILDFLAG(IS_LACROS)
 }
 
 }  // namespace chrome

chrome/browser/feedback/show_feedback_page_lacros.cc

+// Copyright 2020 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/ui/chrome_pages.h"
+#include "chromeos/crosapi/mojom/feedback.mojom.h"
+#include "chromeos/lacros/lacros_chrome_service_impl.h"
+
+namespace chrome {
+namespace internal {
+
+namespace {
+
+crosapi::mojom::LacrosFeedbackSource ToMojoLacrosFeedbackSource(
+    FeedbackSource source) {
+  switch (source) {
+    case kFeedbackSourceBrowserCommand:
+      return crosapi::mojom::LacrosFeedbackSource::kLacrosBrowserCommand;
+    case kFeedbackSourceMdSettingsAboutPage:
+      return crosapi::mojom::LacrosFeedbackSource::kLacrosSettingsAboutPage;
+    default:
+      NOTREACHED() << "ShowFeedbackPage is called by unknown Lacros source";
+      return crosapi::mojom::LacrosFeedbackSource::kLacrosBrowserCommand;
+  }
+}
+
+crosapi::mojom::FeedbackInfoPtr ToMojoFeedbackInfo(
+    const GURL& page_url,
+    FeedbackSource source,
+    const std::string& description_template,
+    const std::string& description_placeholder_text,
+    const std::string& category_tag,
+    const std::string& extra_diagnostics) {
+  auto mojo_feedback = crosapi::mojom::FeedbackInfo::New();
+  mojo_feedback->page_url = page_url;
+  mojo_feedback->source = ToMojoLacrosFeedbackSource(source);
+  mojo_feedback->description_template = description_template;
+  mojo_feedback->description_placeholder_text = description_placeholder_text;
+  mojo_feedback->category_tag = category_tag;
+  mojo_feedback->extra_diagnostics = extra_diagnostics;
+  return mojo_feedback;
+}
+
+}  //  namespace
+
+// Requests to show Feedback ui remotely in ash via crosapi mojo call.
+// Note: This function should only be called from show_feedback_page.cc.
+void ShowFeedbackPageLacros(const GURL& page_url,
+                            FeedbackSource source,
+                            const std::string& description_template,
+                            const std::string& description_placeholder_text,
+                            const std::string& category_tag,
+                            const std::string& extra_diagnostics) {
+  chromeos::LacrosChromeServiceImpl::Get()->feedback_remote()->ShowFeedbackPage(
+      ToMojoFeedbackInfo(page_url, source, description_template,
+                         description_placeholder_text, category_tag,
+                         extra_diagnostics));
+}
+
+}  // namespace internal
+}  // namespace chrome

chrome/browser/feedback/show_feedback_page_unittest.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/test/metrics/histogram_tester.h"
+#include "build/build_config.h"
 #include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/browser_with_test_window_test.h"
@@ -10,7 +11,13 @@
 
 using ShowFeedbackPageTest = BrowserWithTestWindowTest;
 
-TEST_F(ShowFeedbackPageTest, UserFeedbackDisallowed) {
+// TODO(crbug.com/1128855): Fix the test for Lacros build.
+#if BUILDFLAG(IS_LACROS)
+#define MAYBE_UserFeedbackDisallowed DISABLED_UserFeedbackDisallowed
+#else
+#define MAYBE_UserFeedbackDisallowed UserFeedbackDisallowed
+#endif
+TEST_F(ShowFeedbackPageTest, MAYBE_UserFeedbackDisallowed) {
   base::HistogramTester histogram_tester;
   std::string unused;
   chrome::ShowFeedbackPage(browser(), chrome::kFeedbackSourceBrowserCommand,

chrome/browser/feedback/system_logs/log_sources/chrome_internal_log_source.cc

@@ -43,10 +43,13 @@
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/chromeos/arc/arc_util.h"
 #include "chrome/browser/chromeos/arc/policy/arc_policy_bridge.h"
+#include "chrome/browser/chromeos/crosapi/browser_manager.h"
+#include "chrome/browser/chromeos/crosapi/browser_util.h"
 #include "chrome/browser/chromeos/login/demo_mode/demo_session.h"
 #include "chrome/browser/chromeos/login/login_pref_names.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/metrics/chromeos_metrics_provider.h"
+#include "chromeos/constants/chromeos_features.h"
 #include "chromeos/dbus/util/version_loader.h"
 #include "chromeos/system/statistics_provider.h"
 #endif
@@ -73,11 +76,12 @@ constexpr char kPowerApiListKey[] = "chrome.power extensions";
 constexpr char kDataReductionProxyKey[] = "data_reduction_proxy";
 constexpr char kChromeVersionTag[] = "CHROME VERSION";
 
-#if BUILDFLAG(IS_LACROS)
+#if defined(OS_CHROMEOS) || BUILDFLAG(IS_LACROS)
 constexpr char kLacrosChromeVersionPrefix[] = "Lacros ";
 #endif
 
 #if defined(OS_CHROMEOS)
+constexpr char kAshChromeVersionPrefix[] = "Ash ";
 constexpr char kArcPolicyComplianceReportKey[] =
     "CHROMEOS_ARC_POLICY_COMPLIANCE_REPORT";
 constexpr char kArcPolicyKey[] = "CHROMEOS_ARC_POLICY";
@@ -229,6 +233,34 @@ void PopulateEntriesAsync(SystemLogsResponse* response) {
 }
 #endif  // defined(OS_CHROMEOS)
 
+std::string GetChromeVersionString() {
+  // Version of the current running browser.
+  std::string browser_version = chrome::GetVersionString();
+
+// This is used by simple lacros feedback for backward compatibility.
+// TODO(http://crbug.com/1132106): Remove after M87 beta when Feedback
+// crosapi is available in all ash versions.
+#if BUILDFLAG(IS_LACROS)
+  browser_version = kLacrosChromeVersionPrefix + browser_version;
+#endif
+
+#if defined(OS_CHROMEOS)
+  // If lacros-chrome is allowed & supported, and launched before, which
+  // is indicated by |lacros_version| in BrowserManager being set to non-empty
+  // string during lacros startup, attach its version in the chrome
+  // version string.
+  if (chromeos::features::IsLacrosSupportEnabled() &&
+      crosapi::browser_util::IsLacrosAllowed() &&
+      !crosapi::BrowserManager::Get()->lacros_version().empty()) {
+    std::string lacros_version =
+        crosapi::BrowserManager::Get()->lacros_version();
+    return kLacrosChromeVersionPrefix + lacros_version + ", " +
+           kAshChromeVersionPrefix + browser_version;
+  }
+#endif  // defined(OS_CHROMEOS)
+  return browser_version;
+}
+
 #if defined(OS_WIN) && BUILDFLAG(GOOGLE_CHROME_BRANDING)
 // Returns true if the path identified by |key| with the PathService is a parent
 // or ancestor of |child|.
@@ -292,15 +324,7 @@ void ChromeInternalLogSource::Fetch(SysLogsSourceCallback callback) {
   DCHECK(!callback.is_null());
 
   auto response = std::make_unique<SystemLogsResponse>();
-
-#if BUILDFLAG(IS_LACROS)
-  // Add a Lacros prefix string in the chrome version string to
-  // differentiate lacros chrome vs ash chrome in the feedback report.
-  response->emplace(kChromeVersionTag,
-                    kLacrosChromeVersionPrefix + chrome::GetVersionString());
-#else
-  response->emplace(kChromeVersionTag, chrome::GetVersionString());
-#endif
+  response->emplace(kChromeVersionTag, GetChromeVersionString());
 
 #if defined(OS_CHROMEOS)
   response->emplace(kChromeEnrollmentTag, GetEnrollmentStatusString());

chrome/browser/lacros/lacros_chrome_service_delegate_impl.cc

@@ -8,6 +8,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/browser_commands.h"
+#include "chrome/common/channel_info.h"
 
 LacrosChromeServiceDelegateImpl::LacrosChromeServiceDelegateImpl() = default;
 
@@ -19,3 +20,7 @@ void LacrosChromeServiceDelegateImpl::NewWindow() {
   DCHECK(profile) << "No last used profile is found.";
   chrome::NewEmptyWindow(profile);
 }
+
+std::string LacrosChromeServiceDelegateImpl::GetChromeVersion() {
+  return chrome::GetVersionString();
+}

chrome/browser/lacros/lacros_chrome_service_delegate_impl.h

@@ -20,6 +20,7 @@ class LacrosChromeServiceDelegateImpl
 
   // chromeos::LacrosChromeServiceDelegate:
   void NewWindow() override;
+  std::string GetChromeVersion() override;
 };
 
 #endif  // CHROME_BROWSER_LACROS_LACROS_CHROME_SERVICE_DELEGATE_IMPL_H_

chrome/browser/ui/webui/tab_search/tab_search_page_handler_unittest.cc

@@ -7,6 +7,7 @@
 #include "base/test/bind_test_util.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/timer/mock_timer.h"
+#include "build/build_config.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_list.h"
@@ -342,7 +343,13 @@ TEST_F(TabSearchPageHandlerTest, CloseTab) {
   ASSERT_EQ(1, browser2()->tab_strip_model()->count());
 }
 
-TEST_F(TabSearchPageHandlerTest, ShowFeedbackPage) {
+// TODO(crbug.com/1128855): Fix the test for Lacros build.
+#if BUILDFLAG(IS_LACROS)
+#define MAYBE_ShowFeedbackPage DISABLED_ShowFeedbackPage
+#else
+#define MAYBE_ShowFeedbackPage ShowFeedbackPage
+#endif
+TEST_F(TabSearchPageHandlerTest, MAYBE_ShowFeedbackPage) {
   base::HistogramTester histogram_tester;
   handler()->ShowFeedbackPage();
   histogram_tester.ExpectTotalCount("Feedback.RequestSource", 1);

chromeos/crosapi/mojom/BUILD.gn

@@ -9,6 +9,7 @@ mojom("mojom") {
     "account_manager.mojom",
     "bitmap.mojom",
     "crosapi.mojom",
+    "feedback.mojom",
     "keystore_service.mojom",
     "message_center.mojom",
     "notification.mojom",

chromeos/crosapi/mojom/crosapi.mojom

@@ -4,12 +4,27 @@
 
 module crosapi.mojom;
 
+import "chromeos/crosapi/mojom/feedback.mojom";
 import "chromeos/crosapi/mojom/keystore_service.mojom";
 import "chromeos/crosapi/mojom/message_center.mojom";
 import "chromeos/crosapi/mojom/screen_manager.mojom";
 import "chromeos/crosapi/mojom/select_file.mojom";
 import "services/device/public/mojom/hid.mojom";
 
+// LacrosInfo is a set of parameters passed to ash from lacros-chrome
+// upon lacros startup, which contains the lacros information such as version,
+// etc.
+[Stable]
+struct LacrosInfo {
+  // Version of lacros-chrome displayed to user in feedback report, etc.
+  // It includes both browser version and channel in the format of:
+  // {browser version} {channel}
+  // For example, "87.0.0.1 dev", "86.0.4240.38 beta".
+  string lacros_version@0;
+
+  // TODO(crbug.com/1119925): Add more parameters later.
+};
+
 // AshChromeService defines the APIs that live in ash-chrome and are
 // accessed from lacros-chrome. When adding a major new API please note the
 // milestone when you added it, to help us reason about compatibility between
@@ -36,6 +51,15 @@ interface AshChromeService {
   // Binds the HidManager interface for support HID devices.
   // Added in M87.
   BindHidManager@4(pending_receiver<device.mojom.HidManager> receiver);
+
+  // Binds the Feedback interface for showing feedback UI.
+  // Added in M87.
+  [MinVersion=3] BindFeedback@5(pending_receiver<Feedback> receiver);
+
+  // Passes generic lacros information such as lacros version, etc into ash
+  // in |lacros_info| during startup.
+  // Added in M87.
+  [MinVersion=3] OnLacrosStartup@6(LacrosInfo lacros_info);
 };
 
 // LacrosInitParams is a set of parameters for initialization of lacros-chrome,

chromeos/crosapi/mojom/feedback.mojom

+// Copyright 2020 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.
+
+module crosapi.mojom;
+
+import "url/mojom/url.mojom";
+
+[Stable, Extensible]
+enum LacrosFeedbackSource {
+  kLacrosBrowserCommand = 0,
+  kLacrosSettingsAboutPage = 1,
+};
+
+[Stable]
+struct FeedbackInfo {
+  // Url of the web page of lacros chrome from which the user reports an issue.
+  url.mojom.Url page_url@0;
+
+  // Source from which the user reports an issue.
+  LacrosFeedbackSource source@1;
+
+  // Template text for feedback description.
+  string description_template@2;
+
+  // Placehold text of feedback description.
+  string description_placeholder_text@3;
+
+  // Category tag of feedback report.
+  string category_tag@4;
+
+  // Extra diagnostics information.
+  // For example, "Failed to connect to wifi network.".
+  string extra_diagnostics@5;
+};
+
+// This interface is implemented by ash-chrome. It allows lacros-chrome to
+// request ash-chrome to display Feedback ui.
+[Stable]
+interface Feedback {
+  // Displays the Feedback ui.
+  ShowFeedbackPage@0(FeedbackInfo feedback_info);
+};

chromeos/lacros/lacros_chrome_service_delegate.h

@@ -5,6 +5,8 @@
 #ifndef CHROMEOS_LACROS_LACROS_CHROME_SERVICE_DELEGATE_H_
 #define CHROMEOS_LACROS_LACROS_CHROME_SERVICE_DELEGATE_H_
 
+#include <string>
+
 namespace chromeos {
 
 // Interface to inject Chrome dependent behavior into LacrosChromeServiceImpl
@@ -15,6 +17,12 @@ class LacrosChromeServiceDelegate {
 
   // Opens a new browser window.
   virtual void NewWindow() = 0;
+
+  // Returns version of lacros-chrome displayed to user in feedback report, etc.
+  // It includes both browser version and channel in the format of:
+  // {browser version} {channel}
+  // For example, "87.0.0.1 dev", "86.0.4240.38 beta".
+  virtual std::string GetChromeVersion() = 0;
 };
 
 }  // namespace chromeos

chromeos/lacros/lacros_chrome_service_impl.cc

@@ -24,6 +24,12 @@ bool g_disable_all_crosapi_for_tests = false;
 // testing.
 std::atomic<LacrosChromeServiceImpl*> g_instance = {nullptr};
 
+crosapi::mojom::LacrosInfoPtr ToMojo(const std::string& lacros_version) {
+  auto mojo_lacros_info = crosapi::mojom::LacrosInfo::New();
+  mojo_lacros_info->lacros_version = lacros_version;
+  return mojo_lacros_info;
+}
+
 }  // namespace
 
 // This class that holds all state that is affine to a single, never-blocking
@@ -127,6 +133,17 @@ class LacrosChromeServiceNeverBlockingState
     ash_chrome_service_->BindKeystoreService(std::move(pending_receiver));
   }
 
+  void BindFeedbackReceiver(
+      mojo::PendingReceiver<crosapi::mojom::Feedback> pending_receiver) {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    ash_chrome_service_->BindFeedback(std::move(pending_receiver));
+  }
+
+  void OnLacrosStartup(crosapi::mojom::LacrosInfoPtr lacros_info) {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    ash_chrome_service_->OnLacrosStartup(std::move(lacros_info));
+  }
+
   base::WeakPtr<LacrosChromeServiceNeverBlockingState> GetWeakPtr() {
     return weak_factory_.GetWeakPtr();
   }
@@ -258,6 +275,23 @@ void LacrosChromeServiceImpl::BindReceiver(
             &LacrosChromeServiceNeverBlockingState::BindHidManagerReceiver,
             weak_sequenced_state_, std::move(hid_manager_pending_receiver)));
   }
+
+  if (IsFeedbackAvailable()) {
+    never_blocking_sequence_->PostTask(
+        FROM_HERE,
+        base::BindOnce(
+            &LacrosChromeServiceNeverBlockingState::BindFeedbackReceiver,
+            weak_sequenced_state_,
+            feedback_remote_.BindNewPipeAndPassReceiver()));
+  }
+
+  if (IsOnLacrosStartupAvailable()) {
+    never_blocking_sequence_->PostTask(
+        FROM_HERE,
+        base::BindOnce(&LacrosChromeServiceNeverBlockingState::OnLacrosStartup,
+                       weak_sequenced_state_,
+                       ToMojo(delegate_->GetChromeVersion())));
+  }
 }
 
 void LacrosChromeServiceImpl::DisableCrosapiForTests() {
@@ -284,6 +318,14 @@ bool LacrosChromeServiceImpl::IsScreenManagerAvailable() {
   return AshChromeServiceVersion() >= 0;
 }
 
+bool LacrosChromeServiceImpl::IsFeedbackAvailable() {
+  return AshChromeServiceVersion() >= 3;
+}
+
+bool LacrosChromeServiceImpl::IsOnLacrosStartupAvailable() {
+  return AshChromeServiceVersion() >= 3;
+}
+
 void LacrosChromeServiceImpl::BindScreenManagerReceiver(
     mojo::PendingReceiver<crosapi::mojom::ScreenManager> pending_receiver) {
   DCHECK(IsScreenManagerAvailable());

chromeos/lacros/lacros_chrome_service_impl.h

@@ -13,6 +13,7 @@
 #include "base/sequence_checker.h"
 #include "base/sequenced_task_runner.h"
 #include "chromeos/crosapi/mojom/crosapi.mojom.h"
+#include "chromeos/crosapi/mojom/feedback.mojom.h"
 #include "chromeos/crosapi/mojom/keystore_service.mojom.h"
 #include "chromeos/crosapi/mojom/message_center.mojom.h"
 #include "chromeos/crosapi/mojom/screen_manager.mojom.h"
@@ -121,6 +122,16 @@ class COMPONENT_EXPORT(CHROMEOS_LACROS) LacrosChromeServiceImpl {
     return hid_manager_remote_;
   }
 
+  // feedback_remote() can only be used when this method returns true;
+  bool IsFeedbackAvailable();
+
+  // This must be called on the affine sequence.
+  mojo::Remote<crosapi::mojom::Feedback>& feedback_remote() {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(affine_sequence_checker_);
+    DCHECK(IsFeedbackAvailable());
+    return feedback_remote_;
+  }
+
   // --------------------------------------------------------------------------
   // Some clients will want to use mojo::Remotes on arbitrary sequences (e.g.
   // background threads). The following methods allow the client to construct a
@@ -135,6 +146,10 @@ class COMPONENT_EXPORT(CHROMEOS_LACROS) LacrosChromeServiceImpl {
   void BindScreenManagerReceiver(
       mojo::PendingReceiver<crosapi::mojom::ScreenManager> pending_receiver);
 
+  // OnLacrosStartup method of AshChromeService crosapi can only be called
+  // if this method returns true.
+  bool IsOnLacrosStartupAvailable();
+
   const crosapi::mojom::LacrosInitParams* init_params() const {
     return init_params_.get();
   }
@@ -165,6 +180,7 @@ class COMPONENT_EXPORT(CHROMEOS_LACROS) LacrosChromeServiceImpl {
   mojo::Remote<crosapi::mojom::MessageCenter> message_center_remote_;
   mojo::Remote<crosapi::mojom::SelectFile> select_file_remote_;
   mojo::Remote<device::mojom::HidManager> hid_manager_remote_;
+  mojo::Remote<crosapi::mojom::Feedback> feedback_remote_;
 
   // This member allows lacros-chrome to use the KeystoreService interface. This
   // member is affine to the affine sequence. It is initialized in the