Add helper that taps on elements in the webview.

Add helper that taps on elements in the webview.

This CL moves the reusable code out of
webViewLongPressElementForContextMenu and into
webviewRunActionOnElement.

BUG=None.

Committed: https://crrev.com/b29fa38887fc1864c8d0887eb9d9f9a167d72953
Cr-Commit-Position: refs/heads/master@{#417300}

[jif-google, 2016-09-07 16:15:15]

Description was changed from

==========
Add helper tapping on the webview.

BUG=None.
==========

to

==========
Add helper that taps on elements in the webview.

BUG=None.
==========

[jif-google, 2016-09-07 16:17:15]

Description was changed from

==========
Add helper that taps on elements in the webview.

BUG=None.
==========

to

==========
Add helper that taps on elements in the webview.

This CL moves the reusable code out of webViewLongPressElementForContextMenu and
into webviewRunActionOnElement.

BUG=None.
==========

[jif, 2016-09-07 16:18:40]

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

[jif, 2016-09-07 16:20:46]

Patchset #1 (id:1) has been deleted

[jif, 2016-09-07 16:22:01]

ptal

[Eugene But (OOO till 7-30), 2016-09-07 16:44:43]

lgtm, so you are not blocked by timezone difference

https://codereview.chromium.org/2321613002/diff/20001/ios/web/public/test/ear...
File ios/web/public/test/earl_grey/web_view_actions.h (right):

https://codereview.chromium.org/2321613002/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.h:36: id<GREYAction>
webViewPressElement(WebState* state,
s/webViewPressElement/webViewTapElement ?

https://codereview.chromium.org/2321613002/diff/20001/ios/web/public/test/ear...
File ios/web/public/test/earl_grey/web_view_actions.mm (right):

https://codereview.chromium.org/2321613002/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.mm:111: id<GREYAction>
webviewRunActionOnElement(
This method does not run an action, but constructs with the given block. It is
very hard to communicate what this method does via it's name, comments and
arguments. It's main functionality is creating failure action so how about
moving failure action to it's own method:

id<GREYAction> webViewElementNotFound(const std::string& element_id)

and reusing it in other action methods:

id<GREYAction> webViewTapElement() {
  CGRect rect = web::test::GetBoundingRectOfElementWithId(state, element_id);
  return CGRectIsEmpty(rect) ? webViewElementNotFound(element_id) :
grey_tapAtPoint(point);
}

id<GREYAction> webViewLongPressElementForContextMenu() {
  CGRect rect = web::test::GetBoundingRectOfElementWithId(state, element_id);
  if (CGRectIsEmpty(rect)) {
    return webViewElementNotFound(element_id);
  }
  id<GREYAction> longpress = grey_longPressAtPointWithDuration(point,
kContextMenuLongPressDuration);
  if (triggers_context_menu) {
    return longpress;
  }
  return webViewVerifiedActionOnElement(state, longpress, element_id_copy);
}

[Eugene But (OOO till 7-30), 2016-09-07 16:56:38]

https://codereview.chromium.org/2321613002/diff/20001/ios/web/public/test/ear...
File ios/web/public/test/earl_grey/web_view_actions.mm (right):

https://codereview.chromium.org/2321613002/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.mm:233: return
grey_tapAtPoint(point);
Oh and this should be webViewVerifiedActionOnElement. The action should fail if
tap did not happen.

[jif, 2016-09-08 13:41:01]

thanks

https://codereview.chromium.org/2321613002/diff/20001/ios/web/public/test/ear...
File ios/web/public/test/earl_grey/web_view_actions.h (right):

https://codereview.chromium.org/2321613002/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.h:36: id<GREYAction>
webViewPressElement(WebState* state,
On 2016/09/07 16:44:43, Eugene But wrote:
> s/webViewPressElement/webViewTapElement ?

Done.

https://codereview.chromium.org/2321613002/diff/20001/ios/web/public/test/ear...
File ios/web/public/test/earl_grey/web_view_actions.mm (right):

https://codereview.chromium.org/2321613002/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.mm:111: id<GREYAction>
webviewRunActionOnElement(
On 2016/09/07 16:44:43, Eugene But wrote:
> This method does not run an action, but constructs with the given block. It is
> very hard to communicate what this method does via it's name, comments and
> arguments. It's main functionality is creating failure action so how about
> moving failure action to it's own method:
> 
> id<GREYAction> webViewElementNotFound(const std::string& element_id)
> 
> and reusing it in other action methods:
> 
> id<GREYAction> webViewTapElement() {
>   CGRect rect = web::test::GetBoundingRectOfElementWithId(state, element_id);
>   return CGRectIsEmpty(rect) ? webViewElementNotFound(element_id) :
> grey_tapAtPoint(point);
> }
> 
> id<GREYAction> webViewLongPressElementForContextMenu() {
>   CGRect rect = web::test::GetBoundingRectOfElementWithId(state, element_id);
>   if (CGRectIsEmpty(rect)) {
>     return webViewElementNotFound(element_id);
>   }
>   id<GREYAction> longpress = grey_longPressAtPointWithDuration(point,
> kContextMenuLongPressDuration);
>   if (triggers_context_menu) {
>     return longpress;
>   }
>   return webViewVerifiedActionOnElement(state, longpress, element_id_copy);
> }

Sounds better, thanks.

https://codereview.chromium.org/2321613002/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.mm:233: return
grey_tapAtPoint(point);
On 2016/09/07 16:56:38, Eugene But wrote:
> Oh and this should be webViewVerifiedActionOnElement. The action should fail
if
> tap did not happen.

Done.

[jif, 2016-09-08 13:41:12]

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

[commit-bot: I haz the power, 2016-09-08 13:41:26]

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, 2016-09-08 14:31:09]

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

[commit-bot: I haz the power, 2016-09-08 14:31:10]

Dry run: This issue passed the CQ dry run.

[Eugene But (OOO till 7-30), 2016-09-08 15:12:46]

lgtm! Thank you for making the changes. Could you please update CL description
to fit 72 symbols

[jif-google, 2016-09-08 15:15:32]

Description was changed from

==========
Add helper that taps on elements in the webview.

This CL moves the reusable code out of webViewLongPressElementForContextMenu and
into webviewRunActionOnElement.

BUG=None.
==========

to

==========
Add helper that taps on elements in the webview.

This CL moves the reusable code out of webViewLongPressElementForContextMenu and
into
webviewRunActionOnElement.

BUG=None.
==========

[jif-google, 2016-09-08 15:16:10]

Description was changed from

==========
Add helper that taps on elements in the webview.

This CL moves the reusable code out of webViewLongPressElementForContextMenu and
into
webviewRunActionOnElement.

BUG=None.
==========

to

==========
Add helper that taps on elements in the webview.

This CL moves the reusable code out of
webViewLongPressElementForContextMenu and into
webviewRunActionOnElement.

BUG=None.
==========

[jif, 2016-09-08 15:16:56]

On 2016/09/08 15:12:46, Eugene But wrote:
> lgtm! Thank you for making the changes. Could you please update CL description
> to fit 72 symbols

Done

[jif, 2016-09-08 15:17:03]

The CQ bit was checked by jif@chromium.org

[commit-bot: I haz the power, 2016-09-08 15:17:18]

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-09-08 15:21:19]

Description was changed from

==========
Add helper that taps on elements in the webview.

This CL moves the reusable code out of
webViewLongPressElementForContextMenu and into
webviewRunActionOnElement.

BUG=None.
==========

to

==========
Add helper that taps on elements in the webview.

This CL moves the reusable code out of
webViewLongPressElementForContextMenu and into
webviewRunActionOnElement.

BUG=None.
==========

[commit-bot: I haz the power, 2016-09-08 15:21:23]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-09-08 15:23:24]

Description was changed from

==========
Add helper that taps on elements in the webview.

This CL moves the reusable code out of
webViewLongPressElementForContextMenu and into
webviewRunActionOnElement.

BUG=None.
==========

to

==========
Add helper that taps on elements in the webview.

This CL moves the reusable code out of
webViewLongPressElementForContextMenu and into
webviewRunActionOnElement.

BUG=None.

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

[commit-bot: I haz the power, 2016-09-08 15:23:25]

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