Add web shell test to navigate with fragment link.

Add web shell test to navigate with fragment link.

Verify back and forward work after loading a URL with
a fragment link.

BUG=612888
Committed: https://crrev.com/6ea48e55066c2923357eab60aef828d39090ad48
Cr-Commit-Position: refs/heads/master@{#395444}

[baxley, 2016-05-18 19:03:34]

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

[baxley, 2016-05-23 16:43:53]

ping!

Let me know if you have any questions on the EarlGrey stuff, or if you think
someone else should review it.

[kkhorimoto, 2016-05-23 18:21:58]

I'm not familiar with EG yet, but I'm assuming the selectElementWithMatcher +
assertMatcher:grey_notNil is just ensuring that the element described by the
first matcher exists?  If so, then lgtm, otherwise, I'll need some more
explanation on how this works.

[baxley, 2016-05-23 18:45:30]

Description was changed from

==========
Add web shell test to navigate with fragment link.

Verify back and forward work after loading a URL with
a fragment link.

BUG=612888
==========

to

==========
Add web shell test to navigate with fragment link.

Verify back and forward work after loading a URL with
a fragment link.

BUG=612888
==========

[baxley, 2016-05-23 18:45:30]

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

[baxley, 2016-05-23 18:49:04]

On 2016/05/23 18:21:58, kkhorimoto_ wrote:
> I'm not familiar with EG yet, but I'm assuming the selectElementWithMatcher +
> assertMatcher:grey_notNil is just ensuring that the element described by the
> first matcher exists?  If so, then lgtm, otherwise, I'll need some more
> explanation on how this works.

Yes, selectElementWithMatcher walks the UI looking for an element that matches
the "matcher", and returns it. This is chained to the next method call which
performs "assertWithMatcher" on the element that was found by the matcher and
verify that it is not nil.

+eugenebut for OWNERS

[Eugene But (OOO till 7-30), 2016-05-23 20:21:58]

I was wandering if EG tests should test if appropriate WebStateObserver
callbacks are called. We had bugs where web// did not call certain callbacks
causing issues with fullscreen, swipe to navigate, cert lock and other stuff
which is generally hard to test in UI integration tests.

lgtm

https://codereview.chromium.org/1988253002/diff/60001/ios/web/shell/test/web_...
File ios/web/shell/test/web_shell_navigation_egtest.mm (right):

https://codereview.chromium.org/1988253002/diff/60001/ios/web/shell/test/web_...
ios/web/shell/test/web_shell_navigation_egtest.mm:120: const std::string
response = "<a href=\"#hash\" id=\"link\">link</a>";
Optional NIT: s/\"/'

https://codereview.chromium.org/1988253002/diff/60001/ios/web/shell/test/web_...
ios/web/shell/test/web_shell_navigation_egtest.mm:129: [[EarlGrey
selectElementWithMatcher:web::addressFieldText(URL1Text)]
Suggestion for the future:

Should |addressFieldText| take std::string instead of NSString*? This way tests
will not need to create these local variables.

[baxley, 2016-05-23 21:18:02]

Thanks for the reviews!

Regarding WebStateObserver callbacks, I'll file a bug for that, to see if
there's something we can do via code or the UI.

https://codereview.chromium.org/1988253002/diff/60001/ios/web/shell/test/web_...
File ios/web/shell/test/web_shell_navigation_egtest.mm (right):

https://codereview.chromium.org/1988253002/diff/60001/ios/web/shell/test/web_...
ios/web/shell/test/web_shell_navigation_egtest.mm:120: const std::string
response = "<a href=\"#hash\" id=\"link\">link</a>";
On 2016/05/23 20:21:58, Eugene But wrote:
> Optional NIT: s/\"/'

Done.

https://codereview.chromium.org/1988253002/diff/60001/ios/web/shell/test/web_...
ios/web/shell/test/web_shell_navigation_egtest.mm:129: [[EarlGrey
selectElementWithMatcher:web::addressFieldText(URL1Text)]
On 2016/05/23 20:21:58, Eugene But wrote:
> Suggestion for the future:
> 
> Should |addressFieldText| take std::string instead of NSString*? This way
tests
> will not need to create these local variables.

That sounds reasonable. I was hesitant since the comparison is done with
NSStrings, but it probably makes sense to do the conversion inside the helper
method, rather than require every caller to do the conversion (since the typical
use-case is to create std strings for the URLs).

I was trying to avoid having it converted twice, and that doesn't seem to be an
issue.

[baxley, 2016-05-23 21:18:46]

The CQ bit was checked by baxley@chromium.org

[baxley, 2016-05-23 21:18:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
kkhorimoto@chromium.org, eugenebut@chromium.org
Link to the patchset: https://codereview.chromium.org/1988253002/#ps80001
(title: "make html more readable with single-quotes")

[commit-bot: I haz the power, 2016-05-23 21:19:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1988253002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1988253002/80001

[commit-bot: I haz the power, 2016-05-23 22:50:56]

Description was changed from

==========
Add web shell test to navigate with fragment link.

Verify back and forward work after loading a URL with
a fragment link.

BUG=612888
==========

to

==========
Add web shell test to navigate with fragment link.

Verify back and forward work after loading a URL with
a fragment link.

BUG=612888
==========

[commit-bot: I haz the power, 2016-05-23 22:50:57]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-05-23 22:56:20]

Description was changed from

==========
Add web shell test to navigate with fragment link.

Verify back and forward work after loading a URL with
a fragment link.

BUG=612888
==========

to

==========
Add web shell test to navigate with fragment link.

Verify back and forward work after loading a URL with
a fragment link.

BUG=612888

Committed: https://crrev.com/6ea48e55066c2923357eab60aef828d39090ad48
Cr-Commit-Position: refs/heads/master@{#395444}
==========

[commit-bot: I haz the power, 2016-05-23 22:56:21]

Patchset 5 (id:??) landed as
https://crrev.com/6ea48e55066c2923357eab60aef828d39090ad48
Cr-Commit-Position: refs/heads/master@{#395444}