[Suggestions UI] Drop Bitmap references from articles under memory pressure.

BUG=718925

Review-Url: https://codereview.chromium.org/2865963003
Cr-Commit-Position: refs/heads/master@{#471156}

base/BUILD.gn

@@ -2508,6 +2508,7 @@ if (is_android) {
       "android/java/src/org/chromium/base/ContentUriUtils.java",
       "android/java/src/org/chromium/base/ContextUtils.java",
       "android/java/src/org/chromium/base/CpuFeatures.java",
+      "android/java/src/org/chromium/base/DiscardableReferencePool.java",
       "android/java/src/org/chromium/base/EarlyTraceEvent.java",
       "android/java/src/org/chromium/base/EventLog.java",
       "android/java/src/org/chromium/base/FieldTrialList.java",
@@ -2666,6 +2667,7 @@ if (is_android) {
   junit_binary("base_junit_tests") {
     java_files = [
       "android/junit/src/org/chromium/base/BaseChromiumApplicationTest.java",
+      "android/junit/src/org/chromium/base/DiscardableReferencePoolTest.java",
       "android/junit/src/org/chromium/base/LogTest.java",
       "android/junit/src/org/chromium/base/NonThreadSafeTest.java",
       "android/junit/src/org/chromium/base/PromiseTest.java",

base/android/java/src/org/chromium/base/DiscardableReferencePool.java

+// 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.base;
+
+import android.support.annotation.Nullable;
+
+import java.util.Collections;
+import java.util.Set;
+import java.util.WeakHashMap;
+
+/**
+ * A DiscardableReferencePool allows handing out typed references to objects ("payloads") that can
+ * be dropped in one batch ("drained"), e.g. under memory pressure. In contrast to {@link
+ * java.lang.ref.WeakReference}s, which drop their referents when they get garbage collected, a
+ * reference pool gives more precise control over when exactly it is drained.
+ *
+ * <p>Internally it uses a {@link WeakHashMap} with the reference itself as a key to allow the
+ * payloads to be garbage collected regularly when the last reference goes away before the pool is
+ * drained.
+ *
+ * <p>This class and its references are not thread-safe and should not be used simultaneously by
+ * multiple threads.
+ */
+public class DiscardableReferencePool {
+    /**
+     * The underlying data storage. The wildcard type parameter allows using a single pool for
+     * references of any type.
+     */
+    private final Set<DiscardableReference<?>> mPool;
+
+    public DiscardableReferencePool() {
+        WeakHashMap<DiscardableReference<?>, Boolean> map = new WeakHashMap<>();
+        mPool = Collections.newSetFromMap(map);
+    }
+
+    /**
+     * A reference to an object in the pool. Will be nulled out when the pool is drained.
+     * @param <T> The type of the object.
+     */
+    public static class DiscardableReference<T> {
+        @Nullable
+        private T mPayload;
+
+        private DiscardableReference(T payload) {
+            assert payload != null;
+            mPayload = payload;
+        }
+
+        /**
+         * @return The referent, or null if the pool has been drained.
+         */
+        @Nullable
+        public T get() {
+            return mPayload;
+        }
+
+        /**
+         * Clear the referent.
+         */
+        private void discard() {
+            assert mPayload != null;
+            mPayload = null;
+        }
+    }
+
+    /**
+     * @param <T> The type of the object.
+     * @param payload The payload to add to the pool.
+     * @return A new reference to the {@code payload}.
+     */
+    public <T> DiscardableReference<T> put(T payload) {
+        assert payload != null;
+        DiscardableReference<T> reference = new DiscardableReference<>(payload);
+        mPool.add(reference);
+        return reference;
+    }
+
+    /**
+     * Drains the pool, removing all references to objects in the pool and therefore allowing them
+     * to be garbage collected.
+     */
+    public void drain() {
+        for (DiscardableReference<?> ref : mPool) {
+            ref.discard();
+        }
+        mPool.clear();
+    }
+}

base/android/junit/src/org/chromium/base/DiscardableReferencePoolTest.java

+// 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.base;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.annotation.Config;
+
+import org.chromium.base.DiscardableReferencePool.DiscardableReference;
+import org.chromium.testing.local.LocalRobolectricTestRunner;
+
+import java.lang.ref.WeakReference;
+
+/**
+ * Tests for {@link DiscardableReferencePool}.
+ */
+@RunWith(LocalRobolectricTestRunner.class)
+@Config(manifest = Config.NONE)
+public class DiscardableReferencePoolTest {
+    /**
+     * Tests that draining the pool clears references and allows objects to be garbage collected.
+     */
+    @Test
+    public void testDrain() {
+        DiscardableReferencePool pool = new DiscardableReferencePool();
+
+        Object object = new Object();
+        WeakReference<Object> weakReference = new WeakReference<>(object);
+
+        DiscardableReference<Object> discardableReference = pool.put(object);
+        Assert.assertEquals(object, discardableReference.get());
+
+        // Drop reference to the object itself, to allow it to be garbage-collected.
+        object = null;
+
+        pool.drain();
+
+        // The discardable reference should be null now.
+        Assert.assertNull(discardableReference.get());
+
+        // The object is not (strongly) reachable anymore, so the weak reference may or may not be
+        // null (it could be if a GC has happened since the pool was drained).
+        // After an explicit GC call it definitely should be null.
+        Runtime.getRuntime().gc();
+
+        Assert.assertNull(weakReference.get());
+    }
+
+    /**
+     * Tests that dropping the (last) discardable reference to an object allows it to be regularly
+     * garbage collected.
+     */
+    @Test
+    public void testReferenceGCd() {
+        DiscardableReferencePool pool = new DiscardableReferencePool();
+
+        Object object = new Object();
+        WeakReference<Object> weakReference = new WeakReference<>(object);
+
+        DiscardableReference<Object> discardableReference = pool.put(object);
+        Assert.assertEquals(object, discardableReference.get());
+
+        // Drop reference to the object itself and to the discardable reference, allowing the object
+        // to be garbage-collected.
+        object = null;
+        discardableReference = null;
+
+        // The object is not (strongly) reachable anymore, so the weak reference may or may not be
+        // null (it could be if a GC has happened since the pool was drained).
+        // After an explicit GC call it definitely should be null.
+        Runtime.getRuntime().gc();
+
+        Assert.assertNull(weakReference.get());
+    }
+}

chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java

@@ -46,6 +46,7 @@ import org.chromium.base.BaseSwitches;
 import org.chromium.base.Callback;
 import org.chromium.base.CommandLine;
 import org.chromium.base.ContextUtils;
+import org.chromium.base.DiscardableReferencePool;
 import org.chromium.base.SysUtils;
 import org.chromium.base.TraceEvent;
 import org.chromium.base.VisibleForTesting;
@@ -272,6 +273,8 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
     private int mScreenWidthDp;
     private Runnable mRecordMultiWindowModeScreenWidthRunnable;
 
+    private final DiscardableReferencePool mReferencePool = new DiscardableReferencePool();
+
     private AssistStatusHandler mAssistStatusHandler;
 
     // A set of views obscuring all tabs. When this set is nonempty,
@@ -1778,6 +1781,16 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
         super.onBackPressed();
     }
 
+    @Override
+    public void onTrimMemory(int level) {
+        super.onTrimMemory(level);
+        // The conditions are expressed using ranges to capture intermediate levels possibly added
+        // to the API in the future.
+        if ((level >= TRIM_MEMORY_RUNNING_LOW && level < TRIM_MEMORY_UI_HIDDEN)
+                || level >= TRIM_MEMORY_MODERATE) {
+            mReferencePool.drain();
+        }
+    }
 
     private ContentViewCore getContentViewCore() {
         Tab tab = getActivityTab();
@@ -2203,4 +2216,11 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
     public boolean supportsFullscreenActivity() {
         return false;
     }
+
+    /**
+     * @return the reference pool for this activity.
+     */
+    public DiscardableReferencePool getReferencePool() {
+        return mReferencePool;
+    }
 }

chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java

@@ -18,6 +18,7 @@ import android.view.View;
 
 import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.base.CommandLine;
+import org.chromium.base.DiscardableReferencePool;
 import org.chromium.base.Log;
 import org.chromium.base.TraceEvent;
 import org.chromium.base.VisibleForTesting;
@@ -178,8 +179,9 @@ public class NewTabPage
         public NewTabPageManagerImpl(SuggestionsSource suggestionsSource,
                 SuggestionsEventReporter eventReporter,
                 SuggestionsNavigationDelegate navigationDelegate, Profile profile,
-                NativePageHost nativePageHost) {
-            super(suggestionsSource, eventReporter, navigationDelegate, profile, nativePageHost);
+                NativePageHost nativePageHost, DiscardableReferencePool referencePool) {
+            super(suggestionsSource, eventReporter, navigationDelegate, profile, nativePageHost,
+                    referencePool);
         }
 
         @Override
@@ -322,8 +324,8 @@ public class NewTabPage
         SuggestionsNavigationDelegateImpl navigationDelegate =
                 new SuggestionsNavigationDelegateImpl(
                         activity, profile, nativePageHost, tabModelSelector);
-        mNewTabPageManager = new NewTabPageManagerImpl(
-                mSnippetsBridge, eventReporter, navigationDelegate, profile, nativePageHost);
+        mNewTabPageManager = new NewTabPageManagerImpl(mSnippetsBridge, eventReporter,
+                navigationDelegate, profile, nativePageHost, activity.getReferencePool());
         mTileGroupDelegate = new NewTabPageTileGroupDelegate(
                 activity, profile, tabModelSelector, navigationDelegate);
 

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

@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.ntp.snippets;
 import android.graphics.Bitmap;
 import android.support.annotation.Nullable;
 
+import org.chromium.base.DiscardableReferencePool.DiscardableReference;
 import org.chromium.chrome.browser.suggestions.OfflinableSuggestion;
 
 import java.io.File;
@@ -51,7 +52,7 @@ public class SnippetArticle implements OfflinableSuggestion {
     private int mGlobalRank = -1;
 
     /** Bitmap of the thumbnail, fetched lazily, when the RecyclerView wants to show the snippet. */
-    private Bitmap mThumbnailBitmap;
+    private DiscardableReference<Bitmap> mThumbnailBitmap;
 
     /** Stores whether impression of this article has been tracked already. */
     private boolean mImpressionTracked;
@@ -108,11 +109,11 @@ public class SnippetArticle implements OfflinableSuggestion {
      * initially unset.
      */
     public Bitmap getThumbnailBitmap() {
-        return mThumbnailBitmap;
+        return mThumbnailBitmap == null ? null : mThumbnailBitmap.get();
     }
 
     /** Sets the thumbnail bitmap for this article. */
-    public void setThumbnailBitmap(Bitmap bitmap) {
+    public void setThumbnailBitmap(DiscardableReference<Bitmap> bitmap) {
         mThumbnailBitmap = bitmap;
     }
 

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

@@ -357,7 +357,7 @@ public class SnippetArticleViewHolder extends CardViewHolder implements Impressi
             mArticle.setThumbnailBitmap(null);
             Bitmap thumbnail = mThumbnailProvider.getThumbnail(mImageCallback);
             if (thumbnail == null || thumbnail.isRecycled()) return;
-            mArticle.setThumbnailBitmap(thumbnail);
+            mArticle.setThumbnailBitmap(mUiDelegate.getReferencePool().put(thumbnail));
             setThumbnailFromBitmap(thumbnail);
 
             return;
@@ -372,8 +372,9 @@ public class SnippetArticleViewHolder extends CardViewHolder implements Impressi
 
         // mThumbnailView's visibility is modified in updateLayout().
         if (mThumbnailView.getVisibility() != View.VISIBLE) return;
-        if (mArticle.getThumbnailBitmap() != null && !mArticle.getThumbnailBitmap().isRecycled()) {
-            setThumbnailFromBitmap(mArticle.getThumbnailBitmap());
+        Bitmap thumbnail = mArticle.getThumbnailBitmap();
+        if (thumbnail != null && !thumbnail.isRecycled()) {
+            setThumbnailFromBitmap(thumbnail);
             return;
         }
 
@@ -419,7 +420,7 @@ public class SnippetArticleViewHolder extends CardViewHolder implements Impressi
                 thumbnail, targetSize, targetSize, ThumbnailUtils.OPTIONS_RECYCLE_INPUT);
 
         // Store the bitmap to skip the download task next time we display this snippet.
-        snippet.setThumbnailBitmap(scaledThumbnail);
+        snippet.setThumbnailBitmap(mUiDelegate.getReferencePool().put(scaledThumbnail));
 
         // Cross-fade between the placeholder and the thumbnail. We cross-fade because the incoming
         // image may have transparency and we don't want the previous image showing up behind.

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

@@ -12,6 +12,7 @@ import android.view.MotionEvent;
 import android.view.View;
 
 import org.chromium.base.ApiCompatibilityUtils;
+import org.chromium.base.DiscardableReferencePool;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.NativePageHost;
@@ -56,7 +57,8 @@ public class SuggestionsBottomSheetContent implements BottomSheet.BottomSheetCon
                 new SuggestionsNavigationDelegateImpl(activity, profile, sheet, tabModelSelector);
         mTileGroupDelegate = new TileGroupDelegateImpl(
                 activity, profile, tabModelSelector, navigationDelegate, snackbarManager);
-        mSuggestionsUiDelegate = createSuggestionsDelegate(profile, navigationDelegate, sheet);
+        mSuggestionsUiDelegate = createSuggestionsDelegate(
+                profile, navigationDelegate, sheet, activity.getReferencePool());
 
         mView = LayoutInflater.from(activity).inflate(
                 R.layout.suggestions_bottom_sheet_content, null);
@@ -179,7 +181,8 @@ public class SuggestionsBottomSheetContent implements BottomSheet.BottomSheetCon
     }
 
     private static SuggestionsUiDelegateImpl createSuggestionsDelegate(Profile profile,
-            SuggestionsNavigationDelegate navigationDelegate, NativePageHost host) {
+            SuggestionsNavigationDelegate navigationDelegate, NativePageHost host,
+            DiscardableReferencePool referencePool) {
         SnippetsBridge snippetsBridge = null;
         SuggestionsSource suggestionsSource;
         SuggestionsEventReporter eventReporter;
@@ -198,7 +201,7 @@ public class SuggestionsBottomSheetContent implements BottomSheet.BottomSheetCon
         }
 
         SuggestionsUiDelegateImpl delegate = new SuggestionsUiDelegateImpl(
-                suggestionsSource, eventReporter, navigationDelegate, profile, host);
+                suggestionsSource, eventReporter, navigationDelegate, profile, host, referencePool);
         if (snippetsBridge != null) delegate.addDestructionObserver(snippetsBridge);
 
         return delegate;

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

@@ -4,6 +4,7 @@
 
 package org.chromium.chrome.browser.suggestions;
 
+import org.chromium.base.DiscardableReferencePool;
 import org.chromium.chrome.browser.favicon.FaviconHelper.FaviconImageCallback;
 import org.chromium.chrome.browser.favicon.FaviconHelper.IconAvailabilityCallback;
 import org.chromium.chrome.browser.favicon.LargeIconBridge.LargeIconCallback;
@@ -29,6 +30,12 @@ public interface SuggestionsUiDelegate {
     /** Convenience method to access the {@link SuggestionsNavigationDelegate}. */
     SuggestionsNavigationDelegate getNavigationDelegate();
 
+    /**
+     * @return The reference pool to use for large objects that should be dropped under
+     * memory pressure.
+     */
+    DiscardableReferencePool getReferencePool();
+
     // Favicons
 
     /**
@@ -65,6 +72,6 @@ public interface SuggestionsUiDelegate {
      */
     void addDestructionObserver(DestructionObserver destructionObserver);
 
-    /** @return whether the suggestions UI is currently visible. */
+    /** @return Whether the suggestions UI is currently visible. */
     boolean isVisible();
 }
\ No newline at end of file

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

@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.suggestions;
 
 import android.support.annotation.Nullable;
 
+import org.chromium.base.DiscardableReferencePool;
 import org.chromium.chrome.browser.NativePageHost;
 import org.chromium.chrome.browser.favicon.FaviconHelper;
 import org.chromium.chrome.browser.favicon.FaviconHelper.FaviconImageCallback;
@@ -32,6 +33,8 @@ public class SuggestionsUiDelegateImpl implements SuggestionsUiDelegate {
 
     private final NativePageHost mHost;
 
+    private final DiscardableReferencePool mReferencePool;
+
     private FaviconHelper mFaviconHelper;
     private LargeIconBridge mLargeIconBridge;
 
@@ -39,8 +42,8 @@ public class SuggestionsUiDelegateImpl implements SuggestionsUiDelegate {
 
     public SuggestionsUiDelegateImpl(SuggestionsSource suggestionsSource,
             SuggestionsEventReporter eventReporter,
-            SuggestionsNavigationDelegate navigationDelegate, Profile profile,
-            NativePageHost host) {
+            SuggestionsNavigationDelegate navigationDelegate, Profile profile, NativePageHost host,
+            DiscardableReferencePool referencePool) {
         mSuggestionsSource = suggestionsSource;
         mSuggestionsRanker = new SuggestionsRanker();
         mSuggestionsEventReporter = eventReporter;
@@ -48,6 +51,7 @@ public class SuggestionsUiDelegateImpl implements SuggestionsUiDelegate {
 
         mProfile = profile;
         mHost = host;
+        mReferencePool = referencePool;
     }
 
     @Override
@@ -96,6 +100,11 @@ public class SuggestionsUiDelegateImpl implements SuggestionsUiDelegate {
         return mSuggestionsNavigationDelegate;
     }
 
+    @Override
+    public DiscardableReferencePool getReferencePool() {
+        return mReferencePool;
+    }
+
     @Override
     public void addDestructionObserver(DestructionObserver destructionObserver) {
         mDestructionObservers.add(destructionObserver);

chrome/android/javatests/src/org/chromium/chrome/browser/ntp/snippets/ArticleSnippetsTest.java

@@ -4,6 +4,7 @@
 
 package org.chromium.chrome.browser.ntp.snippets;
 
+import android.graphics.Bitmap;
 import android.graphics.BitmapFactory;
 import android.support.test.InstrumentationRegistry;
 import android.support.test.filters.MediumTest;
@@ -16,6 +17,7 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+import org.chromium.base.DiscardableReferencePool;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.base.test.util.Feature;
@@ -146,9 +148,11 @@ public class ArticleSnippetsTest {
                 1466614774, // Publish timestamp
                 10f, // Score
                 1466634774); // Fetch timestamp
-        shortSnippet.setThumbnailBitmap(
+
+        Bitmap thumbnail =
                 BitmapFactory.decodeResource(mActivityTestRule.getActivity().getResources(),
-                        R.drawable.signin_promo_illustration));
+                        R.drawable.signin_promo_illustration);
+        shortSnippet.setThumbnailBitmap(mUiDelegate.getReferencePool().put(thumbnail));
 
         SnippetArticle longSnippet = new SnippetArticle(fullCategory, "id2",
                 new String(new char[20]).replace("\0", "Snippet "),
@@ -206,6 +210,7 @@ public class ArticleSnippetsTest {
         private SuggestionsEventReporter mSuggestionsEventReporter =
                 new DummySuggestionsEventReporter();
         private SuggestionsRanker mSuggestionsRanker = new SuggestionsRanker();
+        private final DiscardableReferencePool mReferencePool = new DiscardableReferencePool();
 
         @Override
         public void getLocalFaviconImageForURL(
@@ -245,6 +250,11 @@ public class ArticleSnippetsTest {
             return mSuggestionsRanker;
         }
 
+        @Override
+        public DiscardableReferencePool getReferencePool() {
+            return mReferencePool;
+        }
+
         @Override
         public void addDestructionObserver(DestructionObserver destructionObserver) {}
 

tools/android/eclipse/.classpath

@@ -25,6 +25,7 @@ to the classpath for downstream development. See "additional_entries" below.
     <classpathentry kind="src" path="android_webview/tools/system_webview_shell/layout_tests/src"/>
     <classpathentry kind="src" path="base/android/java/src"/>
     <classpathentry kind="src" path="base/android/javatests/src"/>
+    <classpathentry kind="src" path="base/android/junit/src"/>
     <classpathentry kind="src" path="base/test/android/javatests/src"/>
     <classpathentry kind="src" path="base/test/android/junit/src"/>
     <classpathentry kind="src" path="chrome/android/java/src"/>