feedback: remove the obsolete Feedback extension app - part 3

The extension app has been migrated to WebUI. - Remove obsolete code in feedbackPrivateAPI. - Remove kFeedbackExtensionId. Bug: b:219968020 Change-Id: I7c4b2fd3394035ee39c6597d935f8ad53703bb3d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3605805 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Xiangdong Kong <xiangdongkong@google.com> Reviewed-by: Ben Wells <benwells@chromium.org> Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Cr-Commit-Position: refs/heads/main@{#1002868}
chromium · May 12, 2022 · 9a4464d · 9a4464d
1 parent c396e9c
commit 9a4464d
Show file tree

Hide file tree

Showing 19 changed files with 8 additions and 206 deletions.
diff --git a/chrome/browser/extensions/api/feedback_private/chrome_feedback_private_delegate.cc b/chrome/browser/extensions/api/feedback_private/chrome_feedback_private_delegate.cc
@@ -221,14 +221,6 @@ void ChromeFeedbackPrivateDelegate::FetchExtraLogs(
   }
 }
 
-void ChromeFeedbackPrivateDelegate::UnloadFeedbackExtension(
-    content::BrowserContext* context) const {
-  extensions::ExtensionSystem::Get(context)
-      ->extension_service()
-      ->component_loader()
-      ->Remove(extension_misc::kFeedbackExtensionId);
-}
-
 api::feedback_private::LandingPageType
 ChromeFeedbackPrivateDelegate::GetLandingPageType(
     const feedback::FeedbackData& feedback_data) const {

diff --git a/chrome/browser/extensions/api/feedback_private/chrome_feedback_private_delegate.h b/chrome/browser/extensions/api/feedback_private/chrome_feedback_private_delegate.h
@@ -34,7 +34,6 @@ class ChromeFeedbackPrivateDelegate : public FeedbackPrivateDelegate {
       api::feedback_private::LogSource source_type) const override;
   void FetchExtraLogs(scoped_refptr<feedback::FeedbackData> feedback_data,
                       FetchExtraLogsCallback callback) const override;
-  void UnloadFeedbackExtension(content::BrowserContext* context) const override;
   api::feedback_private::LandingPageType GetLandingPageType(
       const feedback::FeedbackData& feedback_data) const override;
   void GetLacrosHistograms(GetHistogramsCallback callback) override;

diff --git a/chrome/browser/extensions/extension_keeplist_ash.cc b/chrome/browser/extensions/extension_keeplist_ash.cc
@@ -44,9 +44,9 @@ bool ExtensionRunsInAsh(const std::string& extension_id) {
 
 bool ExtensionAppRunsInAsh(const std::string& app_id) {
   static base::NoDestructor<std::set<base::StringPiece>> keep_list(
-      {file_manager::kAudioPlayerAppId, extension_misc::kFeedbackExtensionId,
-       extension_misc::kFilesManagerAppId, extension_misc::kGoogleKeepAppId,
-       extension_misc::kCalculatorAppId, extension_misc::kTextEditorAppId,
+      {file_manager::kAudioPlayerAppId, extension_misc::kFilesManagerAppId,
+       extension_misc::kGoogleKeepAppId, extension_misc::kCalculatorAppId,
+       extension_misc::kTextEditorAppId,
        extension_misc::kInAppPaymentsSupportAppId,
        extension_misc::kWallpaperManagerId, arc::kPlayStoreAppId,
        extension_misc::kIdentityApiUiAppId, extension_misc::kGnubbyAppId});

diff --git a/chrome/browser/resources/feedback_webui/js/feedback.ts b/chrome/browser/resources/feedback_webui/js/feedback.ts
@@ -68,9 +68,6 @@ class FeedbackHelper {
                 'Feedback: Report for request with ID ' + ID +
                 ' will be sent later.');
           }
-          if (FLOW === chrome.feedbackPrivate.FeedbackFlow.LOGIN) {
-            chrome.feedbackPrivate.loginFeedbackComplete();
-          }
           scheduleWindowClose();
         });
   }
@@ -482,9 +479,6 @@ function sendReport(): boolean {
 function cancel(e: Event) {
   e.preventDefault();
   scheduleWindowClose();
-  if (feedbackInfo.flow === chrome.feedbackPrivate.FeedbackFlow.LOGIN) {
-    chrome.feedbackPrivate.loginFeedbackComplete();
-  }
 }
 
 // <if expr="chromeos_ash">

diff --git a/chrome/browser/ui/ash/shelf/chrome_shelf_controller_unittest.cc b/chrome/browser/ui/ash/shelf/chrome_shelf_controller_unittest.cc
@@ -5105,8 +5105,7 @@ TEST_P(ChromeShelfControllerTest, UnpinnableComponentApps) {
   InitShelfController();
 
   const char* kPinnableApp = file_manager::kFileManagerAppId;
-  const char* kNoPinApps[] = {extension_misc::kFeedbackExtensionId,
-                              ash::eche_app::kEcheAppId};
+  const char* kNoPinApps[] = {ash::eche_app::kEcheAppId};
 
   EXPECT_EQ(AppListControllerDelegate::PIN_EDITABLE,
             GetPinnableForAppID(kPinnableApp, profile()));

diff --git a/chrome/browser/ui/ash/shelf/chrome_shelf_controller_util.cc b/chrome/browser/ui/ash/shelf/chrome_shelf_controller_util.cc
@@ -107,7 +107,6 @@ AppListControllerDelegate::Pinnable GetPinnableForAppID(
   // item that does nothing.
   const char* kNoPinAppIds[] = {
       file_manager::kAudioPlayerAppId,
-      extension_misc::kFeedbackExtensionId,
       ash::eche_app::kEcheAppId,
   };
   if (base::Contains(kNoPinAppIds, app_id))

diff --git a/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc b/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
@@ -57,22 +57,6 @@
 
 using extensions::AppWindow;
 
-namespace {
-
-// The feedback dialog is modal during OOBE and login because it must stay above
-// the views login UI and the webui GAIA login dialog.
-bool IsLoginFeedbackModalDialog(const AppWindow* app_window) {
-  if (app_window->extension_id() != extension_misc::kFeedbackExtensionId)
-    return false;
-
-  using session_manager::SessionState;
-  SessionState state = session_manager::SessionManager::Get()->session_state();
-  return state == SessionState::OOBE || state == SessionState::LOGIN_PRIMARY ||
-         state == SessionState::LOGIN_SECONDARY;
-}
-
-}  // namespace
-
 ChromeNativeAppWindowViewsAuraAsh::ChromeNativeAppWindowViewsAuraAsh()
     : exclusive_access_manager_(
           std::make_unique<ExclusiveAccessManager>(this)) {
@@ -98,10 +82,6 @@ void ChromeNativeAppWindowViewsAuraAsh::InitializeWindow(
   window->SetProperty(chromeos::kImmersiveImpliedByFullscreen, false);
   // TODO(https://crbug.com/997480): Determine if all non-resizable windows
   // should have this behavior, or just the feedback app.
-  if (app_window->extension_id() == extension_misc::kFeedbackExtensionId) {
-    ash::WindowBackdrop::Get(window)->SetBackdropType(
-        ash::WindowBackdrop::BackdropType::kSemiOpaque);
-  }
   window_observation_.Observe(window);
 }
 
@@ -116,9 +96,7 @@ void ChromeNativeAppWindowViewsAuraAsh::OnBeforeWidgetInit(
   // Some windows need to be placed in special containers, for example to make
   // them visible at the login or lock screen.
   absl::optional<int> container_id;
-  if (IsLoginFeedbackModalDialog(app_window()))
-    container_id = ash::kShellWindowId_LockSystemModalContainer;
-  else if (create_params.is_ime_window)
+  if (create_params.is_ime_window)
     container_id = ash::kShellWindowId_ImeWindowParentContainer;
   else if (create_params.show_on_lock_screen)
     container_id = ash::kShellWindowId_LockActionHandlerContainer;
@@ -170,12 +148,6 @@ ChromeNativeAppWindowViewsAuraAsh::CreateNonStandardAppFrame() {
   return frame;
 }
 
-ui::ModalType ChromeNativeAppWindowViewsAuraAsh::GetModalType() const {
-  if (IsLoginFeedbackModalDialog(app_window()))
-    return ui::MODAL_TYPE_SYSTEM;
-  return ChromeNativeAppWindowViewsAura::GetModalType();
-}
-
 ui::ImageModel ChromeNativeAppWindowViewsAuraAsh::GetWindowIcon() {
   const ui::ImageModel& image = ChromeNativeAppWindowViews::GetWindowIcon();
   if (image.IsEmpty())

diff --git a/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.h b/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.h
@@ -88,7 +88,6 @@ class ChromeNativeAppWindowViewsAuraAsh
   // WidgetDelegate:
   std::unique_ptr<views::NonClientFrameView> CreateNonClientFrameView(
       views::Widget* widget) override;
-  ui::ModalType GetModalType() const override;
   ui::ImageModel GetWindowIcon() override;
 
   // NativeAppWindow:

diff --git a/extensions/browser/api/feedback_private/feedback_private_api.cc b/extensions/browser/api/feedback_private/feedback_private_api.cc
@@ -262,62 +262,6 @@ std::unique_ptr<FeedbackInfo> FeedbackPrivateAPI::CreateFeedbackInfo(
   return info;
 }
 
-void FeedbackPrivateAPI::RequestFeedbackForFlow(
-    const std::string& description_template,
-    const std::string& description_placeholder_text,
-    const std::string& category_tag,
-    const std::string& extra_diagnostics,
-    const GURL& page_url,
-    api::feedback_private::FeedbackFlow flow,
-    bool from_assistant,
-    bool include_bluetooth_logs,
-    bool show_questionnaire,
-    bool from_chrome_labs_or_kaleidoscope) {
-  if (browser_context_ && EventRouter::Get(browser_context_)) {
-    auto info = CreateFeedbackInfo(
-        description_template, description_placeholder_text, category_tag,
-        extra_diagnostics, page_url, flow, from_assistant,
-        include_bluetooth_logs, show_questionnaire,
-        from_chrome_labs_or_kaleidoscope);
-
-    auto args = feedback_private::OnFeedbackRequested::Create(*info);
-
-    auto event = std::make_unique<Event>(
-        events::FEEDBACK_PRIVATE_ON_FEEDBACK_REQUESTED,
-        feedback_private::OnFeedbackRequested::kEventName, std::move(args),
-        browser_context_);
-
-    // TODO(weidongg/754329): Using DispatchEventWithLazyListener() is a
-    // temporary fix to the bug. Investigate a better solution that applies to
-    // all scenarios.
-    EventRouter::Get(browser_context_)
-        ->DispatchEventWithLazyListener(extension_misc::kFeedbackExtensionId,
-                                        std::move(event));
-  }
-}
-
-// static
-base::OnceClosure* FeedbackPrivateGetStringsFunction::test_callback_ = nullptr;
-
-ExtensionFunction::ResponseAction FeedbackPrivateGetStringsFunction::Run() {
-  auto params = feedback_private::GetStrings::Params::Create(args());
-  EXTENSION_FUNCTION_VALIDATE(params.get());
-
-  FeedbackPrivateDelegate* feedback_private_delegate =
-      ExtensionsAPIClient::Get()->GetFeedbackPrivateDelegate();
-  DCHECK(feedback_private_delegate);
-  std::unique_ptr<base::DictionaryValue> dict =
-      feedback_private_delegate->GetStrings(
-          browser_context(),
-          params->flow == FeedbackFlow::FEEDBACK_FLOW_SADTABCRASH);
-
-  if (test_callback_ && !test_callback_->is_null())
-    std::move(*test_callback_).Run();
-
-  return RespondNow(
-      OneArgument(base::Value::FromUniquePtrValue(std::move(dict))));
-}
-
 ExtensionFunction::ResponseAction FeedbackPrivateGetUserEmailFunction::Run() {
   FeedbackPrivateDelegate* feedback_private_delegate =
       ExtensionsAPIClient::Get()->GetFeedbackPrivateDelegate();
@@ -432,14 +376,4 @@ void FeedbackPrivateSendFeedbackFunction::OnCompleted(
   }
 }
 
-ExtensionFunction::ResponseAction
-FeedbackPrivateLoginFeedbackCompleteFunction::Run() {
-#if BUILDFLAG(IS_CHROMEOS_ASH)
-  FeedbackPrivateDelegate* feedback_private_delegate =
-      ExtensionsAPIClient::Get()->GetFeedbackPrivateDelegate();
-  feedback_private_delegate->UnloadFeedbackExtension(browser_context());
-#endif
-  return RespondNow(NoArguments());
-}
-
 }  // namespace extensions
diff --git a/extensions/browser/api/feedback_private/feedback_private_api.h b/extensions/browser/api/feedback_private/feedback_private_api.h
@@ -51,17 +51,6 @@ class FeedbackPrivateAPI : public BrowserContextKeyedAPI {
       bool show_questionnaire,
       bool from_chrome_labs_or_kaleidoscope);
 
-  void RequestFeedbackForFlow(const std::string& description_template,
-                              const std::string& description_placeholder_text,
-                              const std::string& category_tag,
-                              const std::string& extra_diagnostics,
-                              const GURL& page_url,
-                              api::feedback_private::FeedbackFlow flow,
-                              bool from_assistant = false,
-                              bool include_bluetooth_logs = false,
-                              bool show_questionnaire = false,
-                              bool from_chrome_labs_or_kaleidoscope = false);
-
   // BrowserContextKeyedAPI implementation.
   static BrowserContextKeyedAPIFactory<FeedbackPrivateAPI>*
   GetFactoryInstance();
@@ -87,27 +76,6 @@ class FeedbackPrivateAPI : public BrowserContextKeyedAPI {
 #endif  // BUILDFLAG(IS_CHROMEOS_ASH)
 };
 
-// Feedback strings.
-class FeedbackPrivateGetStringsFunction : public ExtensionFunction {
- public:
-  DECLARE_EXTENSION_FUNCTION("feedbackPrivate.getStrings",
-                             FEEDBACKPRIVATE_GETSTRINGS)
-
-  // Invoke this callback when this function is called - used for testing.
-  static void set_test_callback(base::OnceClosure* callback) {
-    test_callback_ = callback;
-  }
-
- protected:
-  ~FeedbackPrivateGetStringsFunction() override {}
-
-  // ExtensionFunction:
-  ResponseAction Run() override;
-
- private:
-  static base::OnceClosure* test_callback_;
-};
-
 class FeedbackPrivateGetUserEmailFunction : public ExtensionFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("feedbackPrivate.getUserEmail",
@@ -162,16 +130,6 @@ class FeedbackPrivateSendFeedbackFunction : public ExtensionFunction {
   void OnCompleted(api::feedback_private::LandingPageType type, bool success);
 };
 
-class FeedbackPrivateLoginFeedbackCompleteFunction : public ExtensionFunction {
- public:
-  DECLARE_EXTENSION_FUNCTION("feedbackPrivate.loginFeedbackComplete",
-                             FEEDBACKPRIVATE_LOGINFEEDBACKCOMPLETE)
-
- protected:
-  ~FeedbackPrivateLoginFeedbackCompleteFunction() override {}
-  ResponseAction Run() override;
-};
-
 }  // namespace extensions
 
 #endif  // EXTENSIONS_BROWSER_API_FEEDBACK_PRIVATE_FEEDBACK_PRIVATE_API_H_
diff --git a/extensions/browser/api/feedback_private/feedback_private_delegate.h b/extensions/browser/api/feedback_private/feedback_private_delegate.h
@@ -66,11 +66,6 @@ class FeedbackPrivateDelegate {
       scoped_refptr<feedback::FeedbackData> feedback_data,
       FetchExtraLogsCallback callback) const = 0;
 
-  // Unloads the feedback extension from the current profile, should only be
-  // called when feedback is complete for the login profile.
-  virtual void UnloadFeedbackExtension(
-      content::BrowserContext* context) const = 0;
-
   // Returns the type of the landing page which is shown to the user when the
   // report is successfully sent.
   virtual api::feedback_private::LandingPageType GetLandingPageType(

diff --git a/extensions/browser/extension_function_histogram_value.h b/extensions/browser/extension_function_histogram_value.h
@@ -585,7 +585,7 @@ enum HistogramValue {
   IMAGEWRITER_WRITEFROMFILE = 524,
   IMAGEWRITER_CANCELWRITE = 525,
   IMAGEWRITER_DESTROYPARTITIONS = 526,
-  FEEDBACKPRIVATE_GETSTRINGS = 527,
+  DELETED_FEEDBACKPRIVATE_GETSTRINGS = 527,
   DELETED_LOGPRIVATE_GETHISTORICAL = 528,
   VIRTUALKEYBOARDPRIVATE_MOVECURSOR = 529,
   METRICSPRIVATE_GETVARIATIONPARAMS = 530,
@@ -1390,7 +1390,7 @@ enum HistogramValue {
   AUTOTESTPRIVATE_IMPORTCROSTINI = 1327,
   ACCESSIBILITY_PRIVATE_SETVIRTUALKEYBOARDVISIBLE = 1328,
   AUTOTESTPRIVATE_SHOWVIRTUALKEYBOARDIFENABLED = 1329,
-  FEEDBACKPRIVATE_LOGINFEEDBACKCOMPLETE = 1330,
+  DELETED_FEEDBACKPRIVATE_LOGINFEEDBACKCOMPLETE = 1330,
   FILEMANAGERPRIVATE_SEARCHFILES = 1331,
   MANAGEMENT_INSTALLREPLACEMENTWEBAPP = 1332,
   FILEMANAGERPRIVATE_GETANDROIDPICKERAPPS = 1333,

diff --git a/extensions/common/api/feedback_private.idl b/extensions/common/api/feedback_private.idl
@@ -190,7 +190,6 @@ namespace feedbackPrivate {
   callback GetSystemInformationCallback =
       void(SystemInformation[] systemInformation);
   callback SendFeedbackCallback = void(Status status, LandingPageType type);
-  callback GetStringsCallback = void(object result);
   callback ReadLogSourceCallback = void (ReadLogSourceResult result);
 
   interface Functions {
@@ -212,11 +211,6 @@ namespace feedbackPrivate {
                              optional double formOpenTime,
                              SendFeedbackCallback callback);
 
-    // Gets localized translated strings for feedback. It returns the
-    // strings as a dictionary mapping from string identifier to the
-    // translated string to use in the feedback app UI.
-    static void getStrings(FeedbackFlow flow, GetStringsCallback callback);
-
     // Reads from a log source indicated by <code>source</code>.
     // <p>If <code>incremental</code> is false:
     // <ul>
@@ -245,10 +239,6 @@ namespace feedbackPrivate {
     static void readLogSource(ReadLogSourceParams params,
                               ReadLogSourceCallback callback);
 
-    // Invoked when the extension is complete during sending feedback from the
-    // login page. This is then used to know we can unload the feedback
-    // extension from the login profile.
-    static void loginFeedbackComplete();
   };
 
   interface Events {

diff --git a/extensions/common/constants.cc b/extensions/common/constants.cc
@@ -130,7 +130,6 @@ const char kChromeVoxExtensionId[] = "mndnfokpggljbaajbnioimlmbfngpief";
 // The extension id for the web store extension.
 const char kChromeVoxExtensionId[] = "kgejglhpjiefppelpmljglcjbhoiplfn";
 #endif
-const char kFeedbackExtensionId[] = "gfdkimpbcpahaombhbimeihdjnejgicl";
 const char kPdfExtensionId[] = "mhjfbmdgcfjbbpaeojofohoefgiehjai";
 const char kQuickOfficeComponentExtensionId[] =
     "bpmcpldpdmajfigpchkicefoigmkfalc";
@@ -178,7 +177,6 @@ bool IsSystemUIApp(base::StringPiece extension_id) {
   static const char* const kApps[] = {
       // clang-format off
       kChromeVoxExtensionId,
-      kFeedbackExtensionId,
       kFilesManagerAppId,
       kHighlightsAtlasAppId,
       kHighlightsAppId,

diff --git a/extensions/common/constants.h b/extensions/common/constants.h
@@ -239,9 +239,6 @@ constexpr ExtensionIcons EXTENSION_ICON_INVALID = 0;
 // The extension id of the ChromeVox extension.
 EXTENSIONS_EXPORT extern const char kChromeVoxExtensionId[];
 
-// The extension id of the feedback component extension.
-EXTENSIONS_EXPORT extern const char kFeedbackExtensionId[];
-
 // The extension id of the PDF extension.
 EXTENSIONS_EXPORT extern const char kPdfExtensionId[];
 

diff --git a/extensions/shell/browser/api/feedback_private/shell_feedback_private_delegate.cc b/extensions/shell/browser/api/feedback_private/shell_feedback_private_delegate.cc
@@ -51,11 +51,6 @@ void ShellFeedbackPrivateDelegate::FetchExtraLogs(
   std::move(callback).Run(feedback_data);
 }
 
-void ShellFeedbackPrivateDelegate::UnloadFeedbackExtension(
-    content::BrowserContext* context) const {
-  NOTIMPLEMENTED();
-}
-
 api::feedback_private::LandingPageType
 ShellFeedbackPrivateDelegate::GetLandingPageType(
     const feedback::FeedbackData& feedback_data) const {

diff --git a/extensions/shell/browser/api/feedback_private/shell_feedback_private_delegate.h b/extensions/shell/browser/api/feedback_private/shell_feedback_private_delegate.h
@@ -34,7 +34,6 @@ class ShellFeedbackPrivateDelegate : public FeedbackPrivateDelegate {
       api::feedback_private::LogSource source_type) const override;
   void FetchExtraLogs(scoped_refptr<feedback::FeedbackData> feedback_data,
                       FetchExtraLogsCallback callback) const override;
-  void UnloadFeedbackExtension(content::BrowserContext* context) const override;
   api::feedback_private::LandingPageType GetLandingPageType(
       const feedback::FeedbackData& feedback_data) const override;
   void GetLacrosHistograms(GetHistogramsCallback callback) override;