Ensure FrameNavigationEntry is fully updated.

Ensure FrameNavigationEntry is fully updated.

Currently RendererDidNavigateToExistingPage and RendererDidNavigateToSamePage
only partially update the FrameNavigationEntry for the committing
navigation. This leads to inconsistency between sequence numbers and
PageState, which can cause origin/URL mismatches.

This CL updates both these methods to call AddOrUpdateFrameEntry to
ensure that all members are properly updated.

BUG=630103, 628677
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[nasko, 2016-08-10 00:10:24]

Description was changed from

==========
Ensure FrameNavigationEntry is fully updated.

Currently RendererDidNavigateToExistingPage and RendererDidNavigateToSamePage
only partially update the FrameNavigationEntry for the committing
navigation. This leads to inconsistency between sequence numbers and
PageState, which can cause origin/URL mismatches.

This CL updates both these methods to call AddOrUpdateFrameEntry to
ensure that all members are properly updated.

BUG=630103, 628677
==========

to

==========
Ensure FrameNavigationEntry is fully updated.

Currently RendererDidNavigateToExistingPage and RendererDidNavigateToSamePage
only partially update the FrameNavigationEntry for the committing
navigation. This leads to inconsistency between sequence numbers and
PageState, which can cause origin/URL mismatches.

This CL updates both these methods to call AddOrUpdateFrameEntry to
ensure that all members are properly updated.

BUG=630103, 628677
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[nasko, 2016-08-10 00:13:33]

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

[commit-bot: I haz the power, 2016-08-10 00:14:24]

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

[nasko, 2016-08-10 00:18:38]

Description was changed from

==========
Ensure FrameNavigationEntry is fully updated.

Currently RendererDidNavigateToExistingPage and RendererDidNavigateToSamePage
only partially update the FrameNavigationEntry for the committing
navigation. This leads to inconsistency between sequence numbers and
PageState, which can cause origin/URL mismatches.

This CL updates both these methods to call AddOrUpdateFrameEntry to
ensure that all members are properly updated.

BUG=630103, 628677
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Ensure FrameNavigationEntry is fully updated.

Currently RendererDidNavigateToExistingPage and RendererDidNavigateToSamePage
only partially update the FrameNavigationEntry for the committing
navigation. This leads to inconsistency between sequence numbers and
PageState, which can cause origin/URL mismatches.

This CL updates both these methods to call AddOrUpdateFrameEntry to
ensure that all members are properly updated.

BUG=630103, 628677
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2016-08-10 00:37:04]

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

[commit-bot: I haz the power, 2016-08-10 00:37:06]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[nasko, 2016-08-10 14:20:57]

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

[commit-bot: I haz the power, 2016-08-10 14:21:26]

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

[nasko, 2016-08-10 15:04:38]

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

[nasko, 2016-08-10 15:04:39]

Hey Charlie,
Can you review this CL for me? It fixes the known issue causing origin
mismatches.
Thanks in advance!
Nasko

[commit-bot: I haz the power, 2016-08-10 15:34:24]

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

[commit-bot: I haz the power, 2016-08-10 15:34:25]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Charlie Reis, 2016-08-10 17:03:09]

Fantastic.  LGTM with nits, once we address the one concern about flakiness in
the test below.

https://codereview.chromium.org/2224213005/diff/20001/content/browser/frame_h...
File content/browser/frame_host/navigation_controller_impl.cc (right):

https://codereview.chromium.org/2224213005/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_controller_impl.cc:1223:
static_cast<SiteInstanceImpl*>(rfh->GetSiteInstance()));
This line is redundant with AddOrUpdateFrameEntry.  We should keep the comment
and DCHECK and move them above the AddOrUpdateFrameEntry call, though.

https://codereview.chromium.org/2224213005/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_controller_impl.cc:1264:
existing_entry->SetReferrer(params.referrer);
Setting referrer is redundant with AddOrUpdateFrameEntry.

I suppose I'm ok with leaving the SetURL call, since that also clears the cached
display title.

https://codereview.chromium.org/2224213005/diff/20001/content/browser/frame_h...
File content/browser/frame_host/navigation_controller_impl_browsertest.cc
(right):

https://codereview.chromium.org/2224213005/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_controller_impl_browsertest.cc:60: class
RenderProcessGoneObserver : public content::WebContentsObserver {
Can we use RenderProcessHostWatcher from browser_test_utils, or is there a
specific need for this one?

https://codereview.chromium.org/2224213005/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_controller_impl_browsertest.cc:5641: //
members of FrameNavigationEntry. If not, it is possible to get mismatch
nit: a mismatch

https://codereview.chromium.org/2224213005/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_controller_impl_browsertest.cc:5688:
EXPECT_TRUE(!process_observer.is_gone()) << "Exit status: "
Hmm.  It would be nice to verify this beyond avoiding the crash, so that we're
not relying just on the renderer-side CHECK.

Then again, I don't see an easy way to do that.  I was hoping to use
FrameNavigateParamsCapturer to compare the URL and origin in the
FrameNavigateParams.  Unfortunately, we don't appear to have the origin on
FrameNavigateParams-- it's only on FrameHostMsg_DidCommitProvisionalLoad_Params,
which isn't easily captured or exposed.

And I suppose if we didn't have the renderer-side CHECK, we do have the origin
mismatch kill as well, so checking for process death may be ok.

I'm curious how it passes in --site-per-process, though.  Won't the back
navigation be cross-process, causing the current process to exit?  I wonder if
it's just racy, and the process does exit.  We might be able to verify by
tossing in another navigation or script call after this?

https://codereview.chromium.org/2224213005/diff/20001/content/browser/frame_h...
File content/browser/frame_host/navigation_entry_impl.h (left):

https://codereview.chromium.org/2224213005/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_entry_impl.h:233: void
ClearChildren(FrameTreeNode* frame_tree_node);
Great!  Thanks for removing this.

[nasko, 2016-08-10 18:08:51]

https://codereview.chromium.org/2224213005/diff/20001/content/browser/frame_h...
File content/browser/frame_host/navigation_controller_impl.cc (right):

https://codereview.chromium.org/2224213005/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_controller_impl.cc:1223:
static_cast<SiteInstanceImpl*>(rfh->GetSiteInstance()));
On 2016/08/10 17:03:08, Charlie Reis wrote:
> This line is redundant with AddOrUpdateFrameEntry.  We should keep the comment
> and DCHECK and move them above the AddOrUpdateFrameEntry call, though.

Done.

https://codereview.chromium.org/2224213005/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_controller_impl.cc:1264:
existing_entry->SetReferrer(params.referrer);
On 2016/08/10 17:03:09, Charlie Reis wrote:
> Setting referrer is redundant with AddOrUpdateFrameEntry.
> 
> I suppose I'm ok with leaving the SetURL call, since that also clears the
cached
> display title.

Done.

https://codereview.chromium.org/2224213005/diff/20001/content/browser/frame_h...
File content/browser/frame_host/navigation_controller_impl_browsertest.cc
(right):

https://codereview.chromium.org/2224213005/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_controller_impl_browsertest.cc:60: class
RenderProcessGoneObserver : public content::WebContentsObserver {
On 2016/08/10 17:03:09, Charlie Reis wrote:
> Can we use RenderProcessHostWatcher from browser_test_utils, or is there a
> specific need for this one?

I tried to find it, but couldn't, so put this one together. Now looking at it
though, it requires waiting for the process to exit, which the test won't do
unless I add extra code to make it exit the process. Maybe I can fold the
functionality of this class into RenderProcessHostWatcher as a follow up CL and
eliminate it from here?

https://codereview.chromium.org/2224213005/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_controller_impl_browsertest.cc:5641: //
members of FrameNavigationEntry. If not, it is possible to get mismatch
On 2016/08/10 17:03:09, Charlie Reis wrote:
> nit: a mismatch

Done.

https://codereview.chromium.org/2224213005/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_controller_impl_browsertest.cc:5688:
EXPECT_TRUE(!process_observer.is_gone()) << "Exit status: "
On 2016/08/10 17:03:09, Charlie Reis wrote:
> Hmm.  It would be nice to verify this beyond avoiding the crash, so that we're
> not relying just on the renderer-side CHECK.
> 
> Then again, I don't see an easy way to do that.  I was hoping to use
> FrameNavigateParamsCapturer to compare the URL and origin in the
> FrameNavigateParams.  Unfortunately, we don't appear to have the origin on
> FrameNavigateParams-- it's only on
FrameHostMsg_DidCommitProvisionalLoad_Params,
> which isn't easily captured or exposed.
> 
> And I suppose if we didn't have the renderer-side CHECK, we do have the origin
> mismatch kill as well, so checking for process death may be ok.
> 
> I'm curious how it passes in --site-per-process, though.  Won't the back
> navigation be cross-process, causing the current process to exit?  I wonder if
> it's just racy, and the process does exit.  We might be able to verify by
> tossing in another navigation or script call after this?

I've made it use suborigins on the same site, so we don't get cross-process
navigation. In addition, I'm checking that the origin reported by JavaScript
matches the expectation.

https://codereview.chromium.org/2224213005/diff/20001/content/browser/frame_h...
File content/browser/frame_host/navigation_entry_impl.h (left):

https://codereview.chromium.org/2224213005/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_entry_impl.h:233: void
ClearChildren(FrameTreeNode* frame_tree_node);
On 2016/08/10 17:03:09, Charlie Reis wrote:
> Great!  Thanks for removing this.

No problem! I like removing code :)

[Charlie Reis, 2016-08-10 18:36:16]

https://codereview.chromium.org/2224213005/diff/20001/content/browser/frame_h...
File content/browser/frame_host/navigation_controller_impl_browsertest.cc
(right):

https://codereview.chromium.org/2224213005/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_controller_impl_browsertest.cc:5688:
EXPECT_TRUE(!process_observer.is_gone()) << "Exit status: "
On 2016/08/10 18:08:51, nasko (slow) wrote:
> On 2016/08/10 17:03:09, Charlie Reis wrote:
> > Hmm.  It would be nice to verify this beyond avoiding the crash, so that
we're
> > not relying just on the renderer-side CHECK.
> > 
> > Then again, I don't see an easy way to do that.  I was hoping to use
> > FrameNavigateParamsCapturer to compare the URL and origin in the
> > FrameNavigateParams.  Unfortunately, we don't appear to have the origin on
> > FrameNavigateParams-- it's only on
> FrameHostMsg_DidCommitProvisionalLoad_Params,
> > which isn't easily captured or exposed.
> > 
> > And I suppose if we didn't have the renderer-side CHECK, we do have the
origin
> > mismatch kill as well, so checking for process death may be ok.
> > 
> > I'm curious how it passes in --site-per-process, though.  Won't the back
> > navigation be cross-process, causing the current process to exit?  I wonder
if
> > it's just racy, and the process does exit.  We might be able to verify by
> > tossing in another navigation or script call after this?
> 
> I've made it use suborigins on the same site, so we don't get cross-process
> navigation. In addition, I'm checking that the origin reported by JavaScript
> matches the expectation.

Thanks for adding the origin check-- that helps.

I'm not thrilled with the suborigin change, for a few reasons.  It was nice to
have some coverage for the cross-process case (on the Site Isolation FYI bot),
and it's possible the checks could become site-based instead of origin-based in
the future.

There's also still a bit of a flakiness concern for noticing the process exit
without the fix.  It probably wouldn't be an issue if we keep the CHECK in
SendDidCommitProvisionalLoad, but if we later decide the renderer kill is
sufficient and remove the renderer-side check, then process_observer might not
observe the kill before the test exits.

I think I'd suggest relying solely on the origin check (which would fail if the
process was dead), switching back to cross-site redirects, and removing
RenderProcessGoneObserver entirely.  (I think it's too easy for others to use in
a flaky way.)

[nasko, 2016-08-10 19:42:52]

Still looks good?

https://codereview.chromium.org/2224213005/diff/20001/content/browser/frame_h...
File content/browser/frame_host/navigation_controller_impl_browsertest.cc
(right):

https://codereview.chromium.org/2224213005/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_controller_impl_browsertest.cc:5688:
EXPECT_TRUE(!process_observer.is_gone()) << "Exit status: "
On 2016/08/10 18:36:16, Charlie Reis wrote:
> On 2016/08/10 18:08:51, nasko (slow) wrote:
> > On 2016/08/10 17:03:09, Charlie Reis wrote:
> > > Hmm.  It would be nice to verify this beyond avoiding the crash, so that
> we're
> > > not relying just on the renderer-side CHECK.
> > > 
> > > Then again, I don't see an easy way to do that.  I was hoping to use
> > > FrameNavigateParamsCapturer to compare the URL and origin in the
> > > FrameNavigateParams.  Unfortunately, we don't appear to have the origin on
> > > FrameNavigateParams-- it's only on
> > FrameHostMsg_DidCommitProvisionalLoad_Params,
> > > which isn't easily captured or exposed.
> > > 
> > > And I suppose if we didn't have the renderer-side CHECK, we do have the
> origin
> > > mismatch kill as well, so checking for process death may be ok.
> > > 
> > > I'm curious how it passes in --site-per-process, though.  Won't the back
> > > navigation be cross-process, causing the current process to exit?  I
wonder
> if
> > > it's just racy, and the process does exit.  We might be able to verify by
> > > tossing in another navigation or script call after this?
> > 
> > I've made it use suborigins on the same site, so we don't get cross-process
> > navigation. In addition, I'm checking that the origin reported by JavaScript
> > matches the expectation.
> 
> Thanks for adding the origin check-- that helps.
> 
> I'm not thrilled with the suborigin change, for a few reasons.  It was nice to
> have some coverage for the cross-process case (on the Site Isolation FYI bot),
> and it's possible the checks could become site-based instead of origin-based
in
> the future.
> 
> There's also still a bit of a flakiness concern for noticing the process exit
> without the fix.  It probably wouldn't be an issue if we keep the CHECK in
> SendDidCommitProvisionalLoad, but if we later decide the renderer kill is
> sufficient and remove the renderer-side check, then process_observer might not
> observe the kill before the test exits.
> 
> I think I'd suggest relying solely on the origin check (which would fail if
the
> process was dead), switching back to cross-site redirects, and removing
> RenderProcessGoneObserver entirely.  (I think it's too easy for others to use
in
> a flaky way.)

Done.

[Charlie Reis, 2016-08-10 19:49:03]

Yep, LGTM!

[nasko, 2016-08-10 19:49:32]

The CQ bit was checked by nasko@chromium.org

[commit-bot: I haz the power, 2016-08-10 19:50:04]

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-10 20:52:17]

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

[commit-bot: I haz the power, 2016-08-10 20:52:18]

Try jobs failed on following builders:
  linux_site_isolation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_site_isol...)

[nasko, 2016-08-10 21:26:56]

https://codereview.chromium.org/2224213005/diff/80001/content/browser/frame_h...
File content/browser/frame_host/navigation_controller_impl_browsertest.cc
(right):

https://codereview.chromium.org/2224213005/diff/80001/content/browser/frame_h...
content/browser/frame_host/navigation_controller_impl_browsertest.cc:5642:
embedded_test_server()->GetURL("sub.a.com", "/simple_page.html"));
Went back to using cross-origin instead of cross-site, as cross-site SAME_PAGE
navigation classification doesn't work. The test ends up getting a new session
history entry since the navigation is classified as NEW_PAGE and the entire test
is thrown off.

[nasko, 2016-08-10 21:27:02]

The CQ bit was checked by nasko@chromium.org

[nasko, 2016-08-10 21:27:03]

The patchset sent to the CQ was uploaded after l-g-t-m from creis@chromium.org
Link to the patchset: https://codereview.chromium.org/2224213005/#ps80001
(title: "Use cross-origin instead of cross-site.")

[commit-bot: I haz the power, 2016-08-10 21:27:48]

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

[Charlie Reis, 2016-08-10 21:28:58]

Still LGTM.

https://codereview.chromium.org/2224213005/diff/80001/content/browser/frame_h...
File content/browser/frame_host/navigation_controller_impl_browsertest.cc
(right):

https://codereview.chromium.org/2224213005/diff/80001/content/browser/frame_h...
content/browser/frame_host/navigation_controller_impl_browsertest.cc:5642:
embedded_test_server()->GetURL("sub.a.com", "/simple_page.html"));
On 2016/08/10 21:26:56, nasko (slow) wrote:
> Went back to using cross-origin instead of cross-site, as cross-site SAME_PAGE
> navigation classification doesn't work. The test ends up getting a new session
> history entry since the navigation is classified as NEW_PAGE and the entire
test
> is thrown off.

Yes, I could believe --site-per-process gets that wrong.  Sticking with
cross-origin sounds fine for now, and we can revisit when we get rid of
SAME_PAGE.  Thanks for the fix!

[commit-bot: I haz the power, 2016-08-10 22:38:20]

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

[commit-bot: I haz the power, 2016-08-10 22:38:22]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[nasko, 2016-08-10 23:58:09]

The CQ bit was checked by nasko@chromium.org

[nasko, 2016-08-10 23:58:09]

The patchset sent to the CQ was uploaded after l-g-t-m from creis@chromium.org
Link to the patchset: https://codereview.chromium.org/2224213005/#ps100001
(title: "Disable new test in PlzNavigate mode.")

[commit-bot: I haz the power, 2016-08-10 23:58:38]

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-11 00:03:38]

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

[commit-bot: I haz the power, 2016-08-11 00:03:39]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_site_isolation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_site_isol...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[nasko, 2016-08-11 00:24:49]

The CQ bit was checked by nasko@chromium.org

[nasko, 2016-08-11 00:24:50]

The patchset sent to the CQ was uploaded after l-g-t-m from creis@chromium.org
Link to the patchset: https://codereview.chromium.org/2224213005/#ps120001
(title: "Rebase on ToT.")

[commit-bot: I haz the power, 2016-08-11 00:25:36]

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-11 02:12:16]

Description was changed from

==========
Ensure FrameNavigationEntry is fully updated.

Currently RendererDidNavigateToExistingPage and RendererDidNavigateToSamePage
only partially update the FrameNavigationEntry for the committing
navigation. This leads to inconsistency between sequence numbers and
PageState, which can cause origin/URL mismatches.

This CL updates both these methods to call AddOrUpdateFrameEntry to
ensure that all members are properly updated.

BUG=630103, 628677
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Ensure FrameNavigationEntry is fully updated.

Currently RendererDidNavigateToExistingPage and RendererDidNavigateToSamePage
only partially update the FrameNavigationEntry for the committing
navigation. This leads to inconsistency between sequence numbers and
PageState, which can cause origin/URL mismatches.

This CL updates both these methods to call AddOrUpdateFrameEntry to
ensure that all members are properly updated.

BUG=630103, 628677
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2016-08-11 02:12:18]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-08-11 02:13:36]

Description was changed from

==========
Ensure FrameNavigationEntry is fully updated.

Currently RendererDidNavigateToExistingPage and RendererDidNavigateToSamePage
only partially update the FrameNavigationEntry for the committing
navigation. This leads to inconsistency between sequence numbers and
PageState, which can cause origin/URL mismatches.

This CL updates both these methods to call AddOrUpdateFrameEntry to
ensure that all members are properly updated.

BUG=630103, 628677
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Ensure FrameNavigationEntry is fully updated.

Currently RendererDidNavigateToExistingPage and RendererDidNavigateToSamePage
only partially update the FrameNavigationEntry for the committing
navigation. This leads to inconsistency between sequence numbers and
PageState, which can cause origin/URL mismatches.

This CL updates both these methods to call AddOrUpdateFrameEntry to
ensure that all members are properly updated.

BUG=630103, 628677
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[commit-bot: I haz the power, 2016-08-11 02:13:37]

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