Add RecentTabsPageTest

Add RecentTabsPageTest.

* Adds the first test for RecentTabsPage.
* Extracts RecentlyClosedTabManager interface from RecentlyClosedBridge to enable injection of a fake in tests.
* Deletes a bunch of one-line helper functions.
* Minor cleanups like using diamond operators and restricting visibility.

BUG=659631
Review-Url: https://codereview.chromium.org/2610143002
Cr-Commit-Position: refs/heads/master@{#441935}
Committed: https://chromium.googlesource.com/chromium/src/+/7e0ae49ce66d97e8fb353d55563b0ed040e49416

[Michael van Ouwerkerk, 2017-01-04 10:17:32]

The CQ bit was checked by mvanouwerkerk@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-04 10:17:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Michael van Ouwerkerk, 2017-01-04 10:33:39]

Description was changed from

==========
Add RecentTabsPageTest.

BUG=659631
==========

to

==========
Add RecentTabsPageTest.

* Adds the first test for RecentTabsPage.
* Extracts RecentlyClosedTabManager interface from RecentlyClosedBridge to
enable injection of a fake in tests.
* Deletes a bunch of one-line helper functions.
* Minor cleanups like using diamond operators and restricting visibility.

BUG=659631
==========

[Michael van Ouwerkerk, 2017-01-04 10:34:16]

The CQ bit was checked by mvanouwerkerk@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-04 10:34:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Michael van Ouwerkerk, 2017-01-04 11:15:44]

mvanouwerkerk@chromium.org changed reviewers:
+ bauerb@chromium.org

[Michael van Ouwerkerk, 2017-01-04 11:15:45]

Bernhard, could you take a look please?

[Bernhard Bauer, 2017-01-04 11:54:17]

Neat!

https://codereview.chromium.org/2610143002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/FakeRecentlyClosedTabManager.java
(right):

https://codereview.chromium.org/2610143002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/FakeRecentlyClosedTabManager.java:30:
return tabs;
Could you do `mTabs.subList(0, Math.min(maxTabCount, mTabs.size()))`?

https://codereview.chromium.org/2610143002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsManager.java
(right):

https://codereview.chromium.org/2610143002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsManager.java:87:
getRecentlyClosedTabManager().setRecentlyClosedCallback(new
RecentlyClosedCallback() {
Actually, you could probably pass the callback in to the constructor, then you
don't need to deal with a null callback.

https://codereview.chromium.org/2610143002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsManager.java:117:
mRecentlyClosedBridge.destroy();
It looks like this is the only thing you use the actual bridge for. If you move
this into the interface, you could make this a RecentlyClosedTabManager and in
the constructor either create a real one or use the one for testing.

https://codereview.chromium.org/2610143002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentlyClosedBridge.java
(right):

https://codereview.chromium.org/2610143002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentlyClosedBridge.java:19:
private RecentlyClosedCallback mCallback;
Maybe mark this as @Nullable?

https://codereview.chromium.org/2610143002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentlyClosedTabManager.java
(right):

https://codereview.chromium.org/2610143002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentlyClosedTabManager.java:18:
interface RecentlyClosedCallback {
This name sounds a bit like it's called when a tab is recently closed (which
doesn't even make sense as soon as I write it out in a sentence, but it's still
hella confusing). Given that the context of recently closed tabs is already
established in the name of the outer class, should we just name it Callback (and
always use it qualified)?

https://codereview.chromium.org/2610143002/diff/20001/chrome/browser/android/...
File chrome/browser/android/recently_closed_tabs_bridge.cc (right):

https://codereview.chromium.org/2610143002/diff/20001/chrome/browser/android/...
chrome/browser/android/recently_closed_tabs_bridge.cc:59: :
bridge_(AttachCurrentThread(), bridge),
Nit: I think what I would do is create the ScopedJavaGlobalRef at the call site
and then pass that in.

[commit-bot: I haz the power, 2017-01-04 12:31:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-04 12:31:56]

Dry run: This issue passed the CQ dry run.

[Michael van Ouwerkerk, 2017-01-05 11:02:14]

https://codereview.chromium.org/2610143002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/FakeRecentlyClosedTabManager.java
(right):

https://codereview.chromium.org/2610143002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/FakeRecentlyClosedTabManager.java:30:
return tabs;
On 2017/01/04 11:54:16, Bernhard Bauer wrote:
> Could you do `mTabs.subList(0, Math.min(maxTabCount, mTabs.size()))`?

Perhaps, but: "The returned List is backed by this List so changes to it are
reflected by the other."

That's not the way the real implementation works, and seems like it could lead
to tricky bugs.

https://codereview.chromium.org/2610143002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsManager.java
(right):

https://codereview.chromium.org/2610143002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsManager.java:87:
getRecentlyClosedTabManager().setRecentlyClosedCallback(new
RecentlyClosedCallback() {
On 2017/01/04 11:54:16, Bernhard Bauer wrote:
> Actually, you could probably pass the callback in to the constructor, then you
> don't need to deal with a null callback.

That wouldn't work with an injected manager, as we wouldn't run the constructor
ourselves. Also, interfaces do not define constructors, so the entire mechanism
of providing a callback would be undefined.

https://codereview.chromium.org/2610143002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsManager.java:117:
mRecentlyClosedBridge.destroy();
On 2017/01/04 11:54:16, Bernhard Bauer wrote:
> It looks like this is the only thing you use the actual bridge for. If you
move
> this into the interface, you could make this a RecentlyClosedTabManager and in
> the constructor either create a real one or use the one for testing.

As discussed offline, yes destroy() can be part of the RecentlyClosedTabManager
interface, and then a few things can be simplified. Done.

https://codereview.chromium.org/2610143002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentlyClosedBridge.java
(right):

https://codereview.chromium.org/2610143002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentlyClosedBridge.java:19:
private RecentlyClosedCallback mCallback;
On 2017/01/04 11:54:16, Bernhard Bauer wrote:
> Maybe mark this as @Nullable?

Done.

https://codereview.chromium.org/2610143002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentlyClosedTabManager.java
(right):

https://codereview.chromium.org/2610143002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentlyClosedTabManager.java:18:
interface RecentlyClosedCallback {
On 2017/01/04 11:54:16, Bernhard Bauer wrote:
> This name sounds a bit like it's called when a tab is recently closed (which
> doesn't even make sense as soon as I write it out in a sentence, but it's
still
> hella confusing). Given that the context of recently closed tabs is already
> established in the name of the outer class, should we just name it Callback
(and
> always use it qualified)?

Yes, it's an awkward name. This is not new code :-) As discussed offline, we can
just use a Runnable. Done.

https://codereview.chromium.org/2610143002/diff/20001/chrome/browser/android/...
File chrome/browser/android/recently_closed_tabs_bridge.cc (right):

https://codereview.chromium.org/2610143002/diff/20001/chrome/browser/android/...
chrome/browser/android/recently_closed_tabs_bridge.cc:59: :
bridge_(AttachCurrentThread(), bridge),
On 2017/01/04 11:54:16, Bernhard Bauer wrote:
> Nit: I think what I would do is create the ScopedJavaGlobalRef at the call
site
> and then pass that in.

Done.

[Michael van Ouwerkerk, 2017-01-05 11:02:20]

The CQ bit was checked by mvanouwerkerk@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-05 11:02:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-05 12:13:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-05 12:13:09]

Dry run: This issue passed the CQ dry run.

[Bernhard Bauer, 2017-01-05 17:47:38]

lgtm

https://codereview.chromium.org/2610143002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/FakeRecentlyClosedTabManager.java
(right):

https://codereview.chromium.org/2610143002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/FakeRecentlyClosedTabManager.java:30:
return tabs;
On 2017/01/05 11:02:14, Michael van Ouwerkerk wrote:
> On 2017/01/04 11:54:16, Bernhard Bauer wrote:
> > Could you do `mTabs.subList(0, Math.min(maxTabCount, mTabs.size()))`?
> 
> Perhaps, but: "The returned List is backed by this List so changes to it are
> reflected by the other."
> 
> That's not the way the real implementation works, and seems like it could lead
> to tricky bugs.
> 

I'm okay with the code as it is, but that does bring up a point... Are you
referring to the case where a caller modifies the returned list (thereby
mutating the underlying list), or the case where the underlying list is mutated?
Maybe we should be more explicit in the contract of this class about which (if
any) of these operations may happen.

https://codereview.chromium.org/2610143002/diff/40001/chrome/browser/android/...
File chrome/browser/android/recently_closed_tabs_bridge.cc (right):

https://codereview.chromium.org/2610143002/diff/40001/chrome/browser/android/...
chrome/browser/android/recently_closed_tabs_bridge.cc:59: : bridge_(jbridge),
profile_(profile), tab_restore_service_(NULL) {}
Nit: I think here you want to std::move() the |jbridge|.

(And then while you're here, could you change the NULL to nullptr? Thanks! :) )

[Michael van Ouwerkerk, 2017-01-06 11:11:11]

https://codereview.chromium.org/2610143002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/FakeRecentlyClosedTabManager.java
(right):

https://codereview.chromium.org/2610143002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/FakeRecentlyClosedTabManager.java:30:
return tabs;
On 2017/01/05 17:47:37, Bernhard Bauer wrote:
> On 2017/01/05 11:02:14, Michael van Ouwerkerk wrote:
> > On 2017/01/04 11:54:16, Bernhard Bauer wrote:
> > > Could you do `mTabs.subList(0, Math.min(maxTabCount, mTabs.size()))`?
> > 
> > Perhaps, but: "The returned List is backed by this List so changes to it are
> > reflected by the other."
> > 
> > That's not the way the real implementation works, and seems like it could
lead
> > to tricky bugs.
> > 
> 
> I'm okay with the code as it is, but that does bring up a point... Are you
> referring to the case where a caller modifies the returned list (thereby
> mutating the underlying list), or the case where the underlying list is
mutated?
> Maybe we should be more explicit in the contract of this class about which (if
> any) of these operations may happen.

From the subList docs:
https://docs.oracle.com/javase/8/docs/api/java/util/List.html#subList-int-int-
"The returned list is backed by this list, so non-structural changes in the
returned list are reflected in this list, and vice-versa."

Generally for most such cases, I think we should use something like the Guava
ImmutableList, but we don't use Guava. We could use Collections.unmodifiableList
here to wrap the new list, but that wouldn't change the return type and not
achieve much as it's already a new list.

So, just a minor documentation addition that this is a snapshot.

https://codereview.chromium.org/2610143002/diff/40001/chrome/browser/android/...
File chrome/browser/android/recently_closed_tabs_bridge.cc (right):

https://codereview.chromium.org/2610143002/diff/40001/chrome/browser/android/...
chrome/browser/android/recently_closed_tabs_bridge.cc:59: : bridge_(jbridge),
profile_(profile), tab_restore_service_(NULL) {}
On 2017/01/05 17:47:38, Bernhard Bauer wrote:
> Nit: I think here you want to std::move() the |jbridge|.
> 
> (And then while you're here, could you change the NULL to nullptr? Thanks! :)
)

Done.

[Michael van Ouwerkerk, 2017-01-06 11:11:15]

The CQ bit was checked by mvanouwerkerk@chromium.org

[Michael van Ouwerkerk, 2017-01-06 11:11:16]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org
Link to the patchset: https://codereview.chromium.org/2610143002/#ps60001
(title: "Cleanups.")

[commit-bot: I haz the power, 2017-01-06 11:11:25]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-06 12:19:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-06 12:19:20]

Failed to apply patch for
chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsManager.java:
While running git apply --index -p1;
  error: patch failed:
chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsManager.java:60
  error:
chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsManager.java:
patch does not apply

Patch:      
chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsManager.java
Index:
chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsManager.java
diff --git
a/chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsManager.java
b/chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsManager.java
index
cc198aa1f1790ae9708524048efefd1553ca4a68..6de8d3503bb4c83dfbb5a1306bb016bcfdf3b97e
100644
---
a/chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsManager.java
+++
b/chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsManager.java
@@ -10,6 +10,7 @@ import android.graphics.Bitmap;
 
 import org.chromium.base.ContextUtils;
 import org.chromium.base.ThreadUtils;
+import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.browser.UrlConstants;
 import org.chromium.chrome.browser.favicon.FaviconHelper;
 import org.chromium.chrome.browser.favicon.FaviconHelper.FaviconImageCallback;
@@ -18,8 +19,6 @@ import org.chromium.chrome.browser.metrics.StartupMetrics;
 import org.chromium.chrome.browser.ntp.ForeignSessionHelper.ForeignSession;
 import
org.chromium.chrome.browser.ntp.ForeignSessionHelper.ForeignSessionCallback;
 import org.chromium.chrome.browser.ntp.ForeignSessionHelper.ForeignSessionTab;
-import
org.chromium.chrome.browser.ntp.RecentlyClosedBridge.RecentlyClosedCallback;
-import org.chromium.chrome.browser.ntp.RecentlyClosedBridge.RecentlyClosedTab;
 import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.signin.SigninManager;
 import org.chromium.chrome.browser.signin.SigninManager.SignInStateObserver;
@@ -51,6 +50,8 @@ public class RecentTabsManager implements
AndroidSyncSettingsObserver, SignInSta
     private static final String PREF_SIGNIN_PROMO_DECLINED =
             "recent_tabs_signin_promo_declined";
 
+    private static RecentlyClosedTabManager sRecentlyClosedTabManagerForTests;
+
     private final Profile mProfile;
     private final Tab mTab;
     private final Context mContext;
@@ -60,7 +61,7 @@ public class RecentTabsManager implements
AndroidSyncSettingsObserver, SignInSta
     private List<ForeignSession> mForeignSessions;
     private List<RecentlyClosedTab> mRecentlyClosedTabs;
     private NewTabPagePrefs mNewTabPagePrefs;
-    private RecentlyClosedBridge mRecentlyClosedBridge;
+    private RecentlyClosedTabManager mRecentlyClosedTabManager;
     private SigninManager mSignInManager;
     private UpdatedCallback mUpdatedCallback;
     private boolean mIsDestroyed;
@@ -75,13 +76,23 @@ public class RecentTabsManager implements
AndroidSyncSettingsObserver, SignInSta
     public RecentTabsManager(Tab tab, Profile profile, Context context) {
         mProfile = profile;
         mTab = tab;
-        mForeignSessionHelper = buildForeignSessionHelper(mProfile);
-        mNewTabPagePrefs = buildNewTabPagePrefs(mProfile);
-        mFaviconHelper = buildFaviconHelper();
-        mRecentlyClosedBridge = buildRecentlyClosedBridge(mProfile);
+        mForeignSessionHelper = new ForeignSessionHelper(profile);
+        mNewTabPagePrefs = new NewTabPagePrefs(profile);
+        mFaviconHelper = new FaviconHelper();
+        mRecentlyClosedTabManager = sRecentlyClosedTabManagerForTests != null
+                ? sRecentlyClosedTabManagerForTests
+                : new RecentlyClosedBridge(profile);
         mSignInManager = SigninManager.get(context);
         mContext = context;
 
+        mRecentlyClosedTabManager.setTabsUpdatedRunnable(new Runnable() {
+            @Override
+            public void run() {
+                updateRecentlyClosedTabs();
+                postUpdate();
+            }
+        });
+
         updateRecentlyClosedTabs();
         registerForForeignSessionUpdates();
         updateForeignSessions();
@@ -104,8 +115,8 @@ public class RecentTabsManager implements
AndroidSyncSettingsObserver, SignInSta
         mFaviconHelper.destroy();
         mFaviconHelper = null;
 
-        mRecentlyClosedBridge.destroy();
-        mRecentlyClosedBridge = null;
+        mRecentlyClosedTabManager.destroy();
+        mRecentlyClosedTabManager = null;
 
         mForeignSessionHelper.destroy();
         mForeignSessionHelper = null;
@@ -118,37 +129,6 @@ public class RecentTabsManager implements
AndroidSyncSettingsObserver, SignInSta
         InvalidationController.get(mContext).onRecentTabsPageClosed();
     }
 
-    /**
-     * Returns true if destroy() has been called.
-     */
-    public boolean isDestroyed() {
-        return mIsDestroyed;
-    }
-
-    private static ForeignSessionHelper buildForeignSessionHelper(Profile
profile) {
-        return new ForeignSessionHelper(profile);
-    }
-
-    private static NewTabPagePrefs buildNewTabPagePrefs(Profile profile) {
-        return new NewTabPagePrefs(profile);
-    }
-
-    private static FaviconHelper buildFaviconHelper() {
-        return new FaviconHelper();
-    }
-
-    private RecentlyClosedBridge buildRecentlyClosedBridge(Profile profile) {
-        RecentlyClosedBridge bridge = new RecentlyClosedBridge(profile);
-        bridge.setRecentlyClosedCallback(new RecentlyClosedCallback() {
-            @Override
-            public void onUpdated() {
-                updateRecentlyClosedTabs();
-                postUpdate();
-            }
-        });
-        return bridge;
-    }
-
     private void registerForForeignSessionUpdates() {
         mForeignSessionHelper.setOnForeignSessionCallback(new
ForeignSessionCallback() {
             @Override
@@ -165,8 +145,8 @@ public class RecentTabsManager implements
AndroidSyncSettingsObserver, SignInSta
     }
 
     private void updateRecentlyClosedTabs() {
-        mRecentlyClosedTabs = mRecentlyClosedBridge.getRecentlyClosedTabs(
-                RECENTLY_CLOSED_MAX_TAB_COUNT);
+        mRecentlyClosedTabs =
+               
mRecentlyClosedTabManager.getRecentlyClosedTabs(RECENTLY_CLOSED_MAX_TAB_COUNT);
     }
 
     private void updateForeignSessions() {
@@ -214,7 +194,7 @@ public class RecentTabsManager implements
AndroidSyncSettingsObserver, SignInSta
     public void openRecentlyClosedTab(RecentlyClosedTab tab, int
windowDisposition) {
         if (mIsDestroyed) return;
        
NewTabPageUma.recordAction(NewTabPageUma.ACTION_OPENED_RECENTLY_CLOSED_ENTRY);
-        mRecentlyClosedBridge.openRecentlyClosedTab(mTab, tab,
windowDisposition);
+        mRecentlyClosedTabManager.openRecentlyClosedTab(mTab, tab,
windowDisposition);
     }
 
     /**
@@ -301,7 +281,7 @@ public class RecentTabsManager implements
AndroidSyncSettingsObserver, SignInSta
         return mNewTabPagePrefs.getRecentlyClosedTabsCollapsed();
     }
 
-   /**
+    /**
      * Remove Foreign session to display. Note that it might reappear during
the next sync if the
      * session is not orphaned.
      *
@@ -318,7 +298,7 @@ public class RecentTabsManager implements
AndroidSyncSettingsObserver, SignInSta
      */
     public void clearRecentlyClosedTabs() {
         if (mIsDestroyed) return;
-        mRecentlyClosedBridge.clearRecentlyClosedTabs();
+        mRecentlyClosedTabManager.clearRecentlyClosedTabs();
     }
 
     /**
@@ -369,7 +349,7 @@ public class RecentTabsManager implements
AndroidSyncSettingsObserver, SignInSta
         return mNewTabPagePrefs.getSyncPromoCollapsed();
     }
 
-    protected void postUpdate() {
+    private void postUpdate() {
         if (mUpdatedCallback != null) {
             mUpdatedCallback.onUpdated();
         }
@@ -402,4 +382,9 @@ public class RecentTabsManager implements
AndroidSyncSettingsObserver, SignInSta
     public boolean isSignedIn() {
         return ChromeSigninController.get(mContext).isSignedIn();
     }
+
+    @VisibleForTesting
+    public static void
setRecentlyClosedTabManagerForTests(RecentlyClosedTabManager manager) {
+        sRecentlyClosedTabManagerForTests = manager;
+    }
 }

[Michael van Ouwerkerk, 2017-01-06 13:43:48]

The CQ bit was checked by mvanouwerkerk@chromium.org

[Michael van Ouwerkerk, 2017-01-06 13:43:49]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org
Link to the patchset: https://codereview.chromium.org/2610143002/#ps80001
(title: "fix bad merge")

[commit-bot: I haz the power, 2017-01-06 13:44:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-06 15:03:30]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1483710228748480,
"parent_rev": "d3f91633a7bad0370800470d8ea76078b1ac02db", "commit_rev":
"7e0ae49ce66d97e8fb353d55563b0ed040e49416"}

[commit-bot: I haz the power, 2017-01-06 15:03:58]

Description was changed from

==========
Add RecentTabsPageTest.

* Adds the first test for RecentTabsPage.
* Extracts RecentlyClosedTabManager interface from RecentlyClosedBridge to
enable injection of a fake in tests.
* Deletes a bunch of one-line helper functions.
* Minor cleanups like using diamond operators and restricting visibility.

BUG=659631
==========

to

==========
Add RecentTabsPageTest.

* Adds the first test for RecentTabsPage.
* Extracts RecentlyClosedTabManager interface from RecentlyClosedBridge to
enable injection of a fake in tests.
* Deletes a bunch of one-line helper functions.
* Minor cleanups like using diamond operators and restricting visibility.

BUG=659631

Review-Url: https://codereview.chromium.org/2610143002
Cr-Commit-Position: refs/heads/master@{#441935}
Committed:
https://chromium.googlesource.com/chromium/src/+/7e0ae49ce66d97e8fb353d55563b...
==========

[commit-bot: I haz the power, 2017-01-06 15:03:59]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/7e0ae49ce66d97e8fb353d55563b...