Add a return value to TapWebViewElementWithId

ios/web/public/test/web_view_interaction_test_util.h

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "ios/web/public/web_state/web_state.h"
 
 #include <string>
 
 namespace web {
 namespace test {
 
 enum ElementAction { CLICK, FOCUS };

[Eugene But (OOO till 7-30), 2016/08/26 13:19:56]

According to Chromium Style Guide these should be ELEMENT_ACTION_CLICK and
ELEMENT_ACTION_FOCUS. Could you please fix this.

[jif-google, 2016/08/26 13:30:34]

Acknowledged.

[gambard, 2016/08/26 14:35:52]

Done.

 
-// Attempts to tap the element with |element_id| in the passed in |web_state|
-// using a JavaScript click() event.
-void TapWebViewElementWithId(web::WebState* web_state,
+// Returns whether the element with |element_id| in the passed |web_state| has
+// been tapped using a JavaScript click() event.
+bool TapWebViewElementWithId(web::WebState* web_state,
                              const std::string& element_id);
 
-// Attempts to run the Javascript action specified by |action| on |element_id|
-// in the passed |web_state|.
-void RunActionOnWebViewElementWithId(web::WebState* web_state,
+// Returns whether the Javascript action specified by |action| run on

[jif-google, 2016/08/26 12:20:48]

s/run/ran/

[gambard, 2016/08/26 14:35:52]

Done.

+// |element_id| in the passed |web_state|.
+bool RunActionOnWebViewElementWithId(web::WebState* web_state,

[Eugene But (OOO till 7-30), 2016/08/26 13:19:56]

Do you see a potential of adding new actions? If you can't think about more than
3 actions then I believe that it is better to have a separate API for each
action. Having both TapWebViewElementWithId and RunActionOnWebViewElementWithId
(which also allows clicks) seems inconsistent.

[jif-google, 2016/08/26 13:30:34]

And how about removing TapWebViewElementWithId?

[baxley, 2016/08/26 13:40:05]

We're in progress of adding an API that will tap a webview element with link
text. I think it's more clear to not have RunActionOnWebViewElementWithId
exposed to the user.

Even if we have many APIs that use it, as long as the number of APIs is bound, I
think it's more clear to have explicit methods for each action, rather than have
an open ended one. If we ever have a case (let me know if this is the case)
where we can't predict what other action types can be passed in, then I think
it's okay to expose it.

I think it's good to have RunActionOnWebViewElementWithId to share
implementation across the APIs internally.

[gambard, 2016/08/26 14:35:52]

Done.

[Eugene But (OOO till 7-30), 2016/08/26 15:13:28]

+1 for using RunActionOnWebViewElementWithId.

We we really need API to tap on link with text? What if many links have the same
text? Is there a reason why having TapWebViewElementWithId is not sufficient?

[Eugene But (OOO till 7-30), 2016/08/26 15:13:28]

TapWebViewElementWithId seems more clear that RunActionOnWebViewElementWithId

[baxley, 2016/08/26 15:43:26]

As discussed offline... chrome-urls don't have IDs, so the only way to tap them
now is via href value. However, we concluded we should pursue adding IDs for the
chrome-urls. If there is pushback, or it will take a while, we can add a method
to tap link text as a short term workaround.

                                      const std::string& element_id,
                                      ElementAction action);
 
 }  // namespace test
 }  // namespace web