Move ChromecastConfigAndroid implementation to internal.

Remove send usage report toggle from cast setting menu.
Make the crash report upload directly decided by internal code.
Use timer to control the periodic crash upload tasks.

Bug:79208340

Test:change the frequency of uploading to 5 seconds
     switch the toggle and check whether the update function works
     and the status of upload function.
     view the remove effect of cast setting menu.

Merge-With: eureka-internal/175156

Change-Id: Ia49c10edb3b2b65fd62bf5734d0ca06376a60d09
Reviewed-on: https://chromium-review.googlesource.com/1114227
Commit-Queue: Ziyang Cheng <ziyangch@chromium.org>
Reviewed-by: Simeon Anfinrud <sanfin@chromium.org>
Reviewed-by: Luke Halliwell <halliwell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585550}

chromecast/app/android/crash_handler.cc

@@ -14,6 +14,7 @@
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
 #include "chromecast/app/android/cast_crash_reporter_client_android.h"
+#include "chromecast/base/chromecast_config_android.h"
 #include "chromecast/base/version.h"
 #include "components/crash/content/app/breakpad_linux.h"
 #include "components/crash/content/app/crash_reporter_client.h"
@@ -77,25 +78,18 @@ CrashHandler::~CrashHandler() {
 
 void CrashHandler::Initialize() {
   if (process_type_.empty()) {
-    InitializeUploader();
     breakpad::InitCrashReporter(process_type_);
     return;
   }
-
   if (process_type_ != service_manager::switches::kZygoteProcess) {
     breakpad::InitNonBrowserCrashReporterForAndroid(process_type_);
   }
 }
 
-void CrashHandler::InitializeUploader() {
-  CrashHandler::UploadDumps(crash_dump_path_, "", "", true);
-}
-
 // static
 void CrashHandler::UploadDumps(const base::FilePath& crash_dump_path,
                                const std::string& uuid,
-                               const std::string& application_feedback,
-                               bool periodic_upload) {
+                               const std::string& application_feedback) {
   JNIEnv* env = base::android::AttachCurrentThread();
   base::android::ScopedJavaLocalRef<jstring> crash_dump_path_java =
       base::android::ConvertUTF8ToJavaString(env, crash_dump_path.value());
@@ -103,9 +97,18 @@ void CrashHandler::UploadDumps(const base::FilePath& crash_dump_path,
       base::android::ConvertUTF8ToJavaString(env, uuid);
   base::android::ScopedJavaLocalRef<jstring> application_feedback_java =
       base::android::ConvertUTF8ToJavaString(env, application_feedback);
-  Java_CastCrashHandler_initializeUploader(
-      env, crash_dump_path_java, uuid_java, application_feedback_java,
-      UploadCrashToStaging(), periodic_upload);
+  bool can_send_usage_stats =
+      android::ChromecastConfigAndroid::GetInstance()->CanSendUsageStats();
+
+  if (can_send_usage_stats) {
+    Java_CastCrashHandler_uploadOnce(env, crash_dump_path_java, uuid_java,
+                                     application_feedback_java,
+                                     UploadCrashToStaging());
+  } else {
+    Java_CastCrashHandler_removeCrashDumps(env, crash_dump_path_java, uuid_java,
+                                           application_feedback_java,
+                                           UploadCrashToStaging());
+  }
 }
 
 }  // namespace chromecast

chromecast/app/android/crash_handler.h

@@ -28,8 +28,7 @@ class CrashHandler {
   static bool GetCrashDumpLocation(base::FilePath* crash_dir);
   static void UploadDumps(const base::FilePath& crash_dump_path,
                           const std::string& uuid,
-                          const std::string& application_feedback,
-                          bool periodic_upload);
+                          const std::string& application_feedback);
 
  private:
   CrashHandler(const std::string& process_type,
@@ -38,9 +37,6 @@ class CrashHandler {
 
   void Initialize();
 
-  // Starts a thread to periodically check for uploads
-  void InitializeUploader();
-
   // Path to the current process's log file.
   base::FilePath log_file_path_;
 

chromecast/base/BUILD.gn

@@ -55,8 +55,6 @@ cast_source_set("base") {
     "cast_paths.h",
     "cast_resource.cc",
     "cast_resource.h",
-    "chromecast_config_android.cc",
-    "chromecast_config_android.h",
     "device_capabilities.h",
     "device_capabilities_impl.cc",
     "device_capabilities_impl.h",
@@ -104,6 +102,18 @@ cast_source_set("base") {
   } else {
     deps += [ "//chromecast/internal/base:cast_internal_features" ]
   }
+
+  if (is_android) {
+    sources += [ "chromecast_config_android.h" ]
+    if (chromecast_branding == "public") {
+      sources += [ "chromecast_config_android_dummy.cc" ]
+    } else {
+      sources +=
+          [ "//chromecast/internal/base/chromecast_config_android_internal.cc" ]
+      deps +=
+          [ "//chromecast/internal/base:chromecast_config_android_jni_headers" ]
+    }
+  }
 }
 
 # chromecast related switch values
@@ -289,7 +299,6 @@ process_version("cast_version") {
 if (is_android) {
   generate_jni("jni_headers") {
     sources = [
-      "java/src/org/chromium/chromecast/base/ChromecastConfigAndroid.java",
       "java/src/org/chromium/chromecast/base/DumpstateWriter.java",
       "java/src/org/chromium/chromecast/base/SystemTimeChangeNotifierAndroid.java",
     ]
@@ -320,7 +329,6 @@ if (is_android) {
       # TODO(sanfin): Move these files to another target.
       "$java_src_dir/org/chromium/chromecast/base/CastSettingsManager.java",
       "$java_src_dir/org/chromium/chromecast/base/CastSwitches.java",
-      "$java_src_dir/org/chromium/chromecast/base/ChromecastConfigAndroid.java",
       "$java_src_dir/org/chromium/chromecast/base/DumpstateWriter.java",
       "$java_src_dir/org/chromium/chromecast/base/SystemTimeChangeNotifierAndroid.java",
     ]

chromecast/base/chromecast_config_android.cc

-// Copyright 2014 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 "chromecast/base/chromecast_config_android.h"
-
-#include <utility>
-
-#include "base/android/jni_android.h"
-#include "jni/ChromecastConfigAndroid_jni.h"
-
-using base::android::JavaParamRef;
-
-namespace chromecast {
-namespace android {
-
-// static
-ChromecastConfigAndroid* ChromecastConfigAndroid::GetInstance() {
-  static base::NoDestructor<ChromecastConfigAndroid> instance;
-  return instance.get();
-}
-
-ChromecastConfigAndroid::ChromecastConfigAndroid() {
-}
-
-ChromecastConfigAndroid::~ChromecastConfigAndroid() {
-}
-
-bool ChromecastConfigAndroid::CanSendUsageStats() {
-  // TODO(sanfin): make opt-in.stats pref the source of truth for this data,
-  // instead of Android prefs, then delete ChromecastConfigAndroid.
-  JNIEnv* env = base::android::AttachCurrentThread();
-  return Java_ChromecastConfigAndroid_canSendUsageStats(env);
-}
-
-void ChromecastConfigAndroid::SetSendUsageStats(bool enabled) {
-  JNIEnv* env = base::android::AttachCurrentThread();
-  Java_ChromecastConfigAndroid_setSendUsageStats(env, enabled);
-}
-
-// Registers a handler to be notified when SendUsageStats is changed.
-void ChromecastConfigAndroid::SetSendUsageStatsChangedCallback(
-    base::RepeatingCallback<void(bool)> callback) {
-  send_usage_stats_changed_callback_ = std::move(callback);
-}
-
-void ChromecastConfigAndroid::RunSendUsageStatsChangedCallback(bool enabled) {
-  send_usage_stats_changed_callback_.Run(enabled);
-}
-
-// Called from Java.
-void JNI_ChromecastConfigAndroid_SetSendUsageStatsEnabled(
-    JNIEnv* env,
-    const JavaParamRef<jclass>& caller,
-    jboolean enabled) {
-  ChromecastConfigAndroid::GetInstance()->RunSendUsageStatsChangedCallback(
-      enabled);
-}
-
-}  // namespace android
-}  // namespace chromecast

chromecast/base/chromecast_config_android.h

@@ -5,8 +5,6 @@
 #ifndef CHROMECAST_BASE_CHROMECAST_CONFIG_ANDROID_H_
 #define CHROMECAST_BASE_CHROMECAST_CONFIG_ANDROID_H_
 
-#include <jni.h>
-
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/no_destructor.h"
@@ -20,24 +18,20 @@ class ChromecastConfigAndroid {
 
   // Returns whether or not the user has allowed sending usage stats and
   // crash reports.
-  bool CanSendUsageStats();
-
-  void SetSendUsageStats(bool enabled);
+  virtual bool CanSendUsageStats() = 0;
 
   // Registers a handler to be notified when SendUsageStats is changed.
-  void SetSendUsageStatsChangedCallback(
-      base::RepeatingCallback<void(bool)> callback);
+  virtual void SetSendUsageStatsChangedCallback(
+      base::RepeatingCallback<void(bool)> callback) = 0;
 
-  void RunSendUsageStatsChangedCallback(bool enabled);
+  virtual void RunSendUsageStatsChangedCallback(bool enabled) = 0;
 
- private:
-  friend class base::NoDestructor<ChromecastConfigAndroid>;
+ protected:
+  ChromecastConfigAndroid() {}
 
-  ChromecastConfigAndroid();
-  ~ChromecastConfigAndroid();
-
-  base::RepeatingCallback<void(bool)> send_usage_stats_changed_callback_;
+  virtual ~ChromecastConfigAndroid() {}
 
+ private:
   DISALLOW_COPY_AND_ASSIGN(ChromecastConfigAndroid);
 };
 

chromecast/base/chromecast_config_android_dummy.cc

+// Copyright 2018 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 "chromecast/base/chromecast_config_android.h"
+
+namespace chromecast {
+namespace android {
+
+// Dummy implementation of android::ChromecastConfigAndroid.
+class ChromecastConfigAndroidDummy : public ChromecastConfigAndroid {
+ public:
+  ChromecastConfigAndroidDummy() {}
+
+  ~ChromecastConfigAndroidDummy() override {}
+
+  bool CanSendUsageStats() override { return false; }
+
+  void SetSendUsageStatsChangedCallback(
+      base::RepeatingCallback<void(bool)> callback) override {}
+
+  void RunSendUsageStatsChangedCallback(bool enabled) override {}
+
+ private:
+  friend class base::NoDestructor<ChromecastConfigAndroidDummy>;
+
+  DISALLOW_COPY_AND_ASSIGN(ChromecastConfigAndroidDummy);
+};
+
+// static
+ChromecastConfigAndroid* ChromecastConfigAndroid::GetInstance() {
+  static base::NoDestructor<ChromecastConfigAndroidDummy> instance;
+  return instance.get();
+}
+
+}  // namespace android
+}  // namespace chromecast

chromecast/base/java/src/org/chromium/chromecast/base/CastSettingsManager.java

@@ -7,7 +7,6 @@ package org.chromium.chromecast.base;
 import android.annotation.SuppressLint;
 import android.content.ContentResolver;
 import android.content.Context;
-import android.content.SharedPreferences;
 import android.database.ContentObserver;
 import android.os.Build;
 import android.os.Handler;
@@ -23,12 +22,6 @@ public final class CastSettingsManager {
 
     private static final String PREFS_FILE_NAME = "CastSettings";
 
-    /** Key for the "send usage stats" boolean setting. */
-    private static final String SEND_USAGE_STATS_SETTING = "developer_support";
-
-    /** The default value for the "send usage stats" setting. */
-    private static final boolean SEND_USAGE_STATS_SETTING_DEFAULT = true;
-
     /** The default device name, which is the model name. */
     private static final String DEFAULT_DEVICE_NAME = Build.MODEL;
 
@@ -38,8 +31,6 @@ public final class CastSettingsManager {
     private static final String DEVICE_PROVISIONED_SETTING_KEY = Settings.Global.DEVICE_PROVISIONED;
 
     private final ContentResolver mContentResolver;
-    private final SharedPreferences mSettings;
-    private SharedPreferences.OnSharedPreferenceChangeListener mSharedPreferenceListener;
     private ContentObserver mDeviceNameObserver;
     private ContentObserver mIsDeviceProvisionedObserver;
 
@@ -49,7 +40,6 @@ public final class CastSettingsManager {
      */
     public static class OnSettingChangedListener {
         public void onCastEnabledChanged(boolean enabled) {}
-        public void onSendUsageStatsChanged(boolean enabled) {}
         public void onDeviceNameChanged(String deviceName) {}
     }
 
@@ -62,34 +52,15 @@ public final class CastSettingsManager {
     public static CastSettingsManager createCastSettingsManager(
             Context context, OnSettingChangedListener listener) {
         ContentResolver contentResolver = context.getContentResolver();
-        SharedPreferences settings = context.getSharedPreferences(PREFS_FILE_NAME, 0);
-        return new CastSettingsManager(contentResolver, listener, settings);
+        return new CastSettingsManager(contentResolver, listener);
     }
 
     @SuppressLint("NewApi")
     private CastSettingsManager(
-            ContentResolver contentResolver,
-            OnSettingChangedListener listener,
-            SharedPreferences settings) {
+            ContentResolver contentResolver, OnSettingChangedListener listener) {
         mContentResolver = contentResolver;
-        mSettings = settings;
         mListener = listener;
 
-        mSharedPreferenceListener = new SharedPreferences.OnSharedPreferenceChangeListener() {
-            @Override
-            public void onSharedPreferenceChanged(SharedPreferences prefs, String key) {
-                if (mListener == null) {
-                    return;
-                }
-
-                if (key.equals(SEND_USAGE_STATS_SETTING)) {
-                    mListener.onSendUsageStatsChanged(prefs.getBoolean(key,
-                                    SEND_USAGE_STATS_SETTING_DEFAULT));
-                }
-            }
-        };
-        mSettings.registerOnSharedPreferenceChangeListener(mSharedPreferenceListener);
-
         mDeviceNameObserver = new ContentObserver(new Handler()) {
             @Override
             public void onChange(boolean selfChange) {
@@ -116,8 +87,6 @@ public final class CastSettingsManager {
     }
 
     public void dispose() {
-        mSettings.unregisterOnSharedPreferenceChangeListener(mSharedPreferenceListener);
-        mSharedPreferenceListener = null;
         mContentResolver.unregisterContentObserver(mDeviceNameObserver);
         mDeviceNameObserver = null;
 
@@ -135,14 +104,6 @@ public final class CastSettingsManager {
                 mContentResolver, DEVICE_PROVISIONED_SETTING_KEY, 0) == 1;
     }
 
-    public boolean isSendUsageStatsEnabled() {
-        return mSettings.getBoolean(SEND_USAGE_STATS_SETTING, SEND_USAGE_STATS_SETTING_DEFAULT);
-    }
-
-    public void setSendUsageStatsEnabled(boolean enabled) {
-        mSettings.edit().putBoolean(SEND_USAGE_STATS_SETTING, enabled).apply();
-    }
-
     @SuppressLint("NewApi")
     public String getDeviceName() {
         // TODO(crbug.com/635567): Fix lint properly.

chromecast/base/java/src/org/chromium/chromecast/base/ChromecastConfigAndroid.java

-// Copyright 2015 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.
-
-package org.chromium.chromecast.base;
-
-import android.content.Context;
-
-import org.chromium.base.annotations.CalledByNative;
-import org.chromium.base.annotations.JNINamespace;
-
-/**
- * JNI wrapper class for calls from ChromecastConfigAndroid.
- */
-@JNINamespace("chromecast::android")
-public final class ChromecastConfigAndroid {
-
-    private static CastSettingsManager sSettingsManager;
-
-    public static void initializeForBrowser(Context context) {
-        sSettingsManager = CastSettingsManager.createCastSettingsManager(
-                context, new CastSettingsManager.OnSettingChangedListener() {
-                    @Override
-                    public void onSendUsageStatsChanged(boolean enabled) {
-                        nativeSetSendUsageStatsEnabled(enabled);
-                    }
-                });
-    }
-
-    @CalledByNative
-    public static boolean canSendUsageStats() {
-        return sSettingsManager.isSendUsageStatsEnabled();
-    }
-
-    @CalledByNative
-    public static void setSendUsageStats(boolean enabled) {
-        sSettingsManager.setSendUsageStatsEnabled(enabled);
-    }
-
-    private static native void nativeSetSendUsageStatsEnabled(boolean enabled);
-}

chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastBrowserHelper.java

@@ -12,7 +12,6 @@ import org.chromium.base.Log;
 import org.chromium.base.library_loader.LibraryLoader;
 import org.chromium.base.library_loader.LibraryProcessType;
 import org.chromium.base.library_loader.ProcessInitException;
-import org.chromium.chromecast.base.ChromecastConfigAndroid;
 import org.chromium.content_public.browser.BrowserStartupController;
 import org.chromium.content_public.browser.DeviceUtils;
 import org.chromium.net.NetworkChangeNotifier;
@@ -38,8 +37,6 @@ public class CastBrowserHelper {
 
         Log.d(TAG, "Performing one-time browser initialization");
 
-        ChromecastConfigAndroid.initializeForBrowser(context);
-
         // Initializing the command line must occur before loading the library.
         CastCommandLineHelper.initCommandLineWithSavedArgs(() -> {
             CommandLineInitUtil.initCommandLine(COMMAND_LINE_FILE);

chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastCrashHandler.java

@@ -4,10 +4,8 @@
 
 package org.chromium.chromecast.shell;
 
-import org.chromium.base.Log;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
-import org.chromium.chromecast.base.ChromecastConfigAndroid;
 
 /**
  * JNI wrapper class for accessing CastCrashHandler.
@@ -17,19 +15,18 @@ public final class CastCrashHandler {
     private static final String TAG = "cr_CastCrashHandler";
 
     @CalledByNative
-    public static void initializeUploader(String crashDumpPath, String uuid,
-            String applicationFeedback, boolean uploadCrashToStaging, boolean periodicUpload) {
+    public static void uploadOnce(String crashDumpPath, String uuid, String applicationFeedback,
+            boolean uploadCrashToStaging) {
         CastCrashUploader uploader = CastCrashUploaderFactory.createCastCrashUploader(
                 crashDumpPath, uuid, applicationFeedback, uploadCrashToStaging);
-        if (ChromecastConfigAndroid.canSendUsageStats()) {
-            if (periodicUpload) {
-                uploader.startPeriodicUpload();
-            } else {
-                uploader.uploadOnce();
-            }
-        } else {
-            Log.d(TAG, "Removing crash dumps instead of uploading");
-            uploader.removeCrashDumps();
-        }
+        uploader.uploadOnce();
+    }
+
+    @CalledByNative
+    public static void removeCrashDumps(String crashDumpPath, String uuid,
+            String applicationFeedback, boolean uploadCrashToStaging) {
+        CastCrashUploader uploader = CastCrashUploaderFactory.createCastCrashUploader(
+                crashDumpPath, uuid, applicationFeedback, uploadCrashToStaging);
+        uploader.removeCrashDumps();
     }
 }

chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastCrashUploader.java

@@ -62,21 +62,13 @@ public final class CastCrashUploader {
                 : "https://clients2.google.com/cr/report";
     }
 
-    /** Sets up a periodic uploader, that checks for new dumps to upload every 20 minutes */
-    @SuppressWarnings("FutureReturnValueIgnored")
-    public void startPeriodicUpload() {
-        mExecutorService.scheduleWithFixedDelay(mQueueAllCrashDumpUploadsRunnable,
-                0, // Do first run immediately
-                20, // Run once every 20 minutes
-                TimeUnit.MINUTES);
-    }
-
     @SuppressWarnings("FutureReturnValueIgnored")
     public void uploadOnce() {
         mExecutorService.schedule(mQueueAllCrashDumpUploadsRunnable, 0, TimeUnit.MINUTES);
     }
 
     public void removeCrashDumps() {
+        Log.i(TAG, "Remove crash dumps");
         File crashDumpDirectory = new File(mCrashDumpPath);
         for (File potentialDump : crashDumpDirectory.listFiles()) {
             if (potentialDump.getName().matches(DUMP_FILE_REGEX)) {
@@ -94,6 +86,7 @@ public final class CastCrashUploader {
      */
     private void checkForCrashDumps() {
         if (mCrashDumpPath == null) return;
+
         Log.i(TAG, "Checking for crash dumps");
         File crashDumpDirectory = new File(mCrashDumpPath);
 

chromecast/browser/cast_browser_main_parts.cc

@@ -486,6 +486,10 @@ void CastBrowserMainParts::PreMainMessageLoopRun() {
       base::Unretained(cast_browser_process_->browser_client())));
 #endif  // !defined(OS_ANDROID) && !defined(OS_FUCHSIA)
 
+#if defined(OS_ANDROID)
+  StartPeriodicCrashReportUpload();
+#endif  // defined(OS_ANDROID)
+
   cast_browser_process_->SetNetLog(net_log_.get());
   url_request_context_factory_->InitializeOnUIThread(net_log_.get());
 
@@ -598,6 +602,22 @@ void CastBrowserMainParts::PreMainMessageLoopRun() {
   cast_browser_process_->cast_service()->Start();
 }
 
+#if defined(OS_ANDROID)
+void CastBrowserMainParts::StartPeriodicCrashReportUpload() {
+  OnStartPeriodicCrashReportUpload();
+  crash_reporter_timer_.reset(new base::RepeatingTimer());
+  crash_reporter_timer_->Start(
+      FROM_HERE, base::TimeDelta::FromMinutes(20), this,
+      &CastBrowserMainParts::OnStartPeriodicCrashReportUpload);
+}
+
+void CastBrowserMainParts::OnStartPeriodicCrashReportUpload() {
+  base::FilePath crash_dir;
+  CrashHandler::GetCrashDumpLocation(&crash_dir);
+  CrashHandler::UploadDumps(crash_dir, "", "");
+}
+#endif  // defined(OS_ANDROID)
+
 bool CastBrowserMainParts::MainMessageLoopRun(int* result_code) {
 #if defined(OS_ANDROID)
   // Android does not use native main MessageLoop.

chromecast/browser/cast_browser_main_parts.h

@@ -10,12 +10,17 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/metrics/field_trial.h"
+#include "build/build_config.h"
 #include "build/buildflag.h"
 #include "chromecast/chromecast_buildflags.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_main_parts.h"
 #include "content/public/common/main_function_params.h"
 
+#if defined(OS_ANDROID)
+#include "base/timer/timer.h"
+#endif
+
 class PrefService;
 
 namespace base {
@@ -100,6 +105,12 @@ class CastBrowserMainParts : public content::BrowserMainParts {
   std::unique_ptr<CastWindowManager> window_manager_;
 #endif  //  defined(USE_AURA)
 
+#if defined(OS_ANDROID)
+  void StartPeriodicCrashReportUpload();
+  void OnStartPeriodicCrashReportUpload();
+  std::unique_ptr<base::RepeatingTimer> crash_reporter_timer_;
+#endif
+
 #if BUILDFLAG(IS_CAST_USING_CMA_BACKEND)
   // CMA thread used by AudioManager, MojoRenderer, and MediaPipelineBackend.
   std::unique_ptr<base::Thread> media_thread_;