Commit 31111d18 authored by avi's avatar avi Committed by Commit bot

Ensure that the browser’s copy of page id is in sync with the renderer’s.

This might-maybe-hope-not crash, and so has debugging statements.

BUG=407376
TEST=no crashing, we hope
TBR=rsesek@chromium.org

Review URL: https://codereview.chromium.org/669613005

Cr-Commit-Position: refs/heads/master@{#300311}
parent a13b5096
...@@ -166,6 +166,12 @@ size_t RegisterChromeCrashKeys() { ...@@ -166,6 +166,12 @@ size_t RegisterChromeCrashKeys() {
{ "channel_error_bt", kMediumSize }, { "channel_error_bt", kMediumSize },
{ "remove_route_bt", kMediumSize }, { "remove_route_bt", kMediumSize },
{ "rwhvm_window", kMediumSize }, { "rwhvm_window", kMediumSize },
// The following keys are for diagnosing crashes in http://crbug.com/369661.
// They will not be permanent.
{ "renderer_page_id", kSmallSize },
{ "browser_page_id", kSmallSize },
{ "last_committed_page_id", kSmallSize },
// End http://crbug.com/369661
// media/: // media/:
{ "VideoCaptureDeviceQTKit", kSmallSize }, { "VideoCaptureDeviceQTKit", kSmallSize },
#endif #endif
......
...@@ -1051,6 +1051,7 @@ void RenderFrameHostImpl::OnUpdateTitle( ...@@ -1051,6 +1051,7 @@ void RenderFrameHostImpl::OnUpdateTitle(
int32 page_id, int32 page_id,
const base::string16& title, const base::string16& title,
blink::WebTextDirection title_direction) { blink::WebTextDirection title_direction) {
CHECK_EQ(render_view_host_->page_id_, page_id);
// This message is only sent for top-level frames. TODO(avi): when frame tree // This message is only sent for top-level frames. TODO(avi): when frame tree
// mirroring works correctly, add a check here to enforce it. // mirroring works correctly, add a check here to enforce it.
if (title.length() > kMaxTitleChars) { if (title.length() > kMaxTitleChars) {
......
...@@ -106,6 +106,7 @@ class CONTENT_EXPORT RenderViewHostDelegate { ...@@ -106,6 +106,7 @@ class CONTENT_EXPORT RenderViewHostDelegate {
// The state for the page changed and should be updated. // The state for the page changed and should be updated.
virtual void UpdateState(RenderViewHost* render_view_host, virtual void UpdateState(RenderViewHost* render_view_host,
int32 rvh_page_id, // http://crbug.com/407376
int32 page_id, int32 page_id,
const PageState& state) {} const PageState& state) {}
......
...@@ -1028,7 +1028,7 @@ void RenderViewHostImpl::OnUpdateState(int32 page_id, const PageState& state) { ...@@ -1028,7 +1028,7 @@ void RenderViewHostImpl::OnUpdateState(int32 page_id, const PageState& state) {
return; return;
} }
delegate_->UpdateState(this, page_id, state); delegate_->UpdateState(this, page_id_, page_id, state);
} }
void RenderViewHostImpl::OnUpdateTargetURL(const GURL& url) { void RenderViewHostImpl::OnUpdateTargetURL(const GURL& url) {
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include <utility> #include <utility>
#include "base/command_line.h" #include "base/command_line.h"
#include "base/debug/crash_logging.h"
#include "base/debug/trace_event.h" #include "base/debug/trace_event.h"
#include "base/lazy_instance.h" #include "base/lazy_instance.h"
#include "base/logging.h" #include "base/logging.h"
...@@ -3588,8 +3589,32 @@ void WebContentsImpl::RenderViewDeleted(RenderViewHost* rvh) { ...@@ -3588,8 +3589,32 @@ void WebContentsImpl::RenderViewDeleted(RenderViewHost* rvh) {
} }
void WebContentsImpl::UpdateState(RenderViewHost* rvh, void WebContentsImpl::UpdateState(RenderViewHost* rvh,
int32 rvh_page_id,
int32 page_id, int32 page_id,
const PageState& page_state) { const PageState& page_state) {
if (rvh_page_id != page_id) {
base::debug::SetCrashKeyValue("renderer_page_id",
base::IntToString(page_id));
base::debug::SetCrashKeyValue("browser_page_id",
base::IntToString(rvh_page_id));
NavigationEntryImpl* entry = NavigationEntryImpl::FromNavigationEntry(
controller_.GetLastCommittedEntry());
// The question being answered here is: when there is a page id mismatch
// between the renderer and the browser, which value (if either) matches
// that of the last committed entry?
if (entry) {
if (entry->site_instance() == rvh->GetSiteInstance()) {
base::debug::SetCrashKeyValue("last_committed_page_id",
base::IntToString(entry->GetPageID()));
} else {
base::debug::SetCrashKeyValue("last_committed_page_id",
"site instance mismatch");
}
} else {
base::debug::SetCrashKeyValue("last_committed_page_id", "no entry");
}
CHECK(false);
}
// Ensure that this state update comes from either the active RVH or one of // Ensure that this state update comes from either the active RVH or one of
// the swapped out RVHs. We don't expect to hear from any other RVHs. // the swapped out RVHs. We don't expect to hear from any other RVHs.
// TODO(nasko): This should go through RenderFrameHost. // TODO(nasko): This should go through RenderFrameHost.
......
...@@ -412,6 +412,7 @@ class CONTENT_EXPORT WebContentsImpl ...@@ -412,6 +412,7 @@ class CONTENT_EXPORT WebContentsImpl
int error_code) override; int error_code) override;
virtual void RenderViewDeleted(RenderViewHost* render_view_host) override; virtual void RenderViewDeleted(RenderViewHost* render_view_host) override;
virtual void UpdateState(RenderViewHost* render_view_host, virtual void UpdateState(RenderViewHost* render_view_host,
int32 rvh_page_id,
int32 page_id, int32 page_id,
const PageState& page_state) override; const PageState& page_state) override;
virtual void UpdateTargetURL(const GURL& url) override; virtual void UpdateTargetURL(const GURL& url) override;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment