Create shared matcher for navigation bar done button.

Create shared matcher for navigation bar done button.

Update tests to use shared matcher, rather than implementing
their own.

BUG=693579
Review-Url: https://codereview.chromium.org/2733973002
Cr-Commit-Position: refs/heads/master@{#455134}
Committed: https://chromium.googlesource.com/chromium/src/+/ba2952ea9150740fdeca13c3c973aaa1310a09de

[baxley, 2017-03-06 23:51:44]

Description was changed from

==========
Create shared matcher for navigation bar done button.

BUG=
==========

to

==========
Create shared matcher for navigation bar done button.

Update tests to use shared matcher, rather than implementing
their own.

BUG=693579
==========

[baxley, 2017-03-06 23:51:44]

baxley@chromium.org changed reviewers:
+ eugenebut@chromium.org

[baxley, 2017-03-06 23:56:37]

Sorry for all the reviews, please let me know if you need a break and I can find
someone else to do the review.

I didn't make the change in a few locations where we have C-style matchers that
take an IDS string. Those will be cleaned up separately, since the test isn't
set up to use a matcher (it passes the ID to a helper method that performs the
tap). For example, crbug.com/698866

[Eugene But (OOO till 7-30), 2017-03-07 02:56:41]

Actually thank you for all these reviews :)

https://codereview.chromium.org/2733973002/diff/1/ios/chrome/test/earl_grey/c...
File ios/chrome/test/earl_grey/chrome_matchers.mm (right):

https://codereview.chromium.org/2733973002/diff/1/ios/chrome/test/earl_grey/c...
ios/chrome/test/earl_grey/chrome_matchers.mm:237: grey_sufficientlyVisible(),
nil);
I just noticed that some matchers have |grey_sufficientlyVisible| and some
don't. I to be fair it's quite unexpected to me that these methods have
|grey_sufficientlyVisible| check. Do we even need this check? If so, should we
use a different function name (f.e. SufficientlyVisibleNavigationBarDoneButton)?

[baxley, 2017-03-07 15:28:33]

https://codereview.chromium.org/2733973002/diff/1/ios/chrome/test/earl_grey/c...
File ios/chrome/test/earl_grey/chrome_matchers.mm (right):

https://codereview.chromium.org/2733973002/diff/1/ios/chrome/test/earl_grey/c...
ios/chrome/test/earl_grey/chrome_matchers.mm:237: grey_sufficientlyVisible(),
nil);
On 2017/03/07 02:56:41, Eugene But wrote:
> I just noticed that some matchers have |grey_sufficientlyVisible| and some
> don't. I to be fair it's quite unexpected to me that these methods have
> |grey_sufficientlyVisible| check. Do we even need this check? If so, should we
> use a different function name (f.e.
SufficientlyVisibleNavigationBarDoneButton)?

There was one at least one test that needed it, so I just put it in here. I
think it's better to not have it, so I removed it and created a local variable
matcher for the test that needs it (testClearBrowsingHistory in
history_ui_egtest).

I'm running this through the try bots to make sure nothing else needs it.

[Eugene But (OOO till 7-30), 2017-03-07 16:33:32]

lgtm

https://codereview.chromium.org/2733973002/diff/1/ios/chrome/test/earl_grey/c...
File ios/chrome/test/earl_grey/chrome_matchers.mm (right):

https://codereview.chromium.org/2733973002/diff/1/ios/chrome/test/earl_grey/c...
ios/chrome/test/earl_grey/chrome_matchers.mm:237: grey_sufficientlyVisible(),
nil);
On 2017/03/07 15:28:33, baxley wrote:
> On 2017/03/07 02:56:41, Eugene But wrote:
> > I just noticed that some matchers have |grey_sufficientlyVisible| and some
> > don't. I to be fair it's quite unexpected to me that these methods have
> > |grey_sufficientlyVisible| check. Do we even need this check? If so, should
we
> > use a different function name (f.e.
> SufficientlyVisibleNavigationBarDoneButton)?
> 
> There was one at least one test that needed it, so I just put it in here. I
> think it's better to not have it, so I removed it and created a local variable
> matcher for the test that needs it (testClearBrowsingHistory in
> history_ui_egtest).
> 
> I'm running this through the try bots to make sure nothing else needs it.
Thanks! Could you please file a bug to fix other matchers as well.

[baxley, 2017-03-07 17:49:53]

On 2017/03/07 16:33:32, Eugene But wrote:
> lgtm
> 
>
https://codereview.chromium.org/2733973002/diff/1/ios/chrome/test/earl_grey/c...
> File ios/chrome/test/earl_grey/chrome_matchers.mm (right):
> 
>
https://codereview.chromium.org/2733973002/diff/1/ios/chrome/test/earl_grey/c...
> ios/chrome/test/earl_grey/chrome_matchers.mm:237: grey_sufficientlyVisible(),
> nil);
> On 2017/03/07 15:28:33, baxley wrote:
> > On 2017/03/07 02:56:41, Eugene But wrote:
> > > I just noticed that some matchers have |grey_sufficientlyVisible| and some
> > > don't. I to be fair it's quite unexpected to me that these methods have
> > > |grey_sufficientlyVisible| check. Do we even need this check? If so,
should
> we
> > > use a different function name (f.e.
> > SufficientlyVisibleNavigationBarDoneButton)?
> > 
> > There was one at least one test that needed it, so I just put it in here. I
> > think it's better to not have it, so I removed it and created a local
variable
> > matcher for the test that needs it (testClearBrowsingHistory in
> > history_ui_egtest).
> > 
> > I'm running this through the try bots to make sure nothing else needs it.
> Thanks! Could you please file a bug to fix other matchers as well.

Done. crbug.com/699165

[baxley, 2017-03-07 17:49:57]

The CQ bit was checked by baxley@chromium.org

[commit-bot: I haz the power, 2017-03-07 17:51:03]

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-03-07 18:11:31]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1488908997952940,
"parent_rev": "e7ea29da6f1af5d6c75be744254558786100ef8f", "commit_rev":
"ba2952ea9150740fdeca13c3c973aaa1310a09de"}

[commit-bot: I haz the power, 2017-03-07 18:12:09]

Description was changed from

==========
Create shared matcher for navigation bar done button.

Update tests to use shared matcher, rather than implementing
their own.

BUG=693579
==========

to

==========
Create shared matcher for navigation bar done button.

Update tests to use shared matcher, rather than implementing
their own.

BUG=693579

Review-Url: https://codereview.chromium.org/2733973002
Cr-Commit-Position: refs/heads/master@{#455134}
Committed:
https://chromium.googlesource.com/chromium/src/+/ba2952ea9150740fdeca13c3c973...
==========

[commit-bot: I haz the power, 2017-03-07 18:12:10]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/ba2952ea9150740fdeca13c3c973...