Clean up NavigationControllerImpl::IsURLInPageNavigation

Clean up NavigationControllerImpl::IsURLInPageNavigation

IsURLInPageNavigation was incorrectly being called twice.  This led to
duplicate attempts to try to kill the renderer process when it failed,
and it ran in slightly different states each time.  This CL ensures we
only call it once in the correct state.

BUG=618104
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/1b0f797b91e108f96e75fc668a12b194b30b640e
Cr-Commit-Position: refs/heads/master@{#421557}

[Yeol Park, 2016-09-27 09:39:31]

Description was changed from

==========
Clean up NavigationControllerImpl::IsURLInPageNavigation

Modified for once call NavigationControllerImpl::IsURLInPageNavigation in
NavigatorImpl::DidNavigate.
IsURLInPageNavigation do not call in
NavigationControllerImpl::RendererDidNavigate.
then use 'is_navigation_within_page' variable that was already in use
NavigatorImpl::DidNavigate.

BUG=618104
==========

to

==========
Clean up NavigationControllerImpl::IsURLInPageNavigation

Modified for once call NavigationControllerImpl::IsURLInPageNavigation in
NavigatorImpl::DidNavigate.
IsURLInPageNavigation do not call in
NavigationControllerImpl::RendererDidNavigate.
then use 'is_navigation_within_page' variable that was already in use
NavigatorImpl::DidNavigate.

BUG=618104
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Yeol Park, 2016-09-27 09:40:19]

Description was changed from

==========
Clean up NavigationControllerImpl::IsURLInPageNavigation

Modified for once call NavigationControllerImpl::IsURLInPageNavigation in
NavigatorImpl::DidNavigate.
IsURLInPageNavigation do not call in
NavigationControllerImpl::RendererDidNavigate.
then use 'is_navigation_within_page' variable that was already in use
NavigatorImpl::DidNavigate.

BUG=618104
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Clean up NavigationControllerImpl::IsURLInPageNavigation

Modified for once call NavigationControllerImpl::IsURLInPageNavigation in
NavigatorImpl::DidNavigate.
IsURLInPageNavigation do not call in
NavigationControllerImpl::RendererDidNavigate.
then use 'is_navigation_within_page' variable that was already in use
NavigatorImpl::DidNavigate.

BUG=618104
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Yeol Park, 2016-09-27 09:40:19]

peary2@gmail.com changed reviewers:
+ jinho.bang@samsung.com

[zino, 2016-09-27 11:20:46]

The CQ bit was checked by jinho.bang@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-27 11:20:58]

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-27 12:44:13]

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

[commit-bot: I haz the power, 2016-09-27 12:44:14]

Dry run: This issue passed the CQ dry run.

[Yeol Park, 2016-09-27 13:07:36]

peary2@gmail.com changed reviewers:
+ avi@chromium.org

[Yeol Park, 2016-09-27 14:28:50]

PTAL

[Avi (use Gerrit), 2016-09-27 15:29:41]

avi@chromium.org changed reviewers:
+ creis@chromium.org

[Avi (use Gerrit), 2016-09-27 15:29:42]

Charlie, this looks reasonable to me. WDYT?

[Charlie Reis, 2016-09-27 18:51:43]

Thanks-- this looks good.  A few style corrections (which git cl format can
handle), and you should be set.

https://codereview.chromium.org/2376643002/diff/1/AUTHORS
File AUTHORS (right):

https://codereview.chromium.org/2376643002/diff/1/AUTHORS#newcode757
AUTHORS:757: Yeol Park <peary2@gmail.com>
Great, thanks for signing the CLA.

https://codereview.chromium.org/2376643002/diff/1/content/browser/frame_host/...
File content/browser/frame_host/navigation_controller_impl.cc (right):

https://codereview.chromium.org/2376643002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_controller_impl.cc:775:
LoadCommittedDetails* details, bool is_navigation_within_page) {
Style nit: Move to next line.

https://codereview.chromium.org/2376643002/diff/1/content/browser/frame_host/...
File content/browser/frame_host/navigation_controller_impl.h (right):

https://codereview.chromium.org/2376643002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_controller_impl.h:152:
LoadCommittedDetails* details, bool is_navigation_within_page);
Style nit: For function declarations, if all parameters don't fit on one line,
they should each be on their own line.

https://codereview.chromium.org/2376643002/diff/1/content/browser/frame_host/...
File content/browser/frame_host/navigator_impl.cc (right):

https://codereview.chromium.org/2376643002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigator_impl.cc:570: params, &details,
is_navigation_within_page);
Style nit: Wrong indent.

I recommend using "git cl format" to automatically handle issues like these.

[Yeol Park, 2016-09-28 02:05:07]

Patch set update for Style nit.
used to 'git cl format'
PTAL

[Charlie Reis, 2016-09-28 03:48:40]

Thanks, LGTM.  I'll run try jobs for it and I'll land it tomorrow if they pass.

[Charlie Reis, 2016-09-28 03:48:51]

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

[commit-bot: I haz the power, 2016-09-28 03:49:07]

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-28 05:49:18]

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

[commit-bot: I haz the power, 2016-09-28 05:49:19]

Dry run: This issue passed the CQ dry run.

[zino, 2016-09-28 14:50:47]

looks good but should update CL description.
As per the chromium style guide, each line should be less than 80byte.
Could you please update it?

[Charlie Reis, 2016-09-28 17:21:29]

Description was changed from

==========
Clean up NavigationControllerImpl::IsURLInPageNavigation

Modified for once call NavigationControllerImpl::IsURLInPageNavigation in
NavigatorImpl::DidNavigate.
IsURLInPageNavigation do not call in
NavigationControllerImpl::RendererDidNavigate.
then use 'is_navigation_within_page' variable that was already in use
NavigatorImpl::DidNavigate.

BUG=618104
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Clean up NavigationControllerImpl::IsURLInPageNavigation

IsURLInPageNavigation was incorrectly being called twice.  This led to
duplicate attempts to try to kill the renderer process when it failed,
and it ran in slightly different states each time.  This CL ensures we
only call it once in the correct state.

BUG=618104
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Charlie Reis, 2016-09-28 17:22:41]

On 2016/09/28 14:50:47, zino wrote:
> looks good but should update CL description.
> As per the chromium style guide, each line should be less than 80byte.
> Could you please update it?

Thanks for catching!  The CL description should really be more about *why* the
change is made (and what effect it has), rather than just restating what
changed.  I've rephrased it.

[Charlie Reis, 2016-09-28 17:22:50]

The CQ bit was checked by creis@chromium.org

[commit-bot: I haz the power, 2016-09-28 17:23:22]

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-28 17:28:55]

Description was changed from

==========
Clean up NavigationControllerImpl::IsURLInPageNavigation

IsURLInPageNavigation was incorrectly being called twice.  This led to
duplicate attempts to try to kill the renderer process when it failed,
and it ran in slightly different states each time.  This CL ensures we
only call it once in the correct state.

BUG=618104
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Clean up NavigationControllerImpl::IsURLInPageNavigation

IsURLInPageNavigation was incorrectly being called twice.  This led to
duplicate attempts to try to kill the renderer process when it failed,
and it ran in slightly different states each time.  This CL ensures we
only call it once in the correct state.

BUG=618104
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2016-09-28 17:28:56]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-28 17:31:41]

Description was changed from

==========
Clean up NavigationControllerImpl::IsURLInPageNavigation

IsURLInPageNavigation was incorrectly being called twice.  This led to
duplicate attempts to try to kill the renderer process when it failed,
and it ran in slightly different states each time.  This CL ensures we
only call it once in the correct state.

BUG=618104
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Clean up NavigationControllerImpl::IsURLInPageNavigation

IsURLInPageNavigation was incorrectly being called twice.  This led to
duplicate attempts to try to kill the renderer process when it failed,
and it ran in slightly different states each time.  This CL ensures we
only call it once in the correct state.

BUG=618104
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/1b0f797b91e108f96e75fc668a12b194b30b640e
Cr-Commit-Position: refs/heads/master@{#421557}
==========

[commit-bot: I haz the power, 2016-09-28 17:31:42]

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