Move more classes under the feedback module.

One major refactoring in this CL is removing FeedbackCollector's
dependency on ScreenshotTask, because ScreenshotTask is not
modularizable at the moment. Replace the takeScreenshot parameter in
the init function with a ScreenshotSource object(the interface of
ScreenshotTask), so FeedbackCollector doesn't need to construct a
ScreenshotTask itself.

Some outstanding blockers for further modularization:
* DataReductionProxyFeedbackSource, blocked on
DataReductionProxySettings.
* HelpAndFeedbackLauncherImpl, blocked on AppHooks.
* ScreenshotTask, blocked on ChromeActivity.
* connectivity_checker.cc, blocked on circular dependency of
chrome/browser/profiles.

Bug: 1117343
Change-Id: I93dd06b4146ad57d53c0cdc082d8ff0c9a32cdc0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2478108Reviewed-by: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: Natalie Chouinard <chouinard@chromium.org>
Commit-Queue: Xinghui Lu <xinghuilu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821820}

chrome/android/BUILD.gn

@@ -3215,9 +3215,7 @@ generate_jni("chrome_jni_headers") {
     "java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesSite.java",
     "java/src/org/chromium/chrome/browser/feature_engagement/TrackerFactory.java",
     "java/src/org/chromium/chrome/browser/feedback/ConnectivityChecker.java",
-    "java/src/org/chromium/chrome/browser/feedback/ProcessIdFeedbackSource.java",
     "java/src/org/chromium/chrome/browser/feedback/ScreenshotTask.java",
-    "java/src/org/chromium/chrome/browser/feedback/SystemInfoFeedbackSource.java",
     "java/src/org/chromium/chrome/browser/firstrun/FirstRunUtils.java",
     "java/src/org/chromium/chrome/browser/flags/ChromeSessionState.java",
     "java/src/org/chromium/chrome/browser/gesturenav/OverscrollSceneLayer.java",

chrome/android/chrome_java_sources.gni

@@ -692,28 +692,14 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/feature_engagement/ScreenshotMonitorDelegate.java",
   "java/src/org/chromium/chrome/browser/feature_engagement/ScreenshotTabObserver.java",
   "java/src/org/chromium/chrome/browser/feature_engagement/TrackerFactory.java",
-  "java/src/org/chromium/chrome/browser/feedback/AsyncFeedbackSourceAdapter.java",
   "java/src/org/chromium/chrome/browser/feedback/ChromeFeedbackCollector.java",
   "java/src/org/chromium/chrome/browser/feedback/ConnectivityChecker.java",
   "java/src/org/chromium/chrome/browser/feedback/ConnectivityFeedbackSource.java",
   "java/src/org/chromium/chrome/browser/feedback/ConnectivityTask.java",
   "java/src/org/chromium/chrome/browser/feedback/DataReductionProxyFeedbackSource.java",
   "java/src/org/chromium/chrome/browser/feedback/FeedFeedbackCollector.java",
-  "java/src/org/chromium/chrome/browser/feedback/FeedbackCollector.java",
-  "java/src/org/chromium/chrome/browser/feedback/FeedbackContextFeedbackSource.java",
-  "java/src/org/chromium/chrome/browser/feedback/FeedbackReporter.java",
   "java/src/org/chromium/chrome/browser/feedback/HelpAndFeedbackLauncherImpl.java",
-  "java/src/org/chromium/chrome/browser/feedback/HistogramFeedbackSource.java",
-  "java/src/org/chromium/chrome/browser/feedback/IMEFeedbackSource.java",
-  "java/src/org/chromium/chrome/browser/feedback/InterestFeedFeedbackSource.java",
-  "java/src/org/chromium/chrome/browser/feedback/LowEndDeviceFeedbackSource.java",
-  "java/src/org/chromium/chrome/browser/feedback/PermissionFeedbackSource.java",
-  "java/src/org/chromium/chrome/browser/feedback/ProcessIdFeedbackSource.java",
   "java/src/org/chromium/chrome/browser/feedback/ScreenshotTask.java",
-  "java/src/org/chromium/chrome/browser/feedback/StaticScreenshotSource.java",
-  "java/src/org/chromium/chrome/browser/feedback/SystemInfoFeedbackSource.java",
-  "java/src/org/chromium/chrome/browser/feedback/UrlFeedbackSource.java",
-  "java/src/org/chromium/chrome/browser/feedback/VariationsFeedbackSource.java",
   "java/src/org/chromium/chrome/browser/findinpage/FindToolbar.java",
   "java/src/org/chromium/chrome/browser/findinpage/FindToolbarManager.java",
   "java/src/org/chromium/chrome/browser/findinpage/FindToolbarObserver.java",

chrome/android/java/src/org/chromium/chrome/browser/childaccounts/ChildAccountFeedbackReporter.java

@@ -11,6 +11,7 @@ import org.chromium.base.annotations.CalledByNative;
 import org.chromium.chrome.browser.AppHooks;
 import org.chromium.chrome.browser.feedback.ChromeFeedbackCollector;
 import org.chromium.chrome.browser.feedback.FeedbackReporter;
+import org.chromium.chrome.browser.feedback.ScreenshotTask;
 import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.ui.base.WindowAndroid;
 
@@ -28,7 +29,7 @@ public final class ChildAccountFeedbackReporter {
         }
 
         new ChromeFeedbackCollector(activity, null /* categoryTag */, description,
-                true /* takeScreenshot */,
+                new ScreenshotTask(activity),
                 new ChromeFeedbackCollector.InitParams(profile, url, null),
                 collector -> { sFeedbackReporter.reportFeedback(collector); });
     }

chrome/android/java/src/org/chromium/chrome/browser/feedback/ChromeFeedbackCollector.java

@@ -35,10 +35,10 @@ public class ChromeFeedbackCollector
     }
 
     public ChromeFeedbackCollector(Activity activity, @Nullable String categoryTag,
-            @Nullable String description, boolean takeScreenshot, InitParams initParams,
-            Callback<FeedbackCollector> callback) {
+            @Nullable String description, @Nullable ScreenshotSource screenshotSource,
+            InitParams initParams, Callback<FeedbackCollector> callback) {
         super(categoryTag, description, callback);
-        init(activity, takeScreenshot, initParams);
+        init(activity, screenshotSource, initParams);
     }
 
     @VisibleForTesting

chrome/android/java/src/org/chromium/chrome/browser/feedback/FeedFeedbackCollector.java

@@ -36,11 +36,12 @@ public class FeedFeedbackCollector
     }
 
     public FeedFeedbackCollector(Activity activity, @Nullable String categoryTag,
-            @Nullable String description, @Nullable String feedbackContext, boolean takeScreenshot,
-            InitParams initParams, Callback<FeedbackCollector> callback) {
+            @Nullable String description, @Nullable String feedbackContext,
+            @Nullable ScreenshotSource screenshotSource, InitParams initParams,
+            Callback<FeedbackCollector> callback) {
         super(categoryTag, description, callback);
 
-        init(activity, takeScreenshot, initParams);
+        init(activity, screenshotSource, initParams);
     }
 
     @VisibleForTesting

chrome/android/java/src/org/chromium/chrome/browser/feedback/HelpAndFeedbackLauncherImpl.java

@@ -89,7 +89,7 @@ public class HelpAndFeedbackLauncherImpl implements HelpAndFeedbackLauncher {
             @Nullable String url) {
         RecordUserAction.record("MobileHelpAndFeedback");
         new ChromeFeedbackCollector(activity, null /* categoryTag */, null /* description */,
-                true /* takeScreenshot */,
+                new ScreenshotTask(activity),
                 new ChromeFeedbackCollector.InitParams(profile, url, helpContext),
                 collector -> show(activity, helpContext, collector));
     }
@@ -107,7 +107,7 @@ public class HelpAndFeedbackLauncherImpl implements HelpAndFeedbackLauncher {
     public void showFeedback(final Activity activity, Profile profile, @Nullable String url,
             @Nullable final String categoryTag) {
         new ChromeFeedbackCollector(activity, categoryTag, null /* description */,
-                true /* takeScreenshot */,
+                new ScreenshotTask(activity),
                 new ChromeFeedbackCollector.InitParams(profile, url, null),
                 collector -> showFeedback(activity, collector));
     }
@@ -127,7 +127,7 @@ public class HelpAndFeedbackLauncherImpl implements HelpAndFeedbackLauncher {
             @Nullable final String categoryTag, @Nullable final Map<String, String> feedContext,
             @Nullable final String feedbackContext) {
         new FeedFeedbackCollector(activity, categoryTag, null /* description */, feedbackContext,
-                true /* takeScreenshot */,
+                new ScreenshotTask(activity),
                 new FeedFeedbackCollector.InitParams(profile, url, feedContext),
                 collector -> showFeedback(activity, collector));
     }

chrome/android/java/src/org/chromium/chrome/browser/feedback/ScreenshotTask.java

@@ -29,7 +29,7 @@ import org.chromium.ui.base.WindowAndroid;
  * A utility class to take a feedback-formatted screenshot of an {@link Activity}.
  */
 @JNINamespace("chrome::android")
-final class ScreenshotTask implements ScreenshotSource {
+public final class ScreenshotTask implements ScreenshotSource {
     /**
      * Maximum dimension for the screenshot to be sent to the feedback handler.  This size
      * ensures the size of bitmap < 1MB, which is a requirement of the handler.

chrome/android/junit/src/org/chromium/chrome/browser/feedback/FeedFeedbackCollectorTest.java

@@ -86,9 +86,9 @@ public class FeedFeedbackCollectorTest {
     private static class EmptyFeedFeedbackCollector extends FeedFeedbackCollector {
         EmptyFeedFeedbackCollector(Activity activity, Profile profile, @Nullable String url,
                 @Nullable String categoryTag, @Nullable String description,
-                @Nullable String feedbackContext, boolean takeScreenshot,
+                @Nullable String feedbackContext, @Nullable ScreenshotSource screenshotSource,
                 @Nullable Map<String, String> feedContext, Callback<FeedbackCollector> callback) {
-            super(activity, categoryTag, description, feedbackContext, takeScreenshot,
+            super(activity, categoryTag, description, feedbackContext, screenshotSource,
                     new FeedFeedbackCollector.InitParams(profile, url, feedContext), callback);
         }
 
@@ -125,7 +125,7 @@ public class FeedFeedbackCollectorTest {
 
         FeedFeedbackCollector collector =
                 new EmptyFeedFeedbackCollector(mActivity, mProfile, null, CATEGORY_TAG, DESCRIPTION,
-                        null, false, feedContext, (result) -> callback.onResult(result));
+                        null, null, feedContext, (result) -> callback.onResult(result));
 
         ShadowLooper.runUiThreadTasksIncludingDelayedTasks();
         verify(callback, times(1)).onResult(collector);

chrome/browser/BUILD.gn

@@ -2612,9 +2612,7 @@ static_library("browser") {
       "android/feed/v2/refresh_task_scheduler_impl.cc",
       "android/feed/v2/refresh_task_scheduler_impl.h",
       "android/feedback/connectivity_checker.cc",
-      "android/feedback/process_id_feedback_source.cc",
       "android/feedback/screenshot_task.cc",
-      "android/feedback/system_info_feedback_source.cc",
       "android/foreign_session_helper.cc",
       "android/foreign_session_helper.h",
       "android/historical_tab_saver.cc",
@@ -3124,6 +3122,7 @@ static_library("browser") {
       "//chrome/android/modules/extra_icu/provider:native",
       "//chrome/browser/android/webapk:proto",
       "//chrome/browser/endpoint_fetcher:jni_headers",
+      "//chrome/browser/feedback/android",
       "//chrome/browser/flags:flags_android",
       "//chrome/browser/notifications/chime/android",
       "//chrome/browser/notifications/scheduler/public",

chrome/browser/feedback/android/BUILD.gn

@@ -4,19 +4,61 @@
 
 import("//build/config/android/rules.gni")
 
+generate_jni("jni_headers") {
+  sources = [
+    "java/src/org/chromium/chrome/browser/feedback/ProcessIdFeedbackSource.java",
+    "java/src/org/chromium/chrome/browser/feedback/SystemInfoFeedbackSource.java",
+  ]
+}
+
+source_set("android") {
+  sources = [
+    "process_id_feedback_source.cc",
+    "process_id_feedback_source.h",
+    "system_info_feedback_source.cc",
+  ]
+  deps = [
+    ":jni_headers",
+    "//base",
+    "//content/public/browser",
+    "//gpu/config",
+  ]
+}
+
 android_library("java") {
   sources = [
     "java/src/org/chromium/chrome/browser/feedback/AsyncFeedbackSource.java",
+    "java/src/org/chromium/chrome/browser/feedback/AsyncFeedbackSourceAdapter.java",
+    "java/src/org/chromium/chrome/browser/feedback/FeedbackCollector.java",
+    "java/src/org/chromium/chrome/browser/feedback/FeedbackContextFeedbackSource.java",
+    "java/src/org/chromium/chrome/browser/feedback/FeedbackReporter.java",
     "java/src/org/chromium/chrome/browser/feedback/FeedbackSource.java",
     "java/src/org/chromium/chrome/browser/feedback/FragmentHelpAndFeedbackLauncher.java",
     "java/src/org/chromium/chrome/browser/feedback/HelpAndFeedbackLauncher.java",
+    "java/src/org/chromium/chrome/browser/feedback/HistogramFeedbackSource.java",
+    "java/src/org/chromium/chrome/browser/feedback/IMEFeedbackSource.java",
+    "java/src/org/chromium/chrome/browser/feedback/InterestFeedFeedbackSource.java",
+    "java/src/org/chromium/chrome/browser/feedback/LowEndDeviceFeedbackSource.java",
+    "java/src/org/chromium/chrome/browser/feedback/PermissionFeedbackSource.java",
+    "java/src/org/chromium/chrome/browser/feedback/ProcessIdFeedbackSource.java",
     "java/src/org/chromium/chrome/browser/feedback/ScreenshotSource.java",
+    "java/src/org/chromium/chrome/browser/feedback/StaticScreenshotSource.java",
+    "java/src/org/chromium/chrome/browser/feedback/SystemInfoFeedbackSource.java",
+    "java/src/org/chromium/chrome/browser/feedback/UrlFeedbackSource.java",
+    "java/src/org/chromium/chrome/browser/feedback/VariationsFeedbackSource.java",
   ]
   deps = [
     ":java_resources",
+    "//base:base_java",
+    "//base:jni_java",
     "//chrome/browser/profiles/android:java",
+    "//components/browser_ui/util/android:java",
+    "//components/variations/android:variations_java",
+    "//content/public/android:content_java",
+    "//net/android:net_java",
     "//third_party/android_deps:androidx_annotation_annotation_java",
   ]
+  annotation_processor_deps = [ "//base/android/jni_generator:jni_processor" ]
   resources_package = "org.chromium.chrome.browser.feedback"
 }
 

chrome/browser/feedback/android/DEPS

+include_rules = [
+  "+content/public/android/java/src/org/chromium/content_public",
+]

chrome/android/java/src/org/chromium/chrome/browser/feedback/FeedbackCollector.java → chrome/browser/feedback/android/java/src/org/chromium/chrome/browser/feedback/FeedbackCollector.java

@@ -53,7 +53,8 @@ public abstract class FeedbackCollector<T> implements Runnable {
     }
 
     // Subclasses must invoke init() at construction time.
-    protected void init(Activity activity, boolean takeScreenshot, T initParams) {
+    protected void init(
+            Activity activity, @Nullable ScreenshotSource screenshotTask, T initParams) {
         // 1. Build all synchronous and asynchronous sources.
         mSynchronousSources = buildSynchronousFeedbackSources(initParams);
         mAsynchronousSources = buildAsynchronousFeedbackSources(initParams);
@@ -63,8 +64,8 @@ public abstract class FeedbackCollector<T> implements Runnable {
             assert !(source instanceof AsyncFeedbackSource);
         }
 
-        // 2. Build the screenshot task if necessary.
-        if (takeScreenshot) mScreenshotTask = buildScreenshotSource(activity);
+        // 2. Set |mScreenshotTask| if not null.
+        if (screenshotTask != null) mScreenshotTask = screenshotTask;
 
         // 3. Start all asynchronous sources and the screenshot task.
         CollectionUtil.forEach(mAsynchronousSources, source -> source.start(this));
@@ -85,11 +86,6 @@ public abstract class FeedbackCollector<T> implements Runnable {
     @NonNull
     protected abstract List<AsyncFeedbackSource> buildAsynchronousFeedbackSources(T initParams);
 
-    @VisibleForTesting
-    protected ScreenshotSource buildScreenshotSource(Activity activity) {
-        return new ScreenshotTask(activity);
-    }
-
     /** @return The category tag for this feedback report. */
     public String getCategoryTag() {
         return mCategoryTag;

chrome/android/java/src/org/chromium/chrome/browser/feedback/FeedbackReporter.java → chrome/browser/feedback/android/java/src/org/chromium/chrome/browser/feedback/FeedbackReporter.java

@@ -13,8 +13,5 @@ public interface FeedbackReporter {
      *
      * @param collector the {@link FeedbackCollector} to use for extra data.
      */
-    // clang-format off
-    // TODO(crbug.com/781015): Clang isn't formatting this correctly.
     default void reportFeedback(FeedbackCollector collector) {}
-        // clang-format on
 }

chrome/browser/android/feedback/process_id_feedback_source.cc → chrome/browser/feedback/android/process_id_feedback_source.cc

@@ -2,9 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/android/feedback/process_id_feedback_source.h"
+#include "chrome/browser/feedback/android/process_id_feedback_source.h"
 
-#include "chrome/android/chrome_jni_headers/ProcessIdFeedbackSource_jni.h"
+#include "chrome/browser/feedback/android/jni_headers/ProcessIdFeedbackSource_jni.h"
 #include "content/public/browser/browser_task_traits.h"
 
 #include "base/android/jni_array.h"

chrome/browser/android/feedback/process_id_feedback_source.h → chrome/browser/feedback/android/process_id_feedback_source.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_ANDROID_FEEDBACK_PROCESS_ID_FEEDBACK_SOURCE_H_
-#define CHROME_BROWSER_ANDROID_FEEDBACK_PROCESS_ID_FEEDBACK_SOURCE_H_
+#ifndef CHROME_BROWSER_FEEDBACK_ANDROID_PROCESS_ID_FEEDBACK_SOURCE_H_
+#define CHROME_BROWSER_FEEDBACK_ANDROID_PROCESS_ID_FEEDBACK_SOURCE_H_
 
 #include <map>
 
@@ -45,4 +45,4 @@ class ProcessIdFeedbackSource
 }  // namespace android
 }  // namespace chrome
 
-#endif  // CHROME_BROWSER_ANDROID_FEEDBACK_PROCESS_ID_FEEDBACK_SOURCE_H_
+#endif  // CHROME_BROWSER_FEEDBACK_ANDROID_PROCESS_ID_FEEDBACK_SOURCE_H_

chrome/browser/android/feedback/system_info_feedback_source.cc → chrome/browser/feedback/android/system_info_feedback_source.cc

@@ -6,7 +6,7 @@
 #include "base/android/jni_string.h"
 #include "base/android/scoped_java_ref.h"
 #include "base/system/sys_info.h"
-#include "chrome/android/chrome_jni_headers/SystemInfoFeedbackSource_jni.h"
+#include "chrome/browser/feedback/android/jni_headers/SystemInfoFeedbackSource_jni.h"
 #include "content/public/browser/gpu_data_manager.h"
 #include "gpu/config/gpu_info.h"