Add a file observer to monitor if there is a crash

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

Index: chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpDirectoryObserver.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpDirectoryObserver.java b/chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpDirectoryObserver.java
new file mode 100644
index 0000000000000000000000000000000000000000..8c256b00726255f4864eda309818cd7b2465087e
--- /dev/null
+++ b/chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpDirectoryObserver.java
@@ -0,0 +1,57 @@
+// Copyright 2016 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.content.Context;
+import android.content.Intent;
+import android.os.FileObserver;
+
+import org.chromium.base.ApplicationStatus;
+import org.chromium.base.Log;
+import org.chromium.base.ThreadUtils;
+import org.chromium.base.metrics.RecordUserAction;
+
+import java.io.File;
+
+/**
+ * This class is a singleton that holds utilities for observer to monitor the minidump directory.
+ */
+public final class MinidumpDirectoryObserver extends FileObserver {
+
+    private static final String TAG = "MinidumpDirObserver";
+
+    private static MinidumpDirectoryObserver sMinidumpDirectoryObserver;
+    private static Context sContext = ApplicationStatus.getApplicationContext();
+
+    public static final String MINIDUMP_EXPERIMENT_NAME = "AddMinidumpDirObserver";
+
+    public static void startObserverWatching() {
+        ThreadUtils.assertOnUiThread();

[Yaron, 2016/02/01 20:22:40]

why? Do you just need a stable thread to avoid locking?

[Menglin, 2016/02/02 02:11:29]

Done.

[Yaron, 2016/02/03 21:51:42]

Does this really need to be a singleton and have the overhead of the various
statics? Why can't you just new it up once and let it run?

[Menglin, 2016/02/04 00:17:23]

I was not sure so I manually tested on both. 

case1: not a singleton
step1: open the browser, induce renderer crash
step2: open a second tab, induce renderer crash
what to expect: detects a minidump after step1 and another minidump after step2
what happened: detects a minidump after step1, but wasn't able to detect the
second minidump after step2

case2: singleton
same steps as above.
what happened: was able to detect each minidump.

I suspect the file observer some how "died" with the child process if it's not a
singleton, so it was able to detect the following minidumps

[Yaron, 2016/02/04 01:50:26]

IT really shouldn't. Can you upload a patchset with the code? Were you maintain
a reference to the instance of MinidumpDirectoryObserver cause if not, it would
get GC'd since FileObserver only maintains a weak reference. That would mean
it's just racey. I really would prefer clear ownership though to this being
static

[Menglin, 2016/02/04 21:54:45]

Now MinidumpDirectoryObserver is not a singleton, but the instance is maintained
in 
ChromeBrowserInitializer. I tested it, and it worked out correctly. 
Thank you for your comment!

+        if (sMinidumpDirectoryObserver == null) {
+            sMinidumpDirectoryObserver = new MinidumpDirectoryObserver();
+        }
+        sMinidumpDirectoryObserver.startWatching();
+    }
+
+    private MinidumpDirectoryObserver() {
+        // The file observer detects MOVED_TO for child processes.
+        super(new File(sContext.getCacheDir(), CrashFileManager.CRASH_DUMP_DIR).toString(),

[Yaron, 2016/02/01 20:22:40]

This can cause a StrictMode violation since internally it'll do file.exists and
create it if it didn't exist previously.

[Menglin, 2016/02/02 02:11:29]

I removed the assertOnUiThread(), does that resolve the violation?

[Yaron, 2016/02/03 21:51:42]

No, because it's still called from the UI thread. The call to getCacheDir needs
to not be done from the UI Thread. I think you can use
PathUtils.getCacheDirectory instead.

[Menglin, 2016/02/04 00:17:23]

Done.

+                FileObserver.MOVED_TO);

[Yaron, 2016/02/01 20:22:40]

Is this CL description out of date? I don't see a mention of create

[Menglin, 2016/02/02 02:11:29]

Done.

+    }
+
+    /**
+     *  When a miniudump is detected, upload it to Google crash server

[Yaron, 2016/02/01 20:22:40]

minidump

[Menglin, 2016/02/02 02:11:30]

Done.

+     */
+    @Override
+    public void onEvent(int event, String path) {
+        // This is happening on an off-thread.

[Yaron, 2016/02/01 20:22:40]

What's an "off-thread"? How about:
"This is executed on a thread dedicated to FileObserver"

[Menglin, 2016/02/02 02:11:30]

Done.

+        if (CrashFileManager.isMinidumpMIMEFirstTry(path)) {
+            Intent intent = MinidumpUploadService.createFindAndUploadLastCrashIntent(sContext);
+            sContext.startService(intent);

[Yaron, 2016/02/01 20:22:40]

You should try/catch this for a security exception, same as the previous
location

[Menglin, 2016/02/02 02:11:30]

Done.

+            Log.i(TAG, "Detects a new minidump %s sending intent to MinidumpUploadService", path);
+            RecordUserAction.record("MobileBreakpadUploadAttempt");
+        }
+    }
+}