Gather more debug info for the switch away from page id for titles and state.

content/browser/web_contents/web_contents_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/web_contents/web_contents_impl.h"
 
 #include <utility>
 
 #include "base/command_line.h"
+#include "base/debug/crash_logging.h"
 #include "base/lazy_instance.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/process/process.h"
 #include "base/profiler/scoped_tracker.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
@@ skipping 3864 matching lines @@
   // when the user is scrolling and entering form data, as well as when we're
   // leaving a page, in which case our state may have already been moved to
   // the next page. The navigation controller will look up the appropriate
   // NavigationEntry and update it when it is notified via the delegate.
   RenderViewHostImpl* rvhi = static_cast<RenderViewHostImpl*>(rvh);
   NavigationEntryImpl* entry = controller_.GetEntryWithPageID(
       rvhi->GetSiteInstance(), page_id);
   if (!entry)
     return;
 
-  NavigationEntryImpl* new_entry = controller_.GetEntryWithUniqueID(
-      static_cast<RenderFrameHostImpl*>(rvhi->GetMainFrame())->nav_entry_id());
+  int nav_entry_id =
+      static_cast<RenderFrameHostImpl*>(rvhi->GetMainFrame())->nav_entry_id();
+  NavigationEntryImpl* new_entry =
+      controller_.GetEntryWithUniqueID(nav_entry_id);
+  CHECK(new_entry);

[Avi (use Gerrit), 2015/11/05 00:50:35]

I get that if the page id lookup failed, we wouldn't get this far, so we want
this to exist, but that seems a bit clever.

Perhaps on line 3892 we we remove the null check, gate the crash key value
settings on null, and put the null check after the CHECK_EQ.

(We already do this for the title update, below.)

[Charlie Reis, 2015/11/05 22:12:51]

Done.

[Charlie Reis, 2015/11/05 23:05:37]

Oops, that should have been a null check at the end, rather than a CHECK.  Fixed
in patch set 4.

 
-  DCHECK_EQ(entry, new_entry);
+  base::debug::SetCrashKeyValue("pageid", base::IntToString(page_id));
+  base::debug::SetCrashKeyValue("navuniqueid", base::IntToString(nav_entry_id));
+  base::debug::SetCrashKeyValue(
+      "oldindex", base::IntToString(controller_.GetIndexOfEntry(entry)));
+  base::debug::SetCrashKeyValue(
+      "newindex", base::IntToString(controller_.GetIndexOfEntry(new_entry)));
+  base::debug::SetCrashKeyValue(
+      "lastcommittedindex",
+      base::IntToString(controller_.GetLastCommittedEntryIndex()));
+  base::debug::SetCrashKeyValue("oldurl", entry->GetURL().spec());
+  base::debug::SetCrashKeyValue("newurl", new_entry->GetURL().spec());
+  base::debug::SetCrashKeyValue(
+      "updatedvalue", page_state.GetTopLevelUrlStringTemporaryForBug369661());
+  base::debug::SetCrashKeyValue("oldvalue", entry->GetURL().spec());
+  base::debug::SetCrashKeyValue("newvalue", new_entry->GetURL().spec());
+  CHECK_EQ(entry, new_entry);
 
   if (page_state == entry->GetPageState())
     return;  // Nothing to update.
   entry->SetPageState(page_state);
   controller_.NotifyEntryChanged(entry);
 }
 
 void WebContentsImpl::UpdateTargetURL(RenderViewHost* render_view_host,
                                       const GURL& url) {
   if (fullscreen_widget_routing_id_ != MSG_ROUTING_NONE) {
@@ skipping 182 matching lines @@
                                   base::i18n::TextDirection title_direction) {
   // If we have a title, that's a pretty good indication that we've started
   // getting useful data.
   SetNotWaitingForResponse();
 
   // Try to find the navigation entry, which might not be the current one.
   // For example, it might be from a recently swapped out RFH.
   NavigationEntryImpl* entry = controller_.GetEntryWithPageID(
       render_frame_host->GetSiteInstance(), page_id);
 
-  NavigationEntryImpl* new_entry = controller_.GetEntryWithUniqueID(
-      static_cast<RenderFrameHostImpl*>(render_frame_host)->nav_entry_id());
-  DCHECK_EQ(entry, new_entry);
+  int nav_entry_id =
+      static_cast<RenderFrameHostImpl*>(render_frame_host)->nav_entry_id();
+  NavigationEntryImpl* new_entry =
+      controller_.GetEntryWithUniqueID(nav_entry_id);
+
+  base::debug::SetCrashKeyValue("pageid", base::IntToString(page_id));
+  base::debug::SetCrashKeyValue("navuniqueid", base::IntToString(nav_entry_id));
+  base::debug::SetCrashKeyValue(
+      "oldindex", base::IntToString(controller_.GetIndexOfEntry(entry)));
+  base::debug::SetCrashKeyValue(
+      "newindex", base::IntToString(controller_.GetIndexOfEntry(new_entry)));
+  base::debug::SetCrashKeyValue(
+      "lastcommittedindex",
+      base::IntToString(controller_.GetLastCommittedEntryIndex()));
+  base::debug::SetCrashKeyValue("oldurl",
+                                entry ? entry->GetURL().spec() : "-nullptr-");
+  base::debug::SetCrashKeyValue(
+      "newurl", new_entry ? new_entry->GetURL().spec() : "-nullptr-");
+  base::debug::SetCrashKeyValue("updatedvalue", base::UTF16ToUTF8(title));
+  base::debug::SetCrashKeyValue(
+      "oldvalue", entry ? base::UTF16ToUTF8(entry->GetTitle()) : "-nullptr-");
+  base::debug::SetCrashKeyValue(
+      "newvalue",
+      new_entry ? base::UTF16ToUTF8(new_entry->GetTitle()) : "-nullptr-");
+  CHECK_EQ(entry, new_entry);
 
   // We can handle title updates when we don't have an entry in
   // UpdateTitleForEntry, but only if the update is from the current RVH.
   // TODO(avi): Change to make decisions based on the RenderFrameHost.
   if (!entry && render_frame_host != GetMainFrame())
     return;
 
   // TODO(evan): make use of title_direction.
   // http://code.google.com/p/chromium/issues/detail?id=27094
   if (!UpdateTitleForEntry(entry, title))
@@ skipping 563 matching lines @@
   return NULL;
 }
 
 void WebContentsImpl::SetForceDisableOverscrollContent(bool force_disable) {
   force_disable_overscroll_content_ = force_disable;
   if (view_)
     view_->SetOverscrollControllerEnabled(CanOverscrollContent());
 }
 
 }  // namespace content