[ios] Open links in history status message in new tab and add test

[ios] Open links in history status message in new tab and add test

Makes links in the history entries status message (sync or local)
open in new tab rather than current tab. Also adds
testHistoryEntriesStatusItem to test that a message about history
entries shows up on the history panel when the user is signed in and
that tapping the link in the message opens a new tab with sync info.

BUG=676191
Committed: https://crrev.com/3cc8e9fb672d4dedbdbd04dd88ad741333d1670b
Cr-Commit-Position: refs/heads/master@{#440566}

[Jackie Quinn, 2016-12-21 19:52:33]

jyquinn@chromium.org changed reviewers:
+ baxley@chromium.org, sczs@chromium.org

[baxley, 2016-12-22 17:58:38]

It'd be nice if we didn't have to know about the class types and match that, but
let me know if there is no other way. Thanks!

https://codereview.chromium.org/2598763002/diff/1/ios/chrome/browser/ui/histo...
File ios/chrome/browser/ui/history/history_ui_egtest.mm (right):

https://codereview.chromium.org/2598763002/diff/1/ios/chrome/browser/ui/histo...
ios/chrome/browser/ui/history/history_ui_egtest.mm:120: void SignIn() {
Could you add a comment?

What do you think about a more descriptive name? SignInFakeIdentity? 
Unfortunately I feel like "SignIn" isn't descriptive enough, but I can't think
of something any better... Up to you.

https://codereview.chromium.org/2598763002/diff/1/ios/chrome/browser/ui/histo...
ios/chrome/browser/ui/history/history_ui_egtest.mm:129: const CGFloat
scroll_displacement = 50.0;
Is reliable on phones and iPads?

Sergio fixed some flake by using "grey_scrollToContentEdge(..." instead. If this
works on multiple form factors (big/small landscape/portrait) then I think it's
fine.

https://codereview.chromium.org/2598763002/diff/1/ios/chrome/browser/ui/histo...
ios/chrome/browser/ui/history/history_ui_egtest.mm:161: @interface
AAAHistoryUITestCase : ChromeTestCase {
What you think your special and can make your test run first? :)

https://codereview.chromium.org/2598763002/diff/1/ios/chrome/browser/ui/histo...
ios/chrome/browser/ui/history/history_ui_egtest.mm:258:
HistoryEntriesStatusCell* cell = (HistoryEntriesStatusCell*)element;
ObjCCast ?

https://codereview.chromium.org/2598763002/diff/1/ios/chrome/browser/ui/histo...
ios/chrome/browser/ui/history/history_ui_egtest.mm:267:
assertWithMatcher:emptyEntriesItemMatcher];
Do we need to know about the class type? we can't verify this by text or
anything else on (or not on) the screen?

https://codereview.chromium.org/2598763002/diff/1/ios/chrome/browser/ui/histo...
ios/chrome/browser/ui/history/history_ui_egtest.mm:286:
selectElementWithMatcher:grey_kindOfClass([TransparentLinkButton class])]
Is there anything we can use to match the link, other than the class type?

https://codereview.chromium.org/2598763002/diff/1/ios/chrome/browser/ui/histo...
ios/chrome/browser/ui/history/history_ui_egtest.mm:292:
assertWithMatcher:grey_notNil()];
Are the signed in accounts removed at the end of the test?

[Jackie Quinn, 2016-12-22 20:09:13]

https://codereview.chromium.org/2598763002/diff/1/ios/chrome/browser/ui/histo...
File ios/chrome/browser/ui/history/history_ui_egtest.mm (right):

https://codereview.chromium.org/2598763002/diff/1/ios/chrome/browser/ui/histo...
ios/chrome/browser/ui/history/history_ui_egtest.mm:120: void SignIn() {
On 2016/12/22 17:58:37, baxley wrote:
> Could you add a comment?
> 
> What do you think about a more descriptive name? SignInFakeIdentity? 
> Unfortunately I feel like "SignIn" isn't descriptive enough, but I can't think
> of something any better... Up to you.

Works for me. Done.

https://codereview.chromium.org/2598763002/diff/1/ios/chrome/browser/ui/histo...
ios/chrome/browser/ui/history/history_ui_egtest.mm:129: const CGFloat
scroll_displacement = 50.0;
On 2016/12/22 17:58:37, baxley wrote:
> Is reliable on phones and iPads?
> 
> Sergio fixed some flake by using "grey_scrollToContentEdge(..." instead. If
this
> works on multiple form factors (big/small landscape/portrait) then I think
it's
> fine.

Oh yeah that seems to be a better solution! Updated.

In this case, the sign in element is at the top of the collection, so it's
probably not a big deal (I had just copied over from the sign in egtests 🙃).

https://codereview.chromium.org/2598763002/diff/1/ios/chrome/browser/ui/histo...
ios/chrome/browser/ui/history/history_ui_egtest.mm:161: @interface
AAAHistoryUITestCase : ChromeTestCase {
On 2016/12/22 17:58:37, baxley wrote:
> What you think your special and can make your test run first? :)

Isn't history our most important feature? Done.

https://codereview.chromium.org/2598763002/diff/1/ios/chrome/browser/ui/histo...
ios/chrome/browser/ui/history/history_ui_egtest.mm:258:
HistoryEntriesStatusCell* cell = (HistoryEntriesStatusCell*)element;
On 2016/12/22 17:58:37, baxley wrote:
> ObjCCast ?

Done.

https://codereview.chromium.org/2598763002/diff/1/ios/chrome/browser/ui/histo...
ios/chrome/browser/ui/history/history_ui_egtest.mm:267:
assertWithMatcher:emptyEntriesItemMatcher];
On 2016/12/22 17:58:37, baxley wrote:
> Do we need to know about the class type? we can't verify this by text or
> anything else on (or not on) the screen?

Well, I could wait for the text I look for below to not be on the screen.
Updated.

https://codereview.chromium.org/2598763002/diff/1/ios/chrome/browser/ui/histo...
ios/chrome/browser/ui/history/history_ui_egtest.mm:286:
selectElementWithMatcher:grey_kindOfClass([TransparentLinkButton class])]
On 2016/12/22 17:58:37, baxley wrote:
> Is there anything we can use to match the link, other than the class type?

I don't think so... I could look for the first UI button without any text? The
problem is it doesn't have any other distinguishing features :-( Let me know
what you think.

https://codereview.chromium.org/2598763002/diff/1/ios/chrome/browser/ui/histo...
ios/chrome/browser/ui/history/history_ui_egtest.mm:292:
assertWithMatcher:grey_notNil()];
On 2016/12/22 17:58:37, baxley wrote:
> Are the signed in accounts removed at the end of the test?

Yes, disabling mock auth is run at the end of all tests, and includes signing
out and clearing accounts.

[sczs, 2016-12-22 22:24:21]

lgtm

[baxley, 2016-12-22 22:35:32]

egtest LGTM.

https://codereview.chromium.org/2598763002/diff/1/ios/chrome/browser/ui/histo...
File ios/chrome/browser/ui/history/history_ui_egtest.mm (right):

https://codereview.chromium.org/2598763002/diff/1/ios/chrome/browser/ui/histo...
ios/chrome/browser/ui/history/history_ui_egtest.mm:161: @interface
AAAHistoryUITestCase : ChromeTestCase {
On 2016/12/22 20:09:13, Jackie Quinn wrote:
> On 2016/12/22 17:58:37, baxley wrote:
> > What you think your special and can make your test run first? :)
> 
> Isn't history our most important feature? Done.

By the way, you should have totally called me out on using "your" instead of
"you're"

[Jackie Quinn, 2016-12-22 22:45:14]

https://codereview.chromium.org/2598763002/diff/1/ios/chrome/browser/ui/histo...
File ios/chrome/browser/ui/history/history_ui_egtest.mm (right):

https://codereview.chromium.org/2598763002/diff/1/ios/chrome/browser/ui/histo...
ios/chrome/browser/ui/history/history_ui_egtest.mm:161: @interface
AAAHistoryUITestCase : ChromeTestCase {
On 2016/12/22 22:35:32, baxley wrote:
> On 2016/12/22 20:09:13, Jackie Quinn wrote:
> > On 2016/12/22 17:58:37, baxley wrote:
> > > What you think your special and can make your test run first? :)
> > 
> > Isn't history our most important feature? Done.
> 
> By the way, you should have totally called me out on using "your" instead of
> "you're"
Dang, missed opportunity 🙃

[Jackie Quinn, 2016-12-22 22:47:30]

Description was changed from

==========
[ios] Add testHistoryEntriesStatusItem to history UI egtests

Adds testHistoryEntriesStatusItem to test that a message about history
entries shows up on the history panel when the user is signed in and
that tapping the link in the message opens a new tab with sync info.

BUG=676191
==========

to

==========
[ios] Open links in history status message in new tab and add test

Makes links in the history entries status message (sync or local)
open in new tab rather than current tab. Also adds 
testHistoryEntriesStatusItem to test that a message about history
entries shows up on the history panel when the user is signed in and
that tapping the link in the message opens a new tab with sync info.

BUG=676191
==========

[Jackie Quinn, 2016-12-22 22:47:38]

The CQ bit was checked by jyquinn@chromium.org

[commit-bot: I haz the power, 2016-12-22 22:47:58]

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, 2016-12-23 00:51:10]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1482446858454380,
"parent_rev": "91957f1ebfa3a79033c41ed1bf9cd22d05c936d2", "commit_rev":
"f36406144dcc1c9e6315aa12e3974051c5d375a8"}

[commit-bot: I haz the power, 2016-12-23 00:51:37]

Description was changed from

==========
[ios] Open links in history status message in new tab and add test

Makes links in the history entries status message (sync or local)
open in new tab rather than current tab. Also adds 
testHistoryEntriesStatusItem to test that a message about history
entries shows up on the history panel when the user is signed in and
that tapping the link in the message opens a new tab with sync info.

BUG=676191
==========

to

==========
[ios] Open links in history status message in new tab and add test

Makes links in the history entries status message (sync or local)
open in new tab rather than current tab. Also adds 
testHistoryEntriesStatusItem to test that a message about history
entries shows up on the history panel when the user is signed in and
that tapping the link in the message opens a new tab with sync info.

BUG=676191

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

[commit-bot: I haz the power, 2016-12-23 00:51:41]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-23 00:55:34]

Description was changed from

==========
[ios] Open links in history status message in new tab and add test

Makes links in the history entries status message (sync or local)
open in new tab rather than current tab. Also adds 
testHistoryEntriesStatusItem to test that a message about history
entries shows up on the history panel when the user is signed in and
that tapping the link in the message opens a new tab with sync info.

BUG=676191

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

to

==========
[ios] Open links in history status message in new tab and add test

Makes links in the history entries status message (sync or local)
open in new tab rather than current tab. Also adds
testHistoryEntriesStatusItem to test that a message about history
entries shows up on the history panel when the user is signed in and
that tapping the link in the message opens a new tab with sync info.

BUG=676191

Committed: https://crrev.com/3cc8e9fb672d4dedbdbd04dd88ad741333d1670b
Cr-Commit-Position: refs/heads/master@{#440566}
==========

[commit-bot: I haz the power, 2016-12-23 00:55:35]

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