[Android History] Add tests for removing items

[Android History] Add tests for removing items

Adds HistoryAdapterTest and HistoryManagerTest with tests for
removing history items and basic tests for initializing the
adapter.

Also fixes two bugs related to removing items:
 - Remove header if only 1 item is remaining so empty view is shown
 - Set item positions after removing an item

BUG=654071
Committed: https://crrev.com/c1bc970e4647e0a64455089d6f108625d4ba8d1c
Cr-Commit-Position: refs/heads/master@{#441483}

[Theresa, 2017-01-03 18:32:40]

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

[commit-bot: I haz the power, 2017-01-03 18:33:07]

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

[Theresa, 2017-01-03 19:02:45]

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

[commit-bot: I haz the power, 2017-01-03 19:03:08]

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

[Theresa, 2017-01-03 19:28:02]

twellington@chromium.org changed reviewers:
+ dfalcantara@chromium.org

[Theresa, 2017-01-03 19:28:02]

ptal

[commit-bot: I haz the power, 2017-01-03 19:55:32]

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

[commit-bot: I haz the power, 2017-01-03 19:55:32]

Dry run: This issue passed the CQ dry run.

[Theresa, 2017-01-03 21:57:23]

https://codereview.chromium.org/2603403003/diff/20001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/history/HistoryAdapterTest.java
(right):

https://codereview.chromium.org/2603403003/diff/20001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/history/HistoryAdapterTest.java:75:
// There should be three items - the header, a date and the history item.
This comment needs to be updated.

https://codereview.chromium.org/2603403003/diff/20001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/history/HistoryAdapterTest.java:109:
// There should be three items - the header, a date and the history item.
This comment needs to be updated too.

[Theresa, 2017-01-04 17:22:20]

https://codereview.chromium.org/2603403003/diff/20001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/history/HistoryActivityTest.java
(right):

https://codereview.chromium.org/2603403003/diff/20001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/history/HistoryActivityTest.java:113:
item.remove();
Instead of calling remove directly, this could find the remove button in the
HistoryItemView and click it.

[gone, 2017-01-04 19:35:25]

https://codereview.chromium.org/2603403003/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/history/HistoryAdapter.java
(right):

https://codereview.chromium.org/2603403003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/history/HistoryAdapter.java:141:
removeHeader();
Do you need to worry about adding the header back if #removeItems isn't called,
or if a new history item comes in via a different tab?

https://codereview.chromium.org/2603403003/diff/20001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/history/HistoryActivityTest.java
(right):

https://codereview.chromium.org/2603403003/diff/20001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/history/HistoryActivityTest.java:113:
item.remove();
On 2017/01/04 17:22:20, Theresa wrote:
> Instead of calling remove directly, this could find the remove button in the
> HistoryItemView and click it.

Probably more representative of user behavior, no?  Shouldn't have issues with
tests flaking if there's only one item to click on.

https://codereview.chromium.org/2603403003/diff/20001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/history/HistoryAdapterTest.java
(right):

https://codereview.chromium.org/2603403003/diff/20001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/history/HistoryAdapterTest.java:23:
private static final long DAY_IN_MILLISECONDS = 1000 * 60 * 60 * 24;
TimeUnit.DAYS.toMillis(1)

[Theresa, 2017-01-04 21:00:32]

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

[commit-bot: I haz the power, 2017-01-04 21:00:56]

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

[Theresa, 2017-01-04 21:05:19]

https://codereview.chromium.org/2603403003/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/history/HistoryAdapter.java
(right):

https://codereview.chromium.org/2603403003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/history/HistoryAdapter.java:141:
removeHeader();
On 2017/01/04 19:35:25, dfalcantara (check queue) wrote:
> Do you need to worry about adding the header back if #removeItems isn't
called,

#removeItems() is always called after markItemForRemoval() (if that changes,
this can be revisited). Even if #removeItems() did not get called, once all
items have been removed there is no way to add any items to the adapter without
reloading the history page.

> or if a new history item comes in via a different tab?

We don't currently refresh the list of history items if a new item comes in via
a different tab (on the old UI or the new UI); the user has to manually refresh
the page. The native BrowsingHistoryService would have to implement some
additional HistoryServiceObserver.

https://codereview.chromium.org/2603403003/diff/20001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/history/HistoryActivityTest.java
(right):

https://codereview.chromium.org/2603403003/diff/20001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/history/HistoryActivityTest.java:113:
item.remove();
On 2017/01/04 19:35:25, dfalcantara (check queue) wrote:
> On 2017/01/04 17:22:20, Theresa wrote:
> > Instead of calling remove directly, this could find the remove button in the
> > HistoryItemView and click it.
> 
> Probably more representative of user behavior, no?  Shouldn't have issues with
> tests flaking if there's only one item to click on.

Done.

https://codereview.chromium.org/2603403003/diff/20001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/history/HistoryAdapterTest.java
(right):

https://codereview.chromium.org/2603403003/diff/20001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/history/HistoryAdapterTest.java:23:
private static final long DAY_IN_MILLISECONDS = 1000 * 60 * 60 * 24;
On 2017/01/04 19:35:25, dfalcantara (check queue) wrote:
> TimeUnit.DAYS.toMillis(1)

Done.

https://codereview.chromium.org/2603403003/diff/20001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/history/HistoryAdapterTest.java:75:
// There should be three items - the header, a date and the history item.
On 2017/01/03 21:57:23, Theresa wrote:
> This comment needs to be updated.

Done.

https://codereview.chromium.org/2603403003/diff/20001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/history/HistoryAdapterTest.java:109:
// There should be three items - the header, a date and the history item.
On 2017/01/03 21:57:23, Theresa wrote:
> This comment needs to be updated too.

Done.

[gone, 2017-01-04 21:06:29]

lgtm

[Theresa, 2017-01-04 21:08:06]

The CQ bit was unchecked by twellington@chromium.org

[Theresa, 2017-01-04 21:08:06]

The CQ bit was checked by twellington@chromium.org

[commit-bot: I haz the power, 2017-01-04 21:08:22]

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-04 22:15:09]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1483564086709510,
"parent_rev": "a6c74df25d2451316c2f3866ab606707769a9c2b", "commit_rev":
"27ad279f64b0755eff4f23aa25be47c9f098c39e"}

[commit-bot: I haz the power, 2017-01-04 22:15:40]

Description was changed from

==========
[Android History] Add tests for removing items

Adds HistoryAdapterTest and HistoryManagerTest with tests for
removing history items and basic tests for initializing the
adapter.

Also fixes two bugs related to removing items:
 - Remove header if only 1 item is remaining so empty view is shown
 - Set item positions after removing an item

BUG=654071
==========

to

==========
[Android History] Add tests for removing items

Adds HistoryAdapterTest and HistoryManagerTest with tests for
removing history items and basic tests for initializing the
adapter.

Also fixes two bugs related to removing items:
 - Remove header if only 1 item is remaining so empty view is shown
 - Set item positions after removing an item

BUG=654071

Review-Url: https://codereview.chromium.org/2603403003
==========

[commit-bot: I haz the power, 2017-01-04 22:15:41]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2017-01-04 22:17:18]

Description was changed from

==========
[Android History] Add tests for removing items

Adds HistoryAdapterTest and HistoryManagerTest with tests for
removing history items and basic tests for initializing the
adapter.

Also fixes two bugs related to removing items:
 - Remove header if only 1 item is remaining so empty view is shown
 - Set item positions after removing an item

BUG=654071

Review-Url: https://codereview.chromium.org/2603403003
==========

to

==========
[Android History] Add tests for removing items

Adds HistoryAdapterTest and HistoryManagerTest with tests for
removing history items and basic tests for initializing the
adapter.

Also fixes two bugs related to removing items:
 - Remove header if only 1 item is remaining so empty view is shown
 - Set item positions after removing an item

BUG=654071

Committed: https://crrev.com/c1bc970e4647e0a64455089d6f108625d4ba8d1c
Cr-Commit-Position: refs/heads/master@{#441483}
==========

[commit-bot: I haz the power, 2017-01-04 22:17:19]

Patchset 3 (id:??) landed as
https://crrev.com/c1bc970e4647e0a64455089d6f108625d4ba8d1c
Cr-Commit-Position: refs/heads/master@{#441483}