Commit af3e9e0b 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 is likely to crash, and so has debugging statements.

BUG=407376
TEST=no crashing, we hope

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

Cr-Commit-Position: refs/heads/master@{#297263}
parent c64d3dd1
...@@ -166,6 +166,13 @@ size_t RegisterChromeCrashKeys() { ...@@ -166,6 +166,13 @@ 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 },
{ "quick_discard", kSmallSize },
// End http://crbug.com/369661
// media/: // media/:
{ "VideoCaptureDeviceQTKit", kSmallSize }, { "VideoCaptureDeviceQTKit", kSmallSize },
#endif #endif
......
...@@ -1002,6 +1002,7 @@ void RenderFrameHostImpl::OnUpdateTitle( ...@@ -1002,6 +1002,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) {}
......
...@@ -1091,7 +1091,7 @@ void RenderViewHostImpl::OnUpdateState(int32 page_id, const PageState& state) { ...@@ -1091,7 +1091,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"
...@@ -3631,8 +3632,42 @@ void WebContentsImpl::RenderViewDeleted(RenderViewHost* rvh) { ...@@ -3631,8 +3632,42 @@ 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");
}
// The question being answered here is: when there is a page id mismatch
// between the renderer and the browser, was the WebContents going to throw
// out the message anyway?
if (rvh != GetRenderViewHost() &&
!GetRenderManager()->IsRVHOnSwappedOutList(
static_cast<RenderViewHostImpl*>(rvh))) {
base::debug::SetCrashKeyValue("quick_discard", "yes");
} else {
base::debug::SetCrashKeyValue("quick_discard", "no");
}
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