Add WebView-specific whitelist for crash keys.

WebView shouldn't upload data representing all crash keys declared by
Chrome since some of that data could be private.
To be able to re-use the existing crash-keys mechanism we here add a
WebView-only whitelist that will be applied at the time data is set for
a key, rather than at registration time.
In this way we ensure that all keys that are being set are also being
registered, while allowing WebView to avoid adding all registered keys.

BUG=691560

Review-Url: https://codereview.chromium.org/2717223003
Cr-Commit-Position: refs/heads/master@{#454611}

android_webview/common/crash_reporter/aw_microdump_crash_reporter.cc

@@ -10,6 +10,7 @@
 #include "android_webview/common/crash_reporter/crash_keys.h"
 #include "base/android/build_info.h"
 #include "base/base_paths_android.h"
+#include "base/debug/crash_logging.h"
 #include "base/debug/dump_without_crashing.h"
 #include "base/files/file_path.h"
 #include "base/lazy_instance.h"
@@ -36,6 +37,8 @@ class AwCrashReporterClient : public ::crash_reporter::CrashReporterClient {
 
   // crash_reporter::CrashReporterClient implementation.
   size_t RegisterCrashKeys() override;
+  bool UseCrashKeysWhiteList() override { return true; }
+  const char* const* GetCrashKeyWhiteList() override;
 
   bool IsRunningUnattended() override { return false; }
   bool GetCollectStatsConsent() override {
@@ -86,6 +89,10 @@ size_t AwCrashReporterClient::RegisterCrashKeys() {
   return crash_keys::RegisterWebViewCrashKeys();
 }
 
+const char* const* AwCrashReporterClient::GetCrashKeyWhiteList() {
+  return crash_keys::kWebViewCrashKeyWhiteList;
+}
+
 base::LazyInstance<AwCrashReporterClient>::Leaky g_crash_reporter_client =
     LAZY_INSTANCE_INITIALIZER;
 

android_webview/common/crash_reporter/crash_keys.cc

@@ -5,6 +5,7 @@
 #include "android_webview/common/crash_reporter/crash_keys.h"
 
 #include "base/debug/crash_logging.h"
+#include "components/crash/content/app/breakpad_linux.h"
 #include "components/crash/core/common/crash_keys.h"
 
 using namespace crash_keys;
@@ -12,33 +13,157 @@ using namespace crash_keys;
 namespace android_webview {
 namespace crash_keys {
 
+const char kActiveURL[] = "url-chunk";
+
+const char kFontKeyName[] = "font_key_name";
+
+const char kShutdownType[] = "shutdown-type";
+const char kBrowserUnpinTrace[] = "browser-unpin-trace";
+
 const char kGPUDriverVersion[] = "gpu-driver";
 const char kGPUPixelShaderVersion[] = "gpu-psver";
 const char kGPUVertexShaderVersion[] = "gpu-vsver";
 const char kGPUVendor[] = "gpu-gl-vendor";
 const char kGPURenderer[] = "gpu-gl-renderer";
 
+const char kInputEventFilterSendFailure[] = "input-event-filter-send-failure";
+
+const char kViewCount[] = "view-count";
+
+const char kZeroEncodeDetails[] = "zero-encode-details";
+
 size_t RegisterWebViewCrashKeys() {
   base::debug::CrashKey fixed_keys[] = {
-    { kGPUDriverVersion, kSmallSize },
-    { kGPUPixelShaderVersion, kSmallSize },
-    { kGPUVertexShaderVersion, kSmallSize },
-    { kGPUVendor, kSmallSize },
-    { kGPURenderer, kSmallSize },
+      {"AW_WHITELISTED_DEBUG_KEY", kSmallSize},
+      {"AW_NONWHITELISTED_DEBUG_KEY", kSmallSize},
+      {kClientId, kSmallSize},
+      {kChannel, kSmallSize},
+      {kActiveURL, kLargeSize},
+      {kNumVariations, kSmallSize},
+      {kVariations, kHugeSize},
+      {kShutdownType, kSmallSize},
+      {kBrowserUnpinTrace, kMediumSize},
+      {kGPUDriverVersion, kSmallSize},
+      {kGPUPixelShaderVersion, kSmallSize},
+      {kGPUVertexShaderVersion, kSmallSize},
+      {kGPUVendor, kSmallSize},
+      {kGPURenderer, kSmallSize},
 
-    // content/:
-    { "bad_message_reason", kSmallSize },
-    { "discardable-memory-allocated", kSmallSize },
-    { "discardable-memory-free", kSmallSize },
-    { "mojo-message-error", kMediumSize },
-    { "total-discardable-memory-allocated", kSmallSize },
+      // content/:
+      {"bad_message_reason", kSmallSize},
+      {"discardable-memory-allocated", kSmallSize},
+      {"discardable-memory-free", kSmallSize},
+      {kFontKeyName, kSmallSize},
+      {"mojo-message-error", kMediumSize},
+      {"ppapi_path", kMediumSize},
+      {"subresource_url", kLargeSize},
+      {"total-discardable-memory-allocated", kSmallSize},
+      {kInputEventFilterSendFailure, kSmallSize},
+      {kBug464926CrashKey, kSmallSize},
+      {kViewCount, kSmallSize},
+
+      // media/:
+      {kZeroEncodeDetails, kSmallSize},
+
+      // gin/:
+      {"v8-ignition", kSmallSize},
+
+      // sandbox/:
+      {"seccomp-sigsys", kMediumSize},
+
+      // Temporary for http://crbug.com/575245.
+      {"swapout_frame_id", kSmallSize},
+      {"swapout_proxy_id", kSmallSize},
+      {"swapout_view_id", kSmallSize},
+      {"commit_frame_id", kSmallSize},
+      {"commit_proxy_id", kSmallSize},
+      {"commit_view_id", kSmallSize},
+      {"commit_main_render_frame_id", kSmallSize},
+      {"newproxy_proxy_id", kSmallSize},
+      {"newproxy_view_id", kSmallSize},
+      {"newproxy_opener_id", kSmallSize},
+      {"newproxy_parent_id", kSmallSize},
+      {"rvinit_view_id", kSmallSize},
+      {"rvinit_proxy_id", kSmallSize},
+      {"rvinit_main_frame_id", kSmallSize},
+      {"initrf_frame_id", kSmallSize},
+      {"initrf_proxy_id", kSmallSize},
+      {"initrf_view_id", kSmallSize},
+      {"initrf_main_frame_id", kSmallSize},
+      {"initrf_view_is_live", kSmallSize},
+
+      // Temporary for https://crbug.com/591478.
+      {"initrf_parent_proxy_exists", kSmallSize},
+      {"initrf_render_view_is_live", kSmallSize},
+      {"initrf_parent_is_in_same_site_instance", kSmallSize},
+      {"initrf_parent_process_is_live", kSmallSize},
+      {"initrf_root_is_in_same_site_instance", kSmallSize},
+      {"initrf_root_is_in_same_site_instance_as_parent", kSmallSize},
+      {"initrf_root_process_is_live", kSmallSize},
+      {"initrf_root_proxy_is_live", kSmallSize},
+
+      // Temporary for https://crbug.com/626802.
+      {"newframe_routing_id", kSmallSize},
+      {"newframe_proxy_id", kSmallSize},
+      {"newframe_opener_id", kSmallSize},
+      {"newframe_parent_id", kSmallSize},
+      {"newframe_widget_id", kSmallSize},
+      {"newframe_widget_hidden", kSmallSize},
+      {"newframe_replicated_origin", kSmallSize},
+      {"newframe_oopifs_possible", kSmallSize},
+
+      // Temporary for https://crbug.com/630103.
+      {"origin_mismatch_url", kLargeSize},
+      {"origin_mismatch_origin", kMediumSize},
+      {"origin_mismatch_transition", kSmallSize},
+      {"origin_mismatch_redirects", kSmallSize},
+      {"origin_mismatch_same_page", kSmallSize},
+
+      // Temporary for https://crbug.com/612711.
+      {"aci_wrong_sp_extension_id", kSmallSize},
+
+      // Temporary for https://crbug.com/668633.
+      {"swdh_set_hosted_version_worker_pid", kSmallSize},
+      {"swdh_set_hosted_version_host_pid", kSmallSize},
+      {"swdh_set_hosted_version_is_new_process", kSmallSize},
+      {"swdh_set_hosted_version_restart_count", kSmallSize},
   };
 
+  // This dynamic set of keys is used for sets of key value pairs when gathering
+  // a collection of data, like command line switches or extension IDs.
   std::vector<base::debug::CrashKey> keys(fixed_keys,
                                           fixed_keys + arraysize(fixed_keys));
 
+  GetCrashKeysForCommandLineSwitches(&keys);
+
   return base::debug::InitCrashKeys(&keys.at(0), keys.size(), kChunkMaxLength);
 }
+
+// clang-format off
+const char* const kWebViewCrashKeyWhiteList[] = {
+    "AW_WHITELISTED_DEBUG_KEY",
+    kGPUDriverVersion,
+    kGPUPixelShaderVersion,
+    kGPUVertexShaderVersion,
+    kGPUVendor,
+    kGPURenderer,
+
+    // content/:
+    "bad_message_reason",
+    "discardable-memory-allocated",
+    "discardable-memory-free",
+    "mojo-message-error-1",
+    "mojo-message-error-2",
+    "mojo-message-error-3",
+    "mojo-message-error-4",
+    "total-discardable-memory-allocated",
+    nullptr};
+// clang-format on
+
+void InitCrashKeysForWebViewTesting() {
+  breakpad::InitCrashKeysForTesting();
 }
 
 }  // namespace crash_keys
+
+}  // namespace android_webview

android_webview/common/crash_reporter/crash_keys.h

@@ -6,6 +6,7 @@
 #define ANDROID_WEBVIEW_COMMON_CRASH_KEYS_H_
 
 #include <stddef.h>
+#include <string>
 
 namespace android_webview {
 namespace crash_keys {
@@ -13,6 +14,10 @@ namespace crash_keys {
 // Registers all of the potential crash keys that can be sent to the crash
 // reporting server. Returns the size of the union of all keys.
 size_t RegisterWebViewCrashKeys();
+void InitCrashKeysForWebViewTesting();
+void SetCrashKeyValue(const std::string& key, const std::string& value);
+
+extern const char* const kWebViewCrashKeyWhiteList[];
 
 // Crash Key Name Constants ////////////////////////////////////////////////////
 

android_webview/java/src/org/chromium/android_webview/AwDebug.java

@@ -42,5 +42,15 @@ public class AwDebug {
         return nativeDumpWithoutCrashing(dumpPath);
     }
 
+    public static void initCrashKeysForTesting() {
+        nativeInitCrashKeysForWebViewTesting();
+    }
+
+    public static void setCrashKeyValue(String key, String value) {
+        nativeSetCrashKeyValue(key, value);
+    }
+
     private static native boolean nativeDumpWithoutCrashing(String dumpPath);
+    private static native void nativeInitCrashKeysForWebViewTesting();
+    private static native void nativeSetCrashKeyValue(String key, String value);
 }

android_webview/javatests/src/org/chromium/android_webview/test/AwDebugTest.java

@@ -11,6 +11,9 @@ import org.chromium.base.test.util.Feature;
 import org.chromium.base.test.util.parameter.ParameterizedTest;
 
 import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.util.Scanner;
 
 /**
  * A test suite for AwDebug class.
@@ -18,6 +21,11 @@ import java.io.File;
 // Only works in single-process mode, crbug.com/568825.
 @ParameterizedTest.Set
 public class AwDebugTest extends AwTestBase {
+    private static final String TAG = "cr_AwDebugTest";
+    private static final String WHITELISTED_DEBUG_KEY = "AW_WHITELISTED_DEBUG_KEY";
+    private static final String NON_WHITELISTED_DEBUG_KEY = "AW_NONWHITELISTED_DEBUG_KEY";
+    private static final String DEBUG_VALUE = "AW_DEBUG_VALUE";
+
     @SmallTest
     @Feature({"AndroidWebView", "Debug"})
     public void testDump() throws Throwable {
@@ -30,4 +38,74 @@ public class AwDebugTest extends AwTestBase {
             assertTrue(f.delete());
         }
     }
+
+    @SmallTest
+    @Feature({"AndroidWebView", "Debug"})
+    public void testDumpContainsWhitelistedKey() throws Throwable {
+        File f = File.createTempFile("dump", ".dmp");
+        try {
+            AwDebug.initCrashKeysForTesting();
+            AwDebug.setCrashKeyValue(WHITELISTED_DEBUG_KEY, DEBUG_VALUE);
+            assertTrue(AwDebug.dumpWithoutCrashing(f));
+            assertContainsCrashKeyValue(f, WHITELISTED_DEBUG_KEY, DEBUG_VALUE);
+        } finally {
+            assertTrue(f.delete());
+        }
+    }
+
+    @SmallTest
+    @Feature({"AndroidWebView", "Debug"})
+    public void testDumpDoesNotContainNonWhitelistedKey() throws Throwable {
+        File f = File.createTempFile("dump", ".dmp");
+        try {
+            AwDebug.initCrashKeysForTesting();
+            AwDebug.setCrashKeyValue(NON_WHITELISTED_DEBUG_KEY, DEBUG_VALUE);
+            assertTrue(AwDebug.dumpWithoutCrashing(f));
+            assertNotContainsCrashKeyValue(f, NON_WHITELISTED_DEBUG_KEY);
+        } finally {
+            assertTrue(f.delete());
+        }
+    }
+
+    private void assertNotContainsCrashKeyValue(File dump, String key) throws IOException {
+        String dumpContents = readEntireFile(dump);
+        // We are expecting the following to NOT be in the dumpContents:
+        //
+        // Content-Disposition: form-data; name="AW_DEBUG_KEY"
+        // AW_DEBUG_VALUE
+        assertFalse(dumpContents.contains(getDebugKeyLine(key)));
+    }
+
+    private void assertContainsCrashKeyValue(File dump, String key, String expectedValue)
+            throws IOException {
+        String dumpContents = readEntireFile(dump);
+        // We are expecting the following lines:
+        //
+        // Content-Disposition: form-data; name="AW_DEBUG_KEY"
+        // AW_DEBUG_VALUE
+        String debugKeyLine = getDebugKeyLine(key);
+        assertTrue(dumpContents.contains(debugKeyLine));
+
+        int debugKeyIndex = dumpContents.indexOf(debugKeyLine);
+        // Read the word after the line containing the debug key
+        Scanner debugValueScanner =
+                new Scanner(dumpContents.substring(debugKeyIndex + debugKeyLine.length()));
+        assertTrue(debugValueScanner.hasNext());
+        assertEquals(expectedValue, debugValueScanner.next());
+    }
+
+    private static String getDebugKeyLine(String debugKey) {
+        return "Content-Disposition: form-data; name=\"" + debugKey + "\"";
+    }
+
+    private static String readEntireFile(File file) throws IOException {
+        FileInputStream fileInputStream = new FileInputStream(file);
+        try {
+            byte[] data = new byte[(int) file.length()];
+            fileInputStream.read(data);
+            return new String(data);
+        } finally {
+            fileInputStream.close();
+        }
+    }
 }

android_webview/native/aw_debug.cc

@@ -5,8 +5,10 @@
 #include "android_webview/native/aw_debug.h"
 
 #include "android_webview/common/crash_reporter/aw_microdump_crash_reporter.h"
+#include "android_webview/common/crash_reporter/crash_keys.h"
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
+#include "base/debug/crash_logging.h"
 #include "base/debug/dump_without_crashing.h"
 #include "base/files/file.h"
 #include "base/files/file_path.h"
@@ -36,6 +38,19 @@ static jboolean DumpWithoutCrashing(JNIEnv* env,
   return crash_reporter::DumpWithoutCrashingToFd(target.TakePlatformFile());
 }
 
+static void InitCrashKeysForWebViewTesting(JNIEnv* env,
+                                           const JavaParamRef<jclass>& clazz) {
+  crash_keys::InitCrashKeysForWebViewTesting();
+}
+
+static void SetCrashKeyValue(JNIEnv* env,
+                             const JavaParamRef<jclass>& clazz,
+                             const JavaParamRef<jstring>& key,
+                             const JavaParamRef<jstring>& value) {
+  base::debug::SetCrashKeyValue(ConvertJavaStringToUTF8(env, key),
+                                ConvertJavaStringToUTF8(env, value));
+}
+
 bool RegisterAwDebug(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }

chrome/common/crash_keys.cc

@@ -100,9 +100,8 @@ size_t RegisterChromeCrashKeys() {
   // The following keys may be chunked by the underlying crash logging system,
   // but ultimately constitute a single key-value pair.
   //
-  // If you're adding keys here, please also add them to the following list:
+  // If you're adding keys here, please also add them to the following lists:
   // chrome/app/chrome_crash_reporter_client_win.cc::RegisterCrashKeysHelper(),
-  // and consider adding the new keys to the following list as well:
   // android_webview/common/crash_reporter/crash_keys.cc::
   //     RegisterWebViewCrashKeys().
   base::debug::CrashKey fixed_keys[] = {

components/crash/content/app/breakpad_linux.cc

@@ -101,6 +101,9 @@ ExceptionHandler* g_breakpad = nullptr;
 const char* g_asan_report_str = nullptr;
 #endif
 
+bool g_use_crash_key_white_list = false;
+const char* const* g_crash_key_white_list = nullptr;
+
 #if defined(OS_ANDROID)
 #define G_DUMPS_SUPPRESSED_MAGIC 0x5AFECEDE
 uint32_t g_dumps_suppressed = 0;
@@ -1098,9 +1101,21 @@ void EnableNonBrowserCrashDumping() {
 }
 #endif  // defined(OS_ANDROID)
 
+bool IsInWhiteList(const base::StringPiece& key) {
+  DCHECK(g_crash_key_white_list);
+  for (size_t i = 0; g_crash_key_white_list[i]; ++i) {
+    if (0 == my_strcmp(g_crash_key_white_list[i], key.data())) {
+      return true;
+    }
+  }
+  return false;
+}
+
 void SetCrashKeyValue(const base::StringPiece& key,
                       const base::StringPiece& value) {
-  g_crash_keys->SetKeyValue(key.data(), value.data());
+  if (!g_use_crash_key_white_list || IsInWhiteList(key)) {
+    g_crash_keys->SetKeyValue(key.data(), value.data());
+  }
 }
 
 void ClearCrashKey(const base::StringPiece& key) {
@@ -1112,6 +1127,9 @@ void ClearCrashKey(const base::StringPiece& key) {
 void InitCrashKeys() {
   g_crash_keys = new CrashKeyStorage;
   GetCrashReporterClient()->RegisterCrashKeys();
+  g_use_crash_key_white_list =
+      GetCrashReporterClient()->UseCrashKeysWhiteList();
+  g_crash_key_white_list = GetCrashReporterClient()->GetCrashKeyWhiteList();
   base::debug::SetCrashKeyReportingFunctions(&SetCrashKeyValue, &ClearCrashKey);
 }
 
@@ -1905,6 +1923,10 @@ void InitCrashReporter(const std::string& process_type) {
   InitCrashReporter(process_type, sanitization_info);
 }
 
+void InitCrashKeysForTesting() {
+  InitCrashKeys();
+}
+
 void InitCrashReporter(const std::string& process_type,
                        const SanitizationInfo& sanitization_info) {
 #else

components/crash/content/app/breakpad_linux.h

@@ -17,6 +17,8 @@ namespace breakpad {
 extern void InitCrashReporter(const std::string& process_type);
 
 #if defined(OS_ANDROID)
+extern void InitCrashKeysForTesting();
+
 struct SanitizationInfo {
   bool should_sanitize_dumps = false;
   bool skip_dump_if_principal_mapping_not_referenced = false;

components/crash/content/app/crash_reporter_client.cc

@@ -122,6 +122,14 @@ size_t CrashReporterClient::RegisterCrashKeys() {
   return 0;
 }
 
+bool CrashReporterClient::UseCrashKeysWhiteList() {
+  return false;
+}
+
+const char* const* CrashReporterClient::GetCrashKeyWhiteList() {
+  return nullptr;
+}
+
 bool CrashReporterClient::IsRunningUnattended() {
   return true;
 }

components/crash/content/app/crash_reporter_client.h

@@ -144,6 +144,11 @@ class CrashReporterClient {
   // reporting server. Returns the size of the union of all keys.
   virtual size_t RegisterCrashKeys();
 
+  virtual bool UseCrashKeysWhiteList();
+
+  // Returns a NULL-terminated array of crash keys to whitelist.
+  virtual const char* const* GetCrashKeyWhiteList();
+
   // Returns true if running in unattended mode (for automated testing).
   virtual bool IsRunningUnattended();