OfflineContentProvider changes to start service

chrome/android/java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotifierFactory.java

Index: chrome/android/java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotifierFactory.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotifierFactory.java b/chrome/android/java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotifierFactory.java
new file mode 100644
index 0000000000000000000000000000000000000000..9968d9f111f79efcc6c5e7f875e6e1656f5f7ebf
--- /dev/null
+++ b/chrome/android/java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotifierFactory.java
@@ -0,0 +1,50 @@
+// Copyright 2017 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.download.items;
+
+import android.content.Context;
+
+import org.chromium.chrome.browser.download.items.OfflineContentAggregatorNotifier.NotifierUi;
+import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.components.offline_items_collection.OfflineContentProvider;
+
+/**
+ * A factory helper to create and hold a {@link OfflineContentAggregatorNotifier}.  This will glue
+ * the {@link Profile} owned {@link OfflineContentProvider} to the
+ * {@link org.chromium.chrome.browser.download.DownloadNotificationService}.
+ */
+public class OfflineContentAggregatorNotifierFactory {

[David Trainor- moved to gerrit, 2017/03/18 00:45:41]

I eventually need to have a better story for how we build the Java bridge and
hook it up to the UI.  If nothing from Java tries to access the
OfflineContentAggregatorFactory the bridge isn't built.  This glue needs to be
created from either the DownloadNotificationService or an Activity (both can
start and spin up native).

I think this is okay in the short term until the rest of the bridge gets built
out and the story gets solidified.

[gone, 2017/03/20 19:03:36]

Does it make sense to put this in DeferredStartupHandler somewhere?

+    private static OfflineContentAggregatorNotifier sNotifier;
+
+    /**
+     * Creates a {@link OfflineContentAggregatorNotifier} and connects it to the UI and the
+     * underlying {@link OfflineContentProvider}.  Save to call more than once.  Will be a NOOP if
+     * the notifier is already created.
+     * @param context

[gone, 2017/03/20 19:03:36]

fill this in

[David Trainor- moved to gerrit, 2017/03/25 03:31:13]

Done.

+     */
+    public static void create(Context context) {
+        if (sNotifier != null) return;

[gone, 2017/03/20 19:03:36]

Should synchronize on an Object to prevent this from being created twice. 
You've got no guards against what thread is actually being used to call this.

[David Trainor- moved to gerrit, 2017/03/25 03:31:13]

Put an assert check for UI thread.  LMK if you'd like more strict requirements.

+
+        Profile profile = Profile.getLastUsedProfile();
+        OfflineContentProvider provider = OfflineContentAggregatorFactory.forProfile(profile);
+        NotifierUi ui = new DownloadNotificationServiceNotifierUi(context);
+
+        sNotifier = new OfflineContentAggregatorNotifier(provider, ui);
+    }
+
+    /**
+     * Destroys the internal {@link OfflineContentAggregatorNotifier}.  Safe to call more than once.
+     * Will be a NOOP if called when there is no notifier built.
+     * TODO(dtrainor): Currently we have no path to destroy this, given the various ways Chrome can
+     * be started and stopped.  It's fine to leave this running during the duration of the process
+     * though.
+     */
+    public static void destroy() {
+        if (sNotifier == null) return;

[gone, 2017/03/20 19:03:36]

should probably also synchronize here

+
+        sNotifier.destroy();
+        sNotifier = null;
+    }
+}