[Suggestions] Enforce destruction order for the SnippetsBridge

Make the SuggestionsSource be destroyed after all the other
DestructionObservers to ensure they are still able to use it
while they are being destroyed.

Bug: 742056,738872
Change-Id: I36c299297c647c5fb9e680d49d5912a4469f02da
Reviewed-on: https://chromium-review.googlesource.com/570298
Commit-Queue: Nicolas Dossou-Gbété <dgn@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486720}

chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsBridge.java

@@ -53,12 +53,8 @@ public class SnippetsBridge implements SuggestionsSource {
         mNativeSnippetsBridge = nativeInit(profile);
     }
 
-    /**
-     * Destroys the native bridge. This object can no longer be used to send native commands, and
-     * any observer is nulled out and will stop receiving updates. This object should be discarded.
-     */
     @Override
-    public void onDestroy() {
+    public void destroy() {
         assert mNativeSnippetsBridge != 0;
         nativeDestroy(mNativeSnippetsBridge);
         mNativeSnippetsBridge = 0;
@@ -105,6 +101,7 @@ public class SnippetsBridge implements SuggestionsSource {
 
     @Override
     public void fetchRemoteSuggestions() {
+        assert mNativeSnippetsBridge != 0;
         nativeReloadSuggestions(mNativeSnippetsBridge);
     }
 
@@ -182,6 +179,7 @@ public class SnippetsBridge implements SuggestionsSource {
     @Override
     public void fetchSuggestions(@CategoryInt int category, String[] displayedSuggestionIds,
             Callback<List<SnippetArticle>> callback) {
+        assert mNativeSnippetsBridge != 0;
         nativeFetch(mNativeSnippetsBridge, category, displayedSuggestionIds, callback);
     }
 

chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SuggestionsSource.java

@@ -8,14 +8,13 @@ import android.graphics.Bitmap;
 
 import org.chromium.base.Callback;
 import org.chromium.chrome.browser.ntp.cards.SuggestionsCategoryInfo;
-import org.chromium.chrome.browser.suggestions.DestructionObserver;
 
 import java.util.List;
 
 /**
  * An interface for classes that provide content suggestions.
  */
-public interface SuggestionsSource extends DestructionObserver {
+public interface SuggestionsSource {
     /**
      * An observer for events in the content suggestions service.
      */
@@ -37,6 +36,12 @@ public interface SuggestionsSource extends DestructionObserver {
         void onFullRefreshRequired();
     }
 
+    /**
+     * Destroys the resources associated with the source and all observers will be unregistered.
+     * It should not be used after this is called.
+     */
+    void destroy();
+
     /**
      * Fetches new snippets for all remote categories.
      */

chrome/android/java/src/org/chromium/chrome/browser/suggestions/SuggestionsUiDelegate.java

@@ -39,7 +39,9 @@ public interface SuggestionsUiDelegate {
     // Feature/State checks
 
     /**
-     * Registers a {@link DestructionObserver}, notified when the New Tab Page goes away.
+     * Registers a {@link DestructionObserver}, notified when the delegate's host goes away. It is
+     * guaranteed that the observer will be called before the {@link SuggestionsSource} is
+     * destroyed, but there is no destruction order guarantee otherwise.
      */
     void addDestructionObserver(DestructionObserver destructionObserver);
 

chrome/android/java/src/org/chromium/chrome/browser/suggestions/SuggestionsUiDelegateImpl.java

@@ -42,8 +42,6 @@ public class SuggestionsUiDelegateImpl implements SuggestionsUiDelegate {
 
         mHost = host;
         mReferencePool = referencePool;
-
-        addDestructionObserver(mSuggestionsSource);
     }
 
     @Override
@@ -96,6 +94,11 @@ public class SuggestionsUiDelegateImpl implements SuggestionsUiDelegate {
 
         for (DestructionObserver observer : mDestructionObservers) observer.onDestroy();
 
+        // SuggestionsSource is not registered with the rest of the destruction observers but
+        // instead explicitly destroyed last so that the other destruction observers can use it
+        // while they are called.
+        mSuggestionsSource.destroy();
+
         mIsDestroyed = true;
     }
 }

chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/suggestions/FakeSuggestionsSource.java

@@ -234,7 +234,7 @@ public class FakeSuggestionsSource implements SuggestionsSource {
     }
 
     @Override
-    public void onDestroy() {}
+    public void destroy() {}
 
     @Override
     public int[] getCategories() {