Bug 1348273 - Convert the Android-specific annotations and use the machine-generated whitelist in the crash reporter; r?jchen draft
authorGabriele Svelto <gsvelto@mozilla.com>
Fri, 16 Mar 2018 23:15:50 +0100
changeset 781399 c14726b2d1cf63b2a52f37de30682227742a659a
parent 781398 e06b2524a7591a923ca7e6b0782e30af40ac5cfc
child 781400 79ffbdccdf6e2ca5298dfcc33f9e089e9c9bc46b
push id106292
push usergsvelto@mozilla.com
push dateThu, 12 Apr 2018 22:06:23 +0000
reviewersjchen
bugs1348273
milestone61.0a1
Bug 1348273 - Convert the Android-specific annotations and use the machine-generated whitelist in the crash reporter; r?jchen MozReview-Commit-ID: 7A5bmBdBQ08
mobile/android/base/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetryCrashPingBuilder.java
mobile/android/base/moz.build
mobile/android/components/SessionStore.js
toolkit/crashreporter/CrashAnnotations.yaml
toolkit/crashreporter/generate_crash_reporter_sources.py
widget/android/jni/Utils.cpp
widget/android/nsAppShell.cpp
--- a/mobile/android/base/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetryCrashPingBuilder.java
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetryCrashPingBuilder.java
@@ -3,16 +3,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
 package org.mozilla.gecko.telemetry.pingbuilders;
 
 import android.util.Log;
 
+import org.mozilla.gecko.CrashReporterConstants;
 import org.mozilla.gecko.sync.ExtendedJSONObject;
 import org.mozilla.gecko.sync.NonObjectJSONException;
 import org.mozilla.gecko.util.StringUtils;
 
 import java.io.IOException;
 import java.text.DateFormat;
 import java.text.SimpleDateFormat;
 import java.util.Arrays;
@@ -30,49 +31,16 @@ import java.util.TimeZone;
 public class TelemetryCrashPingBuilder extends TelemetryPingBuilder {
     private static final String LOGTAG = "GeckoTelemetryCrashPingBuilder";
 
     private static final int PING_VERSION = 1;
 
     private static final String ISO8601_DATE = "yyyy-MM-dd";
     private static final String ISO8601_DATE_HOURS = "yyyy-MM-dd'T'HH':00:00.000Z'";
 
-    // The following list should be kept in sync with the one in CrashManager.jsm
-    private static final String[] ANNOTATION_WHITELIST = {
-        "AsyncShutdownTimeout",
-        "AvailablePageFile",
-        "AvailablePhysicalMemory",
-        "AvailableVirtualMemory",
-        "BlockedDllList",
-        "BlocklistInitFailed",
-        "BuildID",
-        "ContainsMemoryReport",
-        "CrashTime",
-        "EventLoopNestingLevel",
-        "ipc_channel_error",
-        "IsGarbageCollecting",
-        "MozCrashReason",
-        "OOMAllocationSize",
-        "ProductID",
-        "ProductName",
-        "ReleaseChannel",
-        "RemoteType",
-        "SecondsSinceLastCrash",
-        "ShutdownProgress",
-        "StartupCrash",
-        "SystemMemoryUsePercentage",
-        "TextureUsage",
-        "TotalPageFile",
-        "TotalPhysicalMemory",
-        "TotalVirtualMemory",
-        "UptimeTS",
-        "User32BeforeBlocklist",
-        "Version",
-    };
-
     public TelemetryCrashPingBuilder(String crashId, String clientId, HashMap<String, String> annotations) {
         super(TelemetryPingBuilder.UNIFIED_TELEMETRY_VERSION);
 
         payload.put("type", "crash");
         payload.put("id", docID);
         payload.put("version", TelemetryPingBuilder.UNIFIED_TELEMETRY_VERSION);
         payload.put("creationDate", currentDate(ISO8601_DATE_HOURS));
         payload.put("clientId", clientId);
@@ -157,17 +125,17 @@ public class TelemetryCrashPingBuilder e
      *
      * @param annotations A map holding the crash annotations
      * @returns A JSON object representing the ping's metadata node
      */
     private static ExtendedJSONObject createMetadataNode(HashMap<String, String> annotations) {
         ExtendedJSONObject node = new ExtendedJSONObject();
 
         for (Entry<String, String> pair : annotations.entrySet()) {
-            if (Arrays.binarySearch(ANNOTATION_WHITELIST, pair.getKey()) >= 0) {
+            if (Arrays.binarySearch(CrashReporterConstants.ANNOTATION_WHITELIST, pair.getKey()) >= 0) {
                 node.put(pair.getKey(), pair.getValue());
             }
         }
 
         return node;
     }
 
     /**
--- a/mobile/android/base/moz.build
+++ b/mobile/android/base/moz.build
@@ -111,31 +111,37 @@ with Files('../app/src/*/res/menu/browse
 
 DEFINES['ANDROID_PACKAGE_NAME'] = CONFIG['ANDROID_PACKAGE_NAME']
 FINAL_TARGET_PP_FILES += ['package-name.txt.in']
 
 GENERATED_FILES += [
     'AndroidManifest.xml',
     'generated/preprocessed/org/mozilla/gecko/AdjustConstants.java',
     'generated/preprocessed/org/mozilla/gecko/AppConstants.java',
+    'generated/preprocessed/org/mozilla/gecko/CrashReporterConstants.java',
     'generated/preprocessed/org/mozilla/gecko/MmaConstants.java',
 ]
 x = GENERATED_FILES['generated/preprocessed/org/mozilla/gecko/AdjustConstants.java']
 x.script = 'generate_build_config.py:generate_java'
 x.inputs += ['AdjustConstants.java.in']
 y = GENERATED_FILES['generated/preprocessed/org/mozilla/gecko/AppConstants.java']
 y.script = 'generate_build_config.py:generate_java'
 y.inputs += ['AppConstants.java.in']
 y = GENERATED_FILES['generated/preprocessed/org/mozilla/gecko/MmaConstants.java']
 y.script = 'generate_build_config.py:generate_java'
 y.inputs += ['MmaConstants.java.in']
 z = GENERATED_FILES['AndroidManifest.xml']
 z.script = 'generate_build_config.py:generate_android_manifest'
 z.inputs += ['AndroidManifest.xml.in']
 
+# Generate CrashReporterConstants.java
+crash_reporter_constants = GENERATED_FILES['generated/preprocessed/org/mozilla/gecko/CrashReporterConstants.java']
+crash_reporter_constants.script = '/toolkit/crashreporter/generate_crash_reporter_sources.py:emit_class'
+crash_reporter_constants.inputs += ['/toolkit/crashreporter/CrashAnnotations.yaml']
+
 # Regular builds invoke `libs` targets that localize files with no AB_CD set
 # into the default resources (res/{values,raw}).
 #
 # Multi-locale builds invoke `chrome-%` targets that localize files into
 # locale-specific resources (res/{values,raw}-AB-rCD).  Single-locale repacks
 # invoke `libs AB_CD=$*` targets that localize files into the default resources
 # (res/{values,raw}).
 #
--- a/mobile/android/components/SessionStore.js
+++ b/mobile/android/components/SessionStore.js
@@ -1321,17 +1321,18 @@ SessionStore.prototype = {
       let currentURI = aWindow.BrowserApp.selectedBrowser.currentURI;
       // if the current URI contains a username/password, remove it
       try {
         currentURI = currentURI.mutate()
                                .setUserPass("")
                                .finalize();
       } catch (ex) { } // ignore failures on about: URIs
 
-      Services.appinfo.annotateCrashReport("URL", currentURI.spec);
+      Services.appinfo.annotateCrashReport(Services.appinfo.URL,
+                                           currentURI.spec);
     } catch (ex) {
       // don't make noise when crashreporter is built but not enabled
       if (ex.result != Cr.NS_ERROR_NOT_INITIALIZED) {
         Cu.reportError("SessionStore:" + ex);
       }
     }
   },
 
--- a/toolkit/crashreporter/CrashAnnotations.yaml
+++ b/toolkit/crashreporter/CrashAnnotations.yaml
@@ -301,16 +301,23 @@ IPCTransportFailureReason:
 
 IsGarbageCollecting:
   description: >
     If true then the JavaScript garbage collector was running when the crash
     occurred.
   type: boolean
   ping: true
 
+
+JavaStackTrace:
+  description: >
+    Java stack trace, only present on Firefox for Android if we encounter an
+    uncaught Java exception.
+  type: string
+
 MarshalActCtxManifestPath:
   description: >
     Proxy stream marshalling current activation context manifest path.
   type: string
 
 MozCrashReason:
   description: >
     Plaintext description of why Firefox crashed, this is usually set by
--- a/toolkit/crashreporter/generate_crash_reporter_sources.py
+++ b/toolkit/crashreporter/generate_crash_reporter_sources.py
@@ -169,8 +169,65 @@ def emit_idl(output, template_filename, 
     generated_idl = generate_idl(template, annotations)
 
     try:
         output.write(generated_idl)
 
     except IOError as ex:
         print("Error while writing out the generated file:\n" + str(ex) + "\n")
         sys.exit(1)
+
+###############################################################################
+# Java code generation                                                        #
+###############################################################################
+
+
+def generate_java_array_initializer(contents):
+    """Generates the initializer for an array of strings.
+    Effectively turns `["a", "b"]` into '   \"a\",\n   \"b\",\n'."""
+
+    initializer = ""
+
+    for name in contents:
+        initializer += "        \"" + name + "\",\n"
+
+    return initializer.strip(",\n")
+
+
+def generate_class(template, annotations):
+    """Fill the class template from the list of annotations."""
+
+    whitelist = extract_crash_ping_whitelist(annotations)
+
+    return "/* This file was autogenerated by " + \
+           "toolkit/crashreporter/generate_crash_reporter_sources.py. " + \
+           "DO NOT EDIT */\n\n" + \
+           string.Template(template).substitute({
+               "whitelist": generate_java_array_initializer(whitelist),
+           })
+
+
+def emit_class(output, annotations_filename):
+    """Generate the CrashReporterConstants.java file."""
+
+    template = "package org.mozilla.gecko;\n" \
+               "\n" \
+               "/**\n" \
+               " * Constants used by the crash reporter. These are\n" \
+               " * generated so that they are kept in sync with the other\n" \
+               " * C++ and JS users.\n" \
+               " */\n" \
+               "\n" \
+               "public class CrashReporterConstants {\n" \
+               "    public static final String[] ANNOTATION_WHITELIST = {\n" \
+               "${whitelist}\n" \
+               "    };\n" \
+               "}\n"
+
+    annotations = read_annotations(annotations_filename)
+    generated_class = generate_class(template, annotations)
+
+    try:
+        output.write(generated_class)
+
+    except IOError as ex:
+        print("Error while writing out the generated file:\n" + str(ex) + "\n")
+        sys.exit(1)
--- a/widget/android/jni/Utils.cpp
+++ b/widget/android/jni/Utils.cpp
@@ -199,18 +199,18 @@ bool HandleUncaughtException(JNIEnv* aEn
     return true;
 }
 
 bool ReportException(JNIEnv* aEnv, jthrowable aExc, jstring aStack)
 {
     bool result = true;
 
     result &= NS_SUCCEEDED(CrashReporter::AnnotateCrashReport(
-            NS_LITERAL_CSTRING("JavaStackTrace"),
-            String::Ref::From(aStack)->ToCString()));
+                           CrashReporter::Annotation::JavaStackTrace,
+                           String::Ref::From(aStack)->ToCString()));
 
     auto appNotes = java::GeckoAppShell::GetAppNotes();
     if (NS_WARN_IF(aEnv->ExceptionCheck())) {
         aEnv->ExceptionDescribe();
         aEnv->ExceptionClear();
     } else if (appNotes) {
         CrashReporter::AppendAppNotesToCrashReport(NS_LITERAL_CSTRING("\n") +
                                                    appNotes->ToCString());
--- a/widget/android/nsAppShell.cpp
+++ b/widget/android/nsAppShell.cpp
@@ -4,23 +4,21 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsAppShell.h"
 
 #include "base/basictypes.h"
 #include "base/message_loop.h"
 #include "base/task.h"
 #include "mozilla/Hal.h"
-#include "nsExceptionHandler.h"
 #include "nsIScreen.h"
 #include "nsIScreenManager.h"
 #include "nsWindow.h"
 #include "nsThreadUtils.h"
 #include "nsICommandLineRunner.h"
-#include "nsICrashReporter.h"
 #include "nsIObserverService.h"
 #include "nsIAppStartup.h"
 #include "nsIGeolocationProvider.h"
 #include "nsCacheService.h"
 #include "nsIDOMEventListener.h"
 #include "nsIDOMWakeLockListener.h"
 #include "nsIPowerManagerService.h"
 #include "nsISpeculativeConnect.h"