Fix crash for renderer-initiated same-document navigation.

Fixed crash for renderer-initiated same-document navigation.

This CL handles the case when renderer performs fragment change
navigation right after user-initiated fast back-forward navigation.

* renderer-initiated is navigation requested by a web page
  (changing the fragment in this case)
* user-initiated is navigation requested by the user via browser ui
  (tapping back button in this case)
* fast back-forward navigation is navigation where the page is
  retrieved from the page cache rather than from the network (via
  |goToBackForwardListItem:| WKWebView API in this case).
  
This is a recent regression caused by introducing NavigationContext
object, which should be created before navigation started and destroyed
after the navigation is finished. This CL creates NavigationContext
during conditions described above.

BUG=737595
TBR=kkhorimoto@chromium.org

Review-Url: https://codereview.chromium.org/2965563002
Cr-Commit-Position: refs/heads/master@{#483574}
Committed: https://chromium.googlesource.com/chromium/src/+/0a9d03ce07a9bec7a397bbb08c2bec0fe0e91d66

[Eugene But (OOO till 7-30), 2017-06-29 18:35:10]

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

[commit-bot: I haz the power, 2017-06-29 18:35:47]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Eugene But (OOO till 7-30), 2017-06-29 18:49:40]

Description was changed from

==========
Fix crash for renderer-initiated same-document navigation.

BUG=737595
==========

to

==========
Fixed crash for renderer-initiated same-document navigation.

This CL handles the case when renderer performs fragment change
navigation right after user-initiated fast back-forward navigation.

* renderer-initiated is navigation requested by a web page
  (changing the fragment in this case)
* user-initiated is navigation requested by the user via browser ui
  (tapping back button in this case)
* fast back-forward navigation is navigation where the page is
  retrieved from the page cache rather than from the network (via
  |goToBackForwardListItem:| WKWebView API in this case).
  
This is a recent regression caused by introducing NavigationContext
object, which should be created before navigation started and destroyed
after the navigation is finished. This CL creates NavigationContext
during conditions described above.

BUG=737595
==========

[commit-bot: I haz the power, 2017-06-29 18:51:19]

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

[commit-bot: I haz the power, 2017-06-29 18:51:20]

Dry run: This issue passed the CQ dry run.

[Eugene But (OOO till 7-30), 2017-06-29 20:00:08]

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

[Eugene But (OOO till 7-30), 2017-06-29 20:00:09]

Hi Danyao,

Me and Kurt are the only people who understand this code well and Kurt is out. I
know that you read our navigation code and was wandering if you would be
comfortable to review this CL? This is RBS fix and I can't want until Kurt comes
back.

Thanks!

[danyao, 2017-06-29 20:48:22]

LGTM

Do we know why the same-document navigation was not caught in
URLDidChangeWithoutDocumentChange?

[Eugene But (OOO till 7-30), 2017-06-29 22:00:45]

On 2017/06/29 20:48:22, danyao wrote:
> LGTM
> 
> Do we know why the same-document navigation was not caught in
> URLDidChangeWithoutDocumentChange?
Thanks! Because WKWebView.loading was true when URL has changed. Hopefully all
this guesswork will go away after NM start using WKBackForwardList.

[Eugene But (OOO till 7-30), 2017-06-30 01:06:10]

The CQ bit was checked by eugenebut@chromium.org

[commit-bot: I haz the power, 2017-06-30 01:06:33]

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-06-30 01:06:34]

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

[commit-bot: I haz the power, 2017-06-30 01:06:36]

No L-G-T-M from a valid reviewer yet.
CQ run can only be started once the patch has received an L-G-T-M from a full
committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a
full super star committer.
Committers are members of the group "project-chromium-committers".
Note that this has nothing to do with OWNERS files.

[Eugene But (OOO till 7-30), 2017-06-30 01:09:04]

Description was changed from

==========
Fixed crash for renderer-initiated same-document navigation.

This CL handles the case when renderer performs fragment change
navigation right after user-initiated fast back-forward navigation.

* renderer-initiated is navigation requested by a web page
  (changing the fragment in this case)
* user-initiated is navigation requested by the user via browser ui
  (tapping back button in this case)
* fast back-forward navigation is navigation where the page is
  retrieved from the page cache rather than from the network (via
  |goToBackForwardListItem:| WKWebView API in this case).
  
This is a recent regression caused by introducing NavigationContext
object, which should be created before navigation started and destroyed
after the navigation is finished. This CL creates NavigationContext
during conditions described above.

BUG=737595
==========

to

==========
Fixed crash for renderer-initiated same-document navigation.

This CL handles the case when renderer performs fragment change
navigation right after user-initiated fast back-forward navigation.

* renderer-initiated is navigation requested by a web page
  (changing the fragment in this case)
* user-initiated is navigation requested by the user via browser ui
  (tapping back button in this case)
* fast back-forward navigation is navigation where the page is
  retrieved from the page cache rather than from the network (via
  |goToBackForwardListItem:| WKWebView API in this case).
  
This is a recent regression caused by introducing NavigationContext
object, which should be created before navigation started and destroyed
after the navigation is finished. This CL creates NavigationContext
during conditions described above.

BUG=737595
TBR=kkhorimoto@chromium.org
==========

[Eugene But (OOO till 7-30), 2017-06-30 01:09:04]

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

[Eugene But (OOO till 7-30), 2017-06-30 01:09:10]

The CQ bit was checked by eugenebut@chromium.org

[commit-bot: I haz the power, 2017-06-30 01:09:23]

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-06-30 01:12:48]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1498784950850210,
"parent_rev": "ffb97bea3e1f0726b17323f29cd330001f296e51", "commit_rev":
"9f89704ea28475e26e855126853e2778f314685d"}

[commit-bot: I haz the power, 2017-06-30 01:13:13]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1498784950850210,
"parent_rev": "50aa5bdcfbfe9e773a82b268d853321c90f17f53", "commit_rev":
"0a9d03ce07a9bec7a397bbb08c2bec0fe0e91d66"}

[commit-bot: I haz the power, 2017-06-30 01:13:24]

Description was changed from

==========
Fixed crash for renderer-initiated same-document navigation.

This CL handles the case when renderer performs fragment change
navigation right after user-initiated fast back-forward navigation.

* renderer-initiated is navigation requested by a web page
  (changing the fragment in this case)
* user-initiated is navigation requested by the user via browser ui
  (tapping back button in this case)
* fast back-forward navigation is navigation where the page is
  retrieved from the page cache rather than from the network (via
  |goToBackForwardListItem:| WKWebView API in this case).
  
This is a recent regression caused by introducing NavigationContext
object, which should be created before navigation started and destroyed
after the navigation is finished. This CL creates NavigationContext
during conditions described above.

BUG=737595
TBR=kkhorimoto@chromium.org
==========

to

==========
Fixed crash for renderer-initiated same-document navigation.

This CL handles the case when renderer performs fragment change
navigation right after user-initiated fast back-forward navigation.

* renderer-initiated is navigation requested by a web page
  (changing the fragment in this case)
* user-initiated is navigation requested by the user via browser ui
  (tapping back button in this case)
* fast back-forward navigation is navigation where the page is
  retrieved from the page cache rather than from the network (via
  |goToBackForwardListItem:| WKWebView API in this case).
  
This is a recent regression caused by introducing NavigationContext
object, which should be created before navigation started and destroyed
after the navigation is finished. This CL creates NavigationContext
during conditions described above.

BUG=737595
TBR=kkhorimoto@chromium.org

Review-Url: https://codereview.chromium.org/2965563002
Cr-Commit-Position: refs/heads/master@{#483574}
Committed:
https://chromium.googlesource.com/chromium/src/+/0a9d03ce07a9bec7a397bbb08c2b...
==========

[commit-bot: I haz the power, 2017-06-30 01:13:26]

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

[cmasso1, 2017-07-05 18:51:59]

cmasso@chromium.org changed reviewers:
+ cmasso@chromium.org

[cmasso1, 2017-07-05 18:51:59]

Can we write tests to prevent this from happening again?

[Eugene But (OOO till 7-30), 2017-07-05 18:59:26]

On 2017/07/05 18:51:59, cmasso1 wrote:
> Can we write tests to prevent this from happening again?
This is very narrow use case to write EG test. WEbController is not testable, so
unit test can not be written as well. WC will be covered by tests after
refacorings which are planned in the future.