Context menu egtests, plus related utilities.

This CL implements basic context menu egtests that run against the web shell, along
with a number of needed utility functions.

Apologies for the size of this CL; some of the utility implementations ended up being
fairly involved.

This CL:

(1) moves ExecuteJavascript out of web_view_matchers to be a public utility function
in web_view_interaction_test_util.

(2) Add web_view_actions and shell_actions for new action providers.

(3) Adds ios/testing/earl_grey/matchers for non-web-specific matchers.

(4) Adds a "verified action" wrapper that will inject a mousedown event handler and
a script callback handler to watch for it, so that UIKit actions that ought to trigger DOM
events can be verified.

(5) Adds a BoundingRectOfElementWithId() utility to find where (in webview bounds
coordinates) an element is.

(6) Adds a method that long-presses a webview element to trigger (or not) a context
menu.

(7) Adds matchers for finding elements in context menus, and finding an element to
tap that will dismiss the context menu. The latter encapsulates the differences between
iPad and iPhone.

(8) Adds header guards to shell_matchers.h and web_view_matchers.h (!)

(9) Changes a bunch of std::string params for utility methods to be references.

(10) (finally) Implements the upstreamable context menu tests, adding a test for simply
triggering the context menu.

BUG=

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

[marq (ping after 24h), 2016-08-30 13:03:19]

Description was changed from

==========
[WORK IN PROGRESS] verified tap and longPress.

Very dirty code, lots of debugging and commented-out temp code.

Location and composition of functions is by no means final.

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

to

==========
This CL implements basic context menu egtests that run against the web shell,
along
with a number of needed utility functions.

Apologies for the size of this CL; some of the utility implementations ended up
being
fairly involved.

This CL:

(1) moves ExecuteJavascript out of web_view_matchers to be a public utility
function
in web_view_interaction_test_util.

(2) Add web_view_actions and shell_actions for new action providers.

(3) Adds ios/testing/earl_grey/matchers for non-web-specific matchers.

(4) Adds a "verified action" wrapper that will inject a mousedown event handler
and
a script callback handler to watch for it, so that UIKit actions that ought to
trigger DOM
events can be verified.

(5) Adds a BoundingRectOfElementWithId() utility to find where (in webview
bounds
coordinates) an element is.

(6) Adds a method that long-presses a webview element to trigger (or not) a
context
menu.

(7) Adds matchers for finding elements in context menus, and finding an element
to 
tap that will dismiss the context menu. The latter encapsulates the differences
between
iPad and iPhone. 

(8) (finally) Implements the upstreamable context menu tests, adding a test for
simply
triggering the context menu. 

Currently testContextMenuWebkitTouchCalloutNoneFromAncestor() is failing on
iPad; I'll resolve that before landing, but wanted to get the reviews of this CL
started.


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

[marq (ping after 24h), 2016-08-30 13:03:19]

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

[marq (ping after 24h), 2016-08-30 17:02:30]

Description was changed from

==========
This CL implements basic context menu egtests that run against the web shell,
along
with a number of needed utility functions.

Apologies for the size of this CL; some of the utility implementations ended up
being
fairly involved.

This CL:

(1) moves ExecuteJavascript out of web_view_matchers to be a public utility
function
in web_view_interaction_test_util.

(2) Add web_view_actions and shell_actions for new action providers.

(3) Adds ios/testing/earl_grey/matchers for non-web-specific matchers.

(4) Adds a "verified action" wrapper that will inject a mousedown event handler
and
a script callback handler to watch for it, so that UIKit actions that ought to
trigger DOM
events can be verified.

(5) Adds a BoundingRectOfElementWithId() utility to find where (in webview
bounds
coordinates) an element is.

(6) Adds a method that long-presses a webview element to trigger (or not) a
context
menu.

(7) Adds matchers for finding elements in context menus, and finding an element
to 
tap that will dismiss the context menu. The latter encapsulates the differences
between
iPad and iPhone. 

(8) (finally) Implements the upstreamable context menu tests, adding a test for
simply
triggering the context menu. 

Currently testContextMenuWebkitTouchCalloutNoneFromAncestor() is failing on
iPad; I'll resolve that before landing, but wanted to get the reviews of this CL
started.


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

to

==========
This CL implements basic context menu egtests that run against the web shell,
along
with a number of needed utility functions.

Apologies for the size of this CL; some of the utility implementations ended up
being
fairly involved.

This CL:

(1) moves ExecuteJavascript out of web_view_matchers to be a public utility
function
in web_view_interaction_test_util.

(2) Add web_view_actions and shell_actions for new action providers.

(3) Adds ios/testing/earl_grey/matchers for non-web-specific matchers.

(4) Adds a "verified action" wrapper that will inject a mousedown event handler
and
a script callback handler to watch for it, so that UIKit actions that ought to
trigger DOM
events can be verified.

(5) Adds a BoundingRectOfElementWithId() utility to find where (in webview
bounds
coordinates) an element is.

(6) Adds a method that long-presses a webview element to trigger (or not) a
context
menu.

(7) Adds matchers for finding elements in context menus, and finding an element
to 
tap that will dismiss the context menu. The latter encapsulates the differences
between
iPad and iPhone. 

(8) Adds header guards to shell_matchers.h and web_view_matchers.h (!)

(9) (finally) Implements the upstreamable context menu tests, adding a test for
simply
triggering the context menu. 

Currently testContextMenuWebkitTouchCalloutNoneFromAncestor() is failing on
iPad; I'll resolve that before landing, but wanted to get the reviews of this CL
started.


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

[Eugene But (OOO till 7-30), 2016-08-30 18:11:06]

Thanks for all helpers!

https://codereview.chromium.org/2275303004/diff/20001/ios/testing/earl_grey/m...
File ios/testing/earl_grey/matchers.h (right):

https://codereview.chromium.org/2275303004/diff/20001/ios/testing/earl_grey/m...
ios/testing/earl_grey/matchers.h:15: id<GREYMatcher>
contextMenuItemContainingText(NSString* text);
The name suggests that |text| can be a substring of item's text. But it looks
like implementation does full match, not  only substring.

https://codereview.chromium.org/2275303004/diff/20001/ios/testing/earl_grey/m...
ios/testing/earl_grey/matchers.h:17: // Matcher for a UI element to tap to
dismiss the the context menu.
Maybe document the difference between iPhone and iPad?

https://codereview.chromium.org/2275303004/diff/20001/ios/testing/earl_grey/m...
File ios/testing/earl_grey/matchers.mm (right):

https://codereview.chromium.org/2275303004/diff/20001/ios/testing/earl_grey/m...
ios/testing/earl_grey/matchers.mm:31:
NSClassFromString(@"CRWWebControllerContainerView")),
CRWWebControllerContainerView is a private web class. And if we change it the
test will fail. Can you avoid this dependency?

https://codereview.chromium.org/2275303004/diff/20001/ios/testing/earl_grey/m...
ios/testing/earl_grey/matchers.mm:35: return
contextMenuItemContainingText(@"Cancel");
Can this string be localized somehow?

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

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.h:8: #import <string>
s/import/include

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.h:22: std::string element_id);
const std::string& ?

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.h:30: void
LongPressWebViewElementForContextMenu(WebState* state,
Is there a reason why this is not inside web_view_interaction_test_util.h? 

Feels similar to TapWebViewElementWithId and is not actually GREYAction.

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

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.mm:13: #include
"ios/testing/earl_grey/wait_util.h"
s/include/import

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.mm:16: #include
"ios/web/web_state/web_state_impl.h"
s/include/import

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.mm:49: "    return 'Element ' +
elementId + ' not found';"
WKWebView allows returning objects. Do you want to return object with
result/error-message pair?

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.mm:89: auto callback =
base::BindBlock(^bool(const base::DictionaryValue& JSON,
s/JSON/json

Also origin_url and user_is_interacting

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.mm:89: auto callback =
base::BindBlock(^bool(const base::DictionaryValue& JSON,
NIT: Do you want to comment-out first param as well?

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.mm:101: // Remove the injected
callback.
s/Remove/Removes

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.mm:139: NSDictionary* userInfo =
@{NSLocalizedDescriptionKey : description};
s/userInfo/user_info

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.mm:191: CGPoint point =
CGPointMake(CGRectGetMidX(rect), CGRectGetMidY(rect));
Should this be something more sophisticated like |tappablePointInRect:| from
downstream code?

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

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_matchers.h:8: #import <string>
Optional NIT: Since you touching this, could you please s/import/include

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

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_matchers.mm:17: #import
"ios/web/public/test/web_view_interaction_test_util.h"
Do you need this include?

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_matchers.mm:18: #include
"ios/web/web_state/web_state_impl.h"
Do you need to import web_state_impl? Maybe importing web_state.h is sufficient?

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/web...
File ios/web/public/test/web_view_interaction_test_util.h (right):

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/web...
ios/web/public/test/web_view_interaction_test_util.h:18:
std::unique_ptr<base::Value> ExecuteScript(web::WebState* web_state,
s/ExecuteScript/ExecuteJavaScript to be consistent with bunch of
ExecuteJavaScript methods

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/web...
ios/web/public/test/web_view_interaction_test_util.h:26: CGRect
BoundingRectOfElementWithId(web::WebState* web_state,
Maybe s/BoundingRectOfElementWithId/GetBoundingElementRect ? Current name sounds
like a good Objective-C name :)

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/web...
File ios/web/public/test/web_view_interaction_test_util.mm (right):

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/web...
ios/web/public/test/web_view_interaction_test_util.mm:90: CGRect bounds =
CGRectZero;
NIT: Do you need this variable? It's lifetime quite long and simply returning
CGRectZero or result of CGRectMake could be better.

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/web...
ios/web/public/test/web_view_interaction_test_util.mm:114: CGFloat scale =
[[web_controller.webViewProxy scrollViewProxy] zoomScale];
No need to use web controller here:

web_state->GetWebViewProxy()

https://codereview.chromium.org/2275303004/diff/20001/ios/web/shell/test/cont...
File ios/web/shell/test/context_menu_egtest.mm (right):

https://codereview.chromium.org/2275303004/diff/20001/ios/web/shell/test/cont...
ios/web/shell/test/context_menu_egtest.mm:9: #import <EarlGrey/EarlGrey.h>
NIT: Please put this import along with other frameworks

https://codereview.chromium.org/2275303004/diff/20001/ios/web/shell/test/cont...
ios/web/shell/test/context_menu_egtest.mm:38: 
Drop endline

https://codereview.chromium.org/2275303004/diff/20001/ios/web/shell/test/cont...
ios/web/shell/test/context_menu_egtest.mm:45: std::string linkId = "link";
s/linkId/linkID

Please fix in other places as well

https://codereview.chromium.org/2275303004/diff/20001/ios/web/shell/test/cont...
ios/web/shell/test/context_menu_egtest.mm:59: id<GREYMatcher> copyItem =
contextMenuItemContainingText(@"Copy");
NIT: Is there a reason why contextMenuItemContainingText call is not inlined?

Same question for other places where |copyItem| variable is defined.

https://codereview.chromium.org/2275303004/diff/20001/ios/web/shell/test/cont...
ios/web/shell/test/context_menu_egtest.mm:61: // Context menu should have a
"copy" item.
System context menu also has "Copy" item. Should we also check that "Open" does
not present? Of change web shell and rename Copy menu item there?

https://codereview.chromium.org/2275303004/diff/20001/ios/web/shell/test/earl...
File ios/web/shell/test/earl_grey/shell_actions.h (right):

https://codereview.chromium.org/2275303004/diff/20001/ios/web/shell/test/earl...
ios/web/shell/test/earl_grey/shell_actions.h:8: #import <string>
s/import/include

https://codereview.chromium.org/2275303004/diff/20001/ios/web/shell/test/earl...
ios/web/shell/test/earl_grey/shell_actions.h:20: void
LongPressWebViewElementForContextMenu(std::string element_id,
const std::string& ?

[baxley, 2016-08-31 04:25:43]

Thanks for all the work on this. This is an amazing CL!

https://codereview.chromium.org/2275303004/diff/20001/ios/testing/earl_grey/m...
File ios/testing/earl_grey/matchers.mm (right):

https://codereview.chromium.org/2275303004/diff/20001/ios/testing/earl_grey/m...
ios/testing/earl_grey/matchers.mm:31:
NSClassFromString(@"CRWWebControllerContainerView")),
nit: grey_allOf is more efficient if the matchers with less "matches" are first?
Would grey_kindOfClass be more specific than gren_not(grey_ancestor(popover))?

It finds all the matches from the first matcher. Then it runs those through the
second, and so on.

[baxley, 2016-08-31 05:27:02]

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/web...
File ios/web/public/test/web_view_interaction_test_util.mm (right):

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/web...
ios/web/public/test/web_view_interaction_test_util.mm:11: #include
"ios/testing/earl_grey/wait_util.h"
Just an FYI. I am working on a CL to remove this dependency from
ios/web/public/test (it shouldn't require EarlGrey). I don't think it should
block you, and you're only adding one use of WaitUntilCondition, which should be
fine.

I'm coordinating this change so Benoit can test his mock sign in stuff without
Earl Grey.

[marq (ping after 24h), 2016-08-31 13:07:59]

Thanks for the fast and thorough feedback! PTAL.

https://codereview.chromium.org/2275303004/diff/20001/ios/testing/earl_grey/m...
File ios/testing/earl_grey/matchers.h (right):

https://codereview.chromium.org/2275303004/diff/20001/ios/testing/earl_grey/m...
ios/testing/earl_grey/matchers.h:15: id<GREYMatcher>
contextMenuItemContainingText(NSString* text);
On 2016/08/30 18:11:04, Eugene But wrote:
> The name suggests that |text| can be a substring of item's text. But it looks
> like implementation does full match, not  only substring.

Done.

https://codereview.chromium.org/2275303004/diff/20001/ios/testing/earl_grey/m...
ios/testing/earl_grey/matchers.h:17: // Matcher for a UI element to tap to
dismiss the the context menu.
On 2016/08/30 18:11:04, Eugene But wrote:
> Maybe document the difference between iPhone and iPad?

Done.

https://codereview.chromium.org/2275303004/diff/20001/ios/testing/earl_grey/m...
File ios/testing/earl_grey/matchers.mm (right):

https://codereview.chromium.org/2275303004/diff/20001/ios/testing/earl_grey/m...
ios/testing/earl_grey/matchers.mm:31:
NSClassFromString(@"CRWWebControllerContainerView")),
On 2016/08/31 04:25:43, baxley wrote:
> nit: grey_allOf is more efficient if the matchers with less "matches" are
first?
> Would grey_kindOfClass be more specific than gren_not(grey_ancestor(popover))?
> 
> It finds all the matches from the first matcher. Then it runs those through
the
> second, and so on.

Done.

https://codereview.chromium.org/2275303004/diff/20001/ios/testing/earl_grey/m...
ios/testing/earl_grey/matchers.mm:31:
NSClassFromString(@"CRWWebControllerContainerView")),
On 2016/08/30 18:11:04, Eugene But wrote:
> CRWWebControllerContainerView is a private web class. And if we change it the
> test will fail. Can you avoid this dependency?

Yep, turns out UIKit makes this very easy by labelling a view
"PopoverDismissRegion" ;-).

https://codereview.chromium.org/2275303004/diff/20001/ios/testing/earl_grey/m...
ios/testing/earl_grey/matchers.mm:35: return
contextMenuItemContainingText(@"Cancel");
On 2016/08/30 18:11:04, Eugene But wrote:
> Can this string be localized somehow?

Yes, now callers pass in a string; if they use localized strings, they can pass
in a localized one. web_shell doesn't, so the egstest just passes in the
hardcoded @"Cancel".

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

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.h:8: #import <string>
On 2016/08/30 18:11:04, Eugene But wrote:
> s/import/include

Done.

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.h:22: std::string element_id);
On 2016/08/30 18:11:04, Eugene But wrote:
> const std::string& ?

Done.

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.h:30: void
LongPressWebViewElementForContextMenu(WebState* state,
On 2016/08/30 18:11:04, Eugene But wrote:
> Is there a reason why this is not inside web_view_interaction_test_util.h? 
> 
> Feels similar to TapWebViewElementWithId and is not actually GREYAction.

Yeah, the problem is that web_view_interaction_test_util doesn't depend on EG at
all, and the functions there don't (for example) throw EG assertions, and this
does. I'll see if I can make it into an action instead.

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

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.mm:13: #include
"ios/testing/earl_grey/wait_util.h"
On 2016/08/30 18:11:05, Eugene But wrote:
> s/include/import

Done.

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.mm:16: #include
"ios/web/web_state/web_state_impl.h"
On 2016/08/30 18:11:05, Eugene But wrote:
> s/include/import

Done.

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.mm:49: "    return 'Element ' +
elementId + ' not found';"
On 2016/08/30 18:11:05, Eugene But wrote:
> WKWebView allows returning objects. Do you want to return object with
> result/error-message pair?

Other code does that, but it's more work to parse; boolean vs. string for
success vs. failure is sufficiently expressive here. Unless you think
consistency is important, I'd rather not introduce more complexity.

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.mm:89: auto callback =
base::BindBlock(^bool(const base::DictionaryValue& JSON,
On 2016/08/30 18:11:05, Eugene But wrote:
> NIT: Do you want to comment-out first param as well?

Yes, I uncommented it during debugging.

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.mm:89: auto callback =
base::BindBlock(^bool(const base::DictionaryValue& JSON,
On 2016/08/30 18:11:05, Eugene But wrote:
> s/JSON/json
> 
> Also origin_url and user_is_interacting

Done.

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.mm:101: // Remove the injected
callback.
On 2016/08/30 18:11:05, Eugene But wrote:
> s/Remove/Removes

Done.

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.mm:139: NSDictionary* userInfo =
@{NSLocalizedDescriptionKey : description};
On 2016/08/30 18:11:05, Eugene But wrote:
> s/userInfo/user_info

Done.

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_actions.mm:191: CGPoint point =
CGPointMake(CGRectGetMidX(rect), CGRectGetMidY(rect));
On 2016/08/30 18:11:05, Eugene But wrote:
> Should this be something more sophisticated like |tappablePointInRect:| from
> downstream code?

-tappablePointInRect: is KIF code I'd have to convert, and in any case, it
doesn't make much sense because whatever logic is being used to determine
tappability is unlikely to be doing DOM inspection, which is what would matter
here.

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

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_matchers.h:8: #import <string>
On 2016/08/30 18:11:05, Eugene But wrote:
> Optional NIT: Since you touching this, could you please s/import/include

Done. Also made all the string params references.

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

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_matchers.mm:17: #import
"ios/web/public/test/web_view_interaction_test_util.h"
On 2016/08/30 18:11:05, Eugene But wrote:
> Do you need this include?

Yes, that's where ExecuteScript was moved to.

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_matchers.mm:18: #include
"ios/web/web_state/web_state_impl.h"
On 2016/08/30 18:11:05, Eugene But wrote:
> Do you need to import web_state_impl? Maybe importing web_state.h is
sufficient?

Correct, I needed it at one point but not now.

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/web...
File ios/web/public/test/web_view_interaction_test_util.h (right):

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/web...
ios/web/public/test/web_view_interaction_test_util.h:18:
std::unique_ptr<base::Value> ExecuteScript(web::WebState* web_state,
On 2016/08/30 18:11:05, Eugene But wrote:
> s/ExecuteScript/ExecuteJavaScript to be consistent with bunch of
> ExecuteJavaScript methods

Done.

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/web...
ios/web/public/test/web_view_interaction_test_util.h:26: CGRect
BoundingRectOfElementWithId(web::WebState* web_state,
On 2016/08/30 18:11:05, Eugene But wrote:
> Maybe s/BoundingRectOfElementWithId/GetBoundingElementRect ? Current name
sounds
> like a good Objective-C name :)

Yeah, I can write Objective-C in any language.

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/web...
File ios/web/public/test/web_view_interaction_test_util.mm (right):

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/web...
ios/web/public/test/web_view_interaction_test_util.mm:11: #include
"ios/testing/earl_grey/wait_util.h"
On 2016/08/31 05:27:02, baxley wrote:
> Just an FYI. I am working on a CL to remove this dependency from
> ios/web/public/test (it shouldn't require EarlGrey). I don't think it should
> block you, and you're only adding one use of WaitUntilCondition, which should
be
> fine.
> 
> I'm coordinating this change so Benoit can test his mock sign in stuff without
> Earl Grey.

Acknowledged.

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/web...
ios/web/public/test/web_view_interaction_test_util.mm:90: CGRect bounds =
CGRectZero;
On 2016/08/30 18:11:05, Eugene But wrote:
> NIT: Do you need this variable? It's lifetime quite long and simply returning
> CGRectZero or result of CGRectMake could be better.

Good catch, done.

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/web...
ios/web/public/test/web_view_interaction_test_util.mm:114: CGFloat scale =
[[web_controller.webViewProxy scrollViewProxy] zoomScale];
On 2016/08/30 18:11:05, Eugene But wrote:
> No need to use web controller here:
> 
> web_state->GetWebViewProxy()

Done.

https://codereview.chromium.org/2275303004/diff/20001/ios/web/shell/test/cont...
File ios/web/shell/test/context_menu_egtest.mm (right):

https://codereview.chromium.org/2275303004/diff/20001/ios/web/shell/test/cont...
ios/web/shell/test/context_menu_egtest.mm:9: #import <EarlGrey/EarlGrey.h>
On 2016/08/30 18:11:05, Eugene But wrote:
> NIT: Please put this import along with other frameworks

Done.

https://codereview.chromium.org/2275303004/diff/20001/ios/web/shell/test/cont...
ios/web/shell/test/context_menu_egtest.mm:38: 
On 2016/08/30 18:11:05, Eugene But wrote:
> Drop endline

Done.

https://codereview.chromium.org/2275303004/diff/20001/ios/web/shell/test/cont...
ios/web/shell/test/context_menu_egtest.mm:45: std::string linkId = "link";
On 2016/08/30 18:11:05, Eugene But wrote:
> s/linkId/linkID
> 
> Please fix in other places as well

Done.

https://codereview.chromium.org/2275303004/diff/20001/ios/web/shell/test/cont...
ios/web/shell/test/context_menu_egtest.mm:59: id<GREYMatcher> copyItem =
contextMenuItemContainingText(@"Copy");
On 2016/08/30 18:11:05, Eugene But wrote:
> NIT: Is there a reason why contextMenuItemContainingText call is not inlined?
> 
> Same question for other places where |copyItem| variable is defined.

In two cases |copyItem| is used twice, so I'd rather have it defined only once.

In the other two cases it prevents hard-to-read line wrapping.

https://codereview.chromium.org/2275303004/diff/20001/ios/web/shell/test/cont...
ios/web/shell/test/context_menu_egtest.mm:61: // Context menu should have a
"copy" item.
On 2016/08/30 18:11:05, Eugene But wrote:
> System context menu also has "Copy" item. Should we also check that "Open"
does
> not present? Of change web shell and rename Copy menu item there?

Let's make the web shell item "Copy Link".

https://codereview.chromium.org/2275303004/diff/20001/ios/web/shell/test/earl...
File ios/web/shell/test/earl_grey/shell_actions.h (right):

https://codereview.chromium.org/2275303004/diff/20001/ios/web/shell/test/earl...
ios/web/shell/test/earl_grey/shell_actions.h:8: #import <string>
On 2016/08/30 18:11:05, Eugene But wrote:
> s/import/include

Done.

https://codereview.chromium.org/2275303004/diff/20001/ios/web/shell/test/earl...
ios/web/shell/test/earl_grey/shell_actions.h:20: void
LongPressWebViewElementForContextMenu(std::string element_id,
On 2016/08/30 18:11:05, Eugene But wrote:
> const std::string& ?

Done.

[marq (ping after 24h), 2016-08-31 13:08:51]

Description was changed from

==========
This CL implements basic context menu egtests that run against the web shell,
along
with a number of needed utility functions.

Apologies for the size of this CL; some of the utility implementations ended up
being
fairly involved.

This CL:

(1) moves ExecuteJavascript out of web_view_matchers to be a public utility
function
in web_view_interaction_test_util.

(2) Add web_view_actions and shell_actions for new action providers.

(3) Adds ios/testing/earl_grey/matchers for non-web-specific matchers.

(4) Adds a "verified action" wrapper that will inject a mousedown event handler
and
a script callback handler to watch for it, so that UIKit actions that ought to
trigger DOM
events can be verified.

(5) Adds a BoundingRectOfElementWithId() utility to find where (in webview
bounds
coordinates) an element is.

(6) Adds a method that long-presses a webview element to trigger (or not) a
context
menu.

(7) Adds matchers for finding elements in context menus, and finding an element
to 
tap that will dismiss the context menu. The latter encapsulates the differences
between
iPad and iPhone. 

(8) Adds header guards to shell_matchers.h and web_view_matchers.h (!)

(9) (finally) Implements the upstreamable context menu tests, adding a test for
simply
triggering the context menu. 

Currently testContextMenuWebkitTouchCalloutNoneFromAncestor() is failing on
iPad; I'll resolve that before landing, but wanted to get the reviews of this CL
started.


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

to

==========
This CL implements basic context menu egtests that run against the web shell,
along
with a number of needed utility functions.

Apologies for the size of this CL; some of the utility implementations ended up
being
fairly involved.

This CL:

(1) moves ExecuteJavascript out of web_view_matchers to be a public utility
function
in web_view_interaction_test_util.

(2) Add web_view_actions and shell_actions for new action providers.

(3) Adds ios/testing/earl_grey/matchers for non-web-specific matchers.

(4) Adds a "verified action" wrapper that will inject a mousedown event handler
and
a script callback handler to watch for it, so that UIKit actions that ought to
trigger DOM
events can be verified.

(5) Adds a BoundingRectOfElementWithId() utility to find where (in webview
bounds
coordinates) an element is.

(6) Adds a method that long-presses a webview element to trigger (or not) a
context
menu.

(7) Adds matchers for finding elements in context menus, and finding an element
to 
tap that will dismiss the context menu. The latter encapsulates the differences
between
iPad and iPhone. 

(8) Adds header guards to shell_matchers.h and web_view_matchers.h (!)

(9) Changes a bunch of std::string params for utility methods to be references.

(10) (finally) Implements the upstreamable context menu tests, adding a test for
simply
triggering the context menu. 


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

[Eugene But (OOO till 7-30), 2016-08-31 15:28:00]

lgtm with fixing references capturing inside the blocks

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

https://codereview.chromium.org/2275303004/diff/20001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_matchers.h:8: #import <string>
On 2016/08/31 13:07:58, marq wrote:
> On 2016/08/30 18:11:05, Eugene But wrote:
> > Optional NIT: Since you touching this, could you please s/import/include
> 
> Done. Also made all the string params references.
Moving to references will break tests.

https://codereview.chromium.org/2275303004/diff/40001/ios/web/public/test/ear...
File ios/web/public/test/earl_grey/web_view_matchers.mm (right):

https://codereview.chromium.org/2275303004/diff/40001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_matchers.mm:47: id<GREYMatcher>
webViewContainingText(const std::string& text,
This should be string by value or local variable (|text| is used inside the
block).

https://codereview.chromium.org/2275303004/diff/40001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_matchers.mm:79: id<GREYMatcher>
webViewContainingBlockedImage(const std::string& image_id,
ditto

https://codereview.chromium.org/2275303004/diff/40001/ios/web/public/test/ear...
ios/web/public/test/earl_grey/web_view_matchers.mm:131: id<GREYMatcher>
webViewCssSelector(const std::string& selector,
ditto

https://codereview.chromium.org/2275303004/diff/40001/ios/web/shell/test/earl...
File ios/web/shell/test/earl_grey/shell_matchers.mm (right):

https://codereview.chromium.org/2275303004/diff/40001/ios/web/shell/test/earl...
ios/web/shell/test/earl_grey/shell_matchers.mm:36: id<GREYMatcher>
addressFieldText(const std::string& text) {
This is going to break tests

[marq (ping after 24h), 2016-08-31 16:19:47]

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

[commit-bot: I haz the power, 2016-08-31 16:20:01]

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

[marq (ping after 24h), 2016-08-31 16:20:04]

The CQ bit was unchecked by marq@chromium.org

[marq (ping after 24h), 2016-08-31 16:20:18]

The CQ bit was checked by marq@chromium.org

[marq (ping after 24h), 2016-08-31 16:20:19]

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

[commit-bot: I haz the power, 2016-08-31 16:20:37]

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-08-31 17:24:09]

Description was changed from

==========
This CL implements basic context menu egtests that run against the web shell,
along
with a number of needed utility functions.

Apologies for the size of this CL; some of the utility implementations ended up
being
fairly involved.

This CL:

(1) moves ExecuteJavascript out of web_view_matchers to be a public utility
function
in web_view_interaction_test_util.

(2) Add web_view_actions and shell_actions for new action providers.

(3) Adds ios/testing/earl_grey/matchers for non-web-specific matchers.

(4) Adds a "verified action" wrapper that will inject a mousedown event handler
and
a script callback handler to watch for it, so that UIKit actions that ought to
trigger DOM
events can be verified.

(5) Adds a BoundingRectOfElementWithId() utility to find where (in webview
bounds
coordinates) an element is.

(6) Adds a method that long-presses a webview element to trigger (or not) a
context
menu.

(7) Adds matchers for finding elements in context menus, and finding an element
to 
tap that will dismiss the context menu. The latter encapsulates the differences
between
iPad and iPhone. 

(8) Adds header guards to shell_matchers.h and web_view_matchers.h (!)

(9) Changes a bunch of std::string params for utility methods to be references.

(10) (finally) Implements the upstreamable context menu tests, adding a test for
simply
triggering the context menu. 


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

to

==========
This CL implements basic context menu egtests that run against the web shell,
along
with a number of needed utility functions.

Apologies for the size of this CL; some of the utility implementations ended up
being
fairly involved.

This CL:

(1) moves ExecuteJavascript out of web_view_matchers to be a public utility
function
in web_view_interaction_test_util.

(2) Add web_view_actions and shell_actions for new action providers.

(3) Adds ios/testing/earl_grey/matchers for non-web-specific matchers.

(4) Adds a "verified action" wrapper that will inject a mousedown event handler
and
a script callback handler to watch for it, so that UIKit actions that ought to
trigger DOM
events can be verified.

(5) Adds a BoundingRectOfElementWithId() utility to find where (in webview
bounds
coordinates) an element is.

(6) Adds a method that long-presses a webview element to trigger (or not) a
context
menu.

(7) Adds matchers for finding elements in context menus, and finding an element
to 
tap that will dismiss the context menu. The latter encapsulates the differences
between
iPad and iPhone. 

(8) Adds header guards to shell_matchers.h and web_view_matchers.h (!)

(9) Changes a bunch of std::string params for utility methods to be references.

(10) (finally) Implements the upstreamable context menu tests, adding a test for
simply
triggering the context menu. 


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

[commit-bot: I haz the power, 2016-08-31 17:24:10]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-08-31 17:26:22]

Description was changed from

==========
This CL implements basic context menu egtests that run against the web shell,
along
with a number of needed utility functions.

Apologies for the size of this CL; some of the utility implementations ended up
being
fairly involved.

This CL:

(1) moves ExecuteJavascript out of web_view_matchers to be a public utility
function
in web_view_interaction_test_util.

(2) Add web_view_actions and shell_actions for new action providers.

(3) Adds ios/testing/earl_grey/matchers for non-web-specific matchers.

(4) Adds a "verified action" wrapper that will inject a mousedown event handler
and
a script callback handler to watch for it, so that UIKit actions that ought to
trigger DOM
events can be verified.

(5) Adds a BoundingRectOfElementWithId() utility to find where (in webview
bounds
coordinates) an element is.

(6) Adds a method that long-presses a webview element to trigger (or not) a
context
menu.

(7) Adds matchers for finding elements in context menus, and finding an element
to 
tap that will dismiss the context menu. The latter encapsulates the differences
between
iPad and iPhone. 

(8) Adds header guards to shell_matchers.h and web_view_matchers.h (!)

(9) Changes a bunch of std::string params for utility methods to be references.

(10) (finally) Implements the upstreamable context menu tests, adding a test for
simply
triggering the context menu. 


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

to

==========
This CL implements basic context menu egtests that run against the web shell,
along
with a number of needed utility functions.

Apologies for the size of this CL; some of the utility implementations ended up
being
fairly involved.

This CL:

(1) moves ExecuteJavascript out of web_view_matchers to be a public utility
function
in web_view_interaction_test_util.

(2) Add web_view_actions and shell_actions for new action providers.

(3) Adds ios/testing/earl_grey/matchers for non-web-specific matchers.

(4) Adds a "verified action" wrapper that will inject a mousedown event handler
and
a script callback handler to watch for it, so that UIKit actions that ought to
trigger DOM
events can be verified.

(5) Adds a BoundingRectOfElementWithId() utility to find where (in webview
bounds
coordinates) an element is.

(6) Adds a method that long-presses a webview element to trigger (or not) a
context
menu.

(7) Adds matchers for finding elements in context menus, and finding an element
to
tap that will dismiss the context menu. The latter encapsulates the differences
between
iPad and iPhone.

(8) Adds header guards to shell_matchers.h and web_view_matchers.h (!)

(9) Changes a bunch of std::string params for utility methods to be references.

(10) (finally) Implements the upstreamable context menu tests, adding a test for
simply
triggering the context menu.

BUG=

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

[commit-bot: I haz the power, 2016-08-31 17:26:23]

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