Add crash keys for currently loaded and active dynamic module

The loaded module key is set the first time a dynamic module is loaded
and never cleared (i.e. it signifies, "has this ever been loaded"),
whereas the active module key is cleared when the dynamic module is
unloaded (i.e. it signifies, "is this currently in use").

Because these key values can change at runtime (in contrast to
BuildInfo information), add a CrashKeys class that can store the
information before the native library has been loaded (where the pure
Java exception reporter will log it manually) and forward it to the
native side afterwards, where Breakpad will automatically take care of
uploading them.

Bug: TBD
Change-Id: Ib67ce32e18dfc8b93be18ff87da06ceeaa450fef
Reviewed-on: https://chromium-review.googlesource.com/1193867Reviewed-by: Peter Wen <wnwen@chromium.org>
Reviewed-by: Michael van Ouwerkerk <mvanouwerkerk@chromium.org>
Commit-Queue: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587317}

chrome/android/BUILD.gn

@@ -390,6 +390,7 @@ action("chrome_android_java_google_api_keys_srcjar") {
 
 java_cpp_enum("chrome_android_java_enums_srcjar") {
   sources = [
+    "//chrome/browser/android/crash/crash_keys_android.h",
     "//chrome/browser/android/customtabs/detached_resource_request.h",
     "//chrome/browser/android/digital_asset_links/digital_asset_links_handler.h",
     "//chrome/browser/android/feedback/connectivity_checker.cc",

chrome/android/java/src/org/chromium/chrome/browser/crash/CrashKeys.java

+// 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.
+
+package org.chromium.chrome.browser.crash;
+
+import android.support.annotation.Nullable;
+
+import org.chromium.base.ThreadUtils;
+import org.chromium.base.annotations.CalledByNative;
+
+import java.util.concurrent.atomic.AtomicReferenceArray;
+
+/**
+ * This class allows setting crash keys from the Java side. The set of crash keys is defined at
+ * build time. To add a new crash key, add a new entry to:
+ * <ol>
+ *     <li>The CrashKeyIndex enum in {@code crash_keys_android.h}</li>
+ *     <li>The CrashKeyString array in {@code crash_keys_android.cc}</li>
+ *     <li>The {@link #KEYS} array in this class.</li>
+ * </ol>
+ */
+public class CrashKeys {
+    private static final String[] KEYS = new String[] {
+            "loaded_dynamic_module", "active_dynamic_module",
+    };
+
+    private final AtomicReferenceArray<String> mValues = new AtomicReferenceArray<>(KEYS.length);
+
+    // Outside of assertions only accessed on the UI thread.
+    private boolean mFlushed;
+
+    private static class Holder { static final CrashKeys INSTANCE = new CrashKeys(); }
+
+    private CrashKeys() {
+        assert CrashKeyIndex.NUM_KEYS == KEYS.length;
+    }
+
+    /**
+     * @return The shared instance of this class.
+     */
+    @CalledByNative
+    public static CrashKeys getInstance() {
+        return Holder.INSTANCE;
+    }
+
+    /**
+     * @param keyIndex The index of a crash key.
+     * @return The key for the given index.
+     */
+    static String getKey(@CrashKeyIndex int keyIndex) {
+        return KEYS[keyIndex];
+    }
+
+    /**
+     * @return An atomic array of all the crash key values. This method should only be called before
+     *         the values have been flushed to the native side.
+     * @see #flushToNative
+     */
+    AtomicReferenceArray<String> getValues() {
+        assert !mFlushed;
+        return mValues;
+    }
+
+    /**
+     * Sets a given crash key to the given value, or clears it. The value will either be stored in
+     * Java (for use by pure-Java exception reporting), or forwarded to the native CrashKeys.
+     * This method should only be called on the UI thread.
+     * @param keyIndex The {@link CrashKeyIndex} of a crash key.
+     * @param value The value for the given key, or null to clear it.
+     */
+    @CalledByNative
+    public void set(@CrashKeyIndex int keyIndex, @Nullable String value) {
+        ThreadUtils.assertOnUiThread();
+        if (mFlushed) {
+            nativeSet(keyIndex, value);
+            return;
+        }
+
+        mValues.set(keyIndex, value);
+    }
+
+    /**
+     * Flushes all crash keys to the native side. This method should be called on the UI thread when
+     * pure-Java exception handling is disabled in favor of native crash reporting.
+     */
+    @CalledByNative
+    void flushToNative() {
+        ThreadUtils.assertOnUiThread();
+
+        assert !mFlushed;
+        for (@CrashKeyIndex int i = 0; i < mValues.length(); i++) {
+            nativeSet(i, mValues.getAndSet(i, null));
+        }
+        mFlushed = true;
+    }
+
+    private native void nativeSet(int key, String value);
+}

chrome/android/java/src/org/chromium/chrome/browser/crash/PureJavaExceptionHandler.java

@@ -48,6 +48,7 @@ public class PureJavaExceptionHandler implements Thread.UncaughtExceptionHandler
         // all the handlers before mParent. In order to disable this handler, globally setting a
         // flag to ignore it seems to be the easiest way.
         sIsDisabled = true;
+        CrashKeys.getInstance().flushToNative();
     }
 
     private void reportJavaException(Throwable e) {

chrome/android/java/src/org/chromium/chrome/browser/crash/PureJavaExceptionReporter.java

@@ -4,6 +4,7 @@
 
 package org.chromium.chrome.browser.crash;
 
+import android.annotation.SuppressLint;
 import android.app.ActivityManager;
 import android.app.ActivityManager.RunningAppProcessInfo;
 import android.content.Context;
@@ -24,6 +25,7 @@ import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.util.UUID;
+import java.util.concurrent.atomic.AtomicReferenceArray;
 
 /**
  * Creates a crash report and uploads it to crash server if there is a Java exception.
@@ -103,6 +105,7 @@ public class PureJavaExceptionReporter {
         }
     }
 
+    @SuppressLint("WrongConstant")
     private void createReport(Throwable javaException) {
         try {
             String minidumpFileName = FILE_PREFIX + mLocalId + FILE_SUFFIX;
@@ -142,6 +145,12 @@ public class PureJavaExceptionReporter {
         addPairedString(CUSTOM_THEMES, buildInfo.customThemes);
         addPairedString(RESOURCES_VERSION, buildInfo.resourcesVersion);
 
+        AtomicReferenceArray<String> values = CrashKeys.getInstance().getValues();
+        for (int i = 0; i < values.length(); i++) {
+            String value = values.get(i);
+            if (value != null) addPairedString(CrashKeys.getKey(i), value);
+        }
+
         addString(mBoundary);
     }
 

chrome/android/java/src/org/chromium/chrome/browser/customtabs/dynamicmodule/ModuleLoader.java

@@ -15,6 +15,8 @@ import org.chromium.base.AsyncTask;
 import org.chromium.base.Callback;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.Log;
+import org.chromium.chrome.browser.crash.CrashKeyIndex;
+import org.chromium.chrome.browser.crash.CrashKeys;
 
 /**
  * Dynamically loads a module from another apk.
@@ -24,6 +26,7 @@ public class ModuleLoader {
 
     /** Specifies the module package name and entry point class name. */
     private final ComponentName mComponentName;
+    private final String mModuleId;
     private int mModuleUseCount;
     @Nullable
     private ModuleEntryPoint mModuleEntryPoint;
@@ -34,6 +37,18 @@ public class ModuleLoader {
      */
     public ModuleLoader(ComponentName componentName) {
         mComponentName = componentName;
+        String packageName = componentName.getPackageName();
+        String version = "";
+        try {
+            version = ContextUtils.getApplicationContext()
+                              .getPackageManager()
+                              .getPackageInfo(packageName, 0)
+                              .versionName;
+        } catch (PackageManager.NameNotFoundException ignored) {
+            // Ignore the exception. Failure to find the package name will be handled in
+            // getModuleContext() below.
+        }
+        mModuleId = packageName + ":" + version;
     }
 
     public ComponentName getComponentName() {
@@ -83,6 +98,7 @@ public class ModuleLoader {
         mModuleUseCount--;
         if (mModuleUseCount == 0) {
             mModuleEntryPoint.onDestroy();
+            CrashKeys.getInstance().set(CrashKeyIndex.ACTIVE_DYNAMIC_MODULE, null);
             mModuleEntryPoint = null;
         }
     }
@@ -100,7 +116,7 @@ public class ModuleLoader {
          * @param callback The callback to receive the result of the task. If there was a problem
          *     the result will be null.
          */
-        private LoadClassTask(Context moduleContext, Callback<ModuleEntryPoint> callback) {
+        LoadClassTask(Context moduleContext, Callback<ModuleEntryPoint> callback) {
             mModuleContext = moduleContext;
             mCallback = callback;
         }
@@ -161,6 +177,10 @@ public class ModuleLoader {
                     return;
                 }
 
+                CrashKeys crashKeys = CrashKeys.getInstance();
+                crashKeys.set(CrashKeyIndex.LOADED_DYNAMIC_MODULE, mModuleId);
+                crashKeys.set(CrashKeyIndex.ACTIVE_DYNAMIC_MODULE, mModuleId);
+
                 entryPoint.init(moduleHost);
                 ModuleMetrics.recordLoadResult(ModuleMetrics.LoadResult.SUCCESS_NEW);
                 mModuleEntryPoint = entryPoint;

chrome/android/java_sources.gni

@@ -353,6 +353,7 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/coordinator/CoordinatorLayoutForPointer.java",
   "java/src/org/chromium/chrome/browser/crash/ChromeMinidumpUploadJobService.java",
   "java/src/org/chromium/chrome/browser/crash/ChromeMinidumpUploaderDelegate.java",
+  "java/src/org/chromium/chrome/browser/crash/CrashKeys.java",
   "java/src/org/chromium/chrome/browser/crash/LogcatExtractionRunnable.java",
   "java/src/org/chromium/chrome/browser/crash/MinidumpLogcatPrepender.java",
   "java/src/org/chromium/chrome/browser/crash/MinidumpUploadRetry.java",

chrome/android/javatests/src/org/chromium/chrome/browser/crash/PureJavaExceptionReporterTest.java

@@ -11,6 +11,7 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+import org.chromium.base.ThreadUtils;
 import org.chromium.base.test.BaseJUnit4ClassRunner;
 import org.chromium.components.minidump_uploader.CrashTestRule;
 
@@ -71,6 +72,13 @@ public class PureJavaExceptionReporterTest {
         return sb.toString();
     }
 
+    private void verifyField(String minidumpString, String field) {
+        Assert.assertTrue("Report field \"" + field
+                        + "\" is not included in the report. Minidump string is \"" + minidumpString
+                        + "\"",
+                minidumpString.contains(field));
+    }
+
     @Test
     @SmallTest
     public void verifyMinidumpContentAndUpload() {
@@ -80,11 +88,9 @@ public class PureJavaExceptionReporterTest {
         String minidumpString = readFileToString(reporter.getMinidumpFile());
 
         for (String field : REPORT_FIELDS) {
-            Assert.assertTrue("Report field \"" + field
-                            + "\" is not included in the report. Minidump string is \""
-                            + minidumpString + "\"",
-                    minidumpString.contains(field));
+            verifyField(minidumpString, field);
         }
+
         // Exception string should be included in the stack trace.
         Assert.assertTrue(minidumpString.contains(EXCEPTION_NAME));
 
@@ -93,4 +99,20 @@ public class PureJavaExceptionReporterTest {
 
         Assert.assertTrue(reporter.reportUploaded());
     }
+
+    @Test
+    @SmallTest
+    public void verifyCrashKeys() {
+        ThreadUtils.runOnUiThreadBlocking(
+                () -> { CrashKeys.getInstance().set(CrashKeyIndex.LOADED_DYNAMIC_MODULE, "foo"); });
+
+        TestPureJavaExceptionReporter reporter = new TestPureJavaExceptionReporter();
+        reporter.createAndUploadReport(new RuntimeException());
+        String minidumpString = readFileToString(reporter.getMinidumpFile());
+
+        Assert.assertTrue(
+                minidumpString.contains(CrashKeys.getKey(CrashKeyIndex.LOADED_DYNAMIC_MODULE)));
+        Assert.assertFalse(
+                minidumpString.contains(CrashKeys.getKey(CrashKeyIndex.ACTIVE_DYNAMIC_MODULE)));
+    }
 }

chrome/browser/BUILD.gn

@@ -2075,6 +2075,8 @@ jumbo_split_static_library("browser") {
       "android/contextualsearch/unhandled_tap_web_contents_observer.cc",
       "android/contextualsearch/unhandled_tap_web_contents_observer.h",
       "android/cookies/cookies_fetcher_util.cc",
+      "android/crash/crash_keys_android.cc",
+      "android/crash/crash_keys_android.h",
       "android/crash/pure_java_exception_handler.cc",
       "android/crash/pure_java_exception_handler.h",
       "android/customtabs/detached_resource_request.cc",
@@ -4530,6 +4532,7 @@ if (is_android) {
       "../android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchTabHelper.java",
       "../android/java/src/org/chromium/chrome/browser/contextualsearch/CtrSuppression.java",
       "../android/java/src/org/chromium/chrome/browser/cookies/CookiesFetcher.java",
+      "../android/java/src/org/chromium/chrome/browser/crash/CrashKeys.java",
       "../android/java/src/org/chromium/chrome/browser/crash/MinidumpUploadService.java",
       "../android/java/src/org/chromium/chrome/browser/crash/PureJavaExceptionHandler.java",
       "../android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java",

chrome/browser/android/crash/crash_keys_android.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 "chrome/browser/android/crash/crash_keys_android.h"
+
+#include "base/android/jni_android.h"
+#include "base/android/jni_string.h"
+#include "components/crash/core/common/crash_key.h"
+#include "jni/CrashKeys_jni.h"
+
+namespace {
+
+using JavaCrashKey = crash_reporter::CrashKeyString<64>;
+
+JavaCrashKey& GetCrashKey(int index) {
+  // See CrashKeys.java for how to add a new crash key.
+  static JavaCrashKey crash_keys[] = {
+      {"loaded_dynamic_module", JavaCrashKey::Tag::kArray},
+      {"active_dynamic_module", JavaCrashKey::Tag::kArray},
+  };
+  static_assert(
+      base::size(crash_keys) == static_cast<size_t>(CrashKeyIndex::NUM_KEYS),
+      "crash_keys out of sync with index enum");
+
+  return crash_keys[index];
+}
+
+}  // namespace
+
+void SetAndroidCrashKey(CrashKeyIndex index, const std::string& value) {
+  JNIEnv* env = base::android::AttachCurrentThread();
+  base::android::ScopedJavaLocalRef<jobject> jinstance =
+      Java_CrashKeys_getInstance(env);
+  Java_CrashKeys_set(env, jinstance, static_cast<jint>(index),
+                     base::android::ConvertUTF8ToJavaString(env, value));
+}
+
+void ClearAndroidCrashKey(CrashKeyIndex index) {
+  JNIEnv* env = base::android::AttachCurrentThread();
+  base::android::ScopedJavaLocalRef<jobject> jinstance =
+      Java_CrashKeys_getInstance(env);
+  Java_CrashKeys_set(env, jinstance, static_cast<jint>(index), nullptr);
+}
+
+void FlushAndroidCrashKeys() {
+  JNIEnv* env = base::android::AttachCurrentThread();
+  base::android::ScopedJavaLocalRef<jobject> jinstance =
+      Java_CrashKeys_getInstance(env);
+  Java_CrashKeys_flushToNative(env, jinstance);
+}
+
+static void JNI_CrashKeys_Set(
+    JNIEnv* env,
+    const base::android::JavaParamRef<jobject>& obj,
+    jint key,
+    const base::android::JavaParamRef<jstring>& value) {
+  if (value.is_null()) {
+    GetCrashKey(key).Clear();
+  } else {
+    GetCrashKey(key).Set(base::android::ConvertJavaStringToUTF8(env, value));
+  }
+}

chrome/browser/android/crash/crash_keys_android.h

+// 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.
+
+#ifndef CHROME_BROWSER_ANDROID_CRASH_CRASH_KEYS_ANDROID_H_
+#define CHROME_BROWSER_ANDROID_CRASH_CRASH_KEYS_ANDROID_H_
+
+#include <string>
+
+// See CrashKeys.java for how to add a new crash key.
+// A Java counterpart will be generated for this enum.
+// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.chrome.browser.crash
+enum class CrashKeyIndex {
+  LOADED_DYNAMIC_MODULE = 0,
+  ACTIVE_DYNAMIC_MODULE,
+  NUM_KEYS
+};
+
+// These methods are only exposed for testing -- normal usage should be from
+// Java.
+void SetAndroidCrashKey(CrashKeyIndex index, const std::string& value);
+void ClearAndroidCrashKey(CrashKeyIndex index);
+void FlushAndroidCrashKeys();
+
+#endif  // CHROME_BROWSER_ANDROID_CRASH_CRASH_KEYS_ANDROID_H_

chrome/browser/android/crash/crash_keys_android_unittest.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 "chrome/browser/android/crash/crash_keys_android.h"
+
+#include "components/crash/core/common/crash_key.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using crash_reporter::GetCrashKeyValue;
+
+class CrashKeysAndroidTest : public testing::Test {
+ public:
+  void SetUp() override {
+    crash_reporter::ResetCrashKeysForTesting();
+    crash_reporter::InitializeCrashKeys();
+  }
+
+  void TearDown() override { crash_reporter::ResetCrashKeysForTesting(); }
+};
+
+TEST_F(CrashKeysAndroidTest, Default) {
+  EXPECT_TRUE(GetCrashKeyValue("loaded_dynamic_module").empty());
+  EXPECT_TRUE(GetCrashKeyValue("active_dynamic_module").empty());
+}
+
+TEST_F(CrashKeysAndroidTest, SetAndClear) {
+  SetAndroidCrashKey(CrashKeyIndex::LOADED_DYNAMIC_MODULE, "foobar");
+  SetAndroidCrashKey(CrashKeyIndex::ACTIVE_DYNAMIC_MODULE, "blurp");
+  EXPECT_TRUE(GetCrashKeyValue("loaded_dynamic_module").empty());
+  EXPECT_TRUE(GetCrashKeyValue("active_dynamic_module").empty());
+
+  ClearAndroidCrashKey(CrashKeyIndex::ACTIVE_DYNAMIC_MODULE);
+  FlushAndroidCrashKeys();
+  EXPECT_EQ(GetCrashKeyValue("loaded_dynamic_module"), "foobar");
+  EXPECT_TRUE(GetCrashKeyValue("active_dynamic_module").empty());
+
+  ClearAndroidCrashKey(CrashKeyIndex::LOADED_DYNAMIC_MODULE);
+  EXPECT_TRUE(GetCrashKeyValue("loaded_dynamic_module").empty());
+  EXPECT_TRUE(GetCrashKeyValue("active_dynamic_module").empty());
+}

chrome/test/BUILD.gn

@@ -2350,6 +2350,7 @@ test("unit_tests") {
 
     # TODO(newt): move this to test_support_unit?
     "../browser/android/chrome_backup_agent_unittest.cc",
+    "../browser/android/crash/crash_keys_android_unittest.cc",
     "../browser/android/customtabs/detached_resource_request_unittest.cc",
     "../browser/android/favicon_helper_unittest.cc",
     "../browser/android/mock_location_settings.cc",