Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(410)

Unified Diff: content/browser/frame_host/navigation_controller_impl.cc

Issue 1171973004: Ensure the new navigation classifier works in the real world. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: nits Created 5 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/common/crash_keys.cc ('k') | content/browser/frame_host/render_frame_host_impl.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/browser/frame_host/navigation_controller_impl.cc
diff --git a/content/browser/frame_host/navigation_controller_impl.cc b/content/browser/frame_host/navigation_controller_impl.cc
index 5d4dee8bb4f528239f5aa090c019182e197bf25f..3c25b2e2f813106986a9c6d68b40ddd7ed357bbd 100644
--- a/content/browser/frame_host/navigation_controller_impl.cc
+++ b/content/browser/frame_host/navigation_controller_impl.cc
@@ -6,6 +6,7 @@
#include "base/bind.h"
#include "base/command_line.h"
+#include "base/debug/crash_logging.h"
#include "base/logging.h"
#include "base/metrics/histogram.h"
#include "base/strings/string_number_conversions.h" // Temporary
@@ -792,6 +793,11 @@ bool NavigationControllerImpl::RendererDidNavigate(
// Do navigation-type specific actions. These will make and commit an entry.
details->type = ClassifyNavigation(rfh, params);
NavigationType new_type = ClassifyNavigationWithoutPageID(rfh, params);
+ if (details->type == NAVIGATION_TYPE_NAV_IGNORE &&
+ new_type == NAVIGATION_TYPE_NAV_IGNORE) {
+ base::debug::SetCrashKeyValue("369661-doubleignore",
+ rfh->CommitCountString());
+ }
bool ignore_mismatch = false;
// There are disagreements on some Android bots over SAME_PAGE between the two
// classifiers so ignore disagreements if that's the case.
@@ -816,7 +822,22 @@ bool NavigationControllerImpl::RendererDidNavigate(
ignore_mismatch = true;
}
if (!ignore_mismatch) {
- DCHECK_EQ(details->type, new_type);
+ base::debug::SetCrashKeyValue("369661-oldtype",
+ base::IntToString(details->type));
+ base::debug::SetCrashKeyValue("369661-newtype",
+ base::IntToString(new_type));
+ base::debug::SetCrashKeyValue("369661-navurl", params.url.spec());
+ base::debug::SetCrashKeyValue("369661-naventryid",
+ base::IntToString(params.nav_entry_id));
+ base::debug::SetCrashKeyValue("369661-didcreatenew",
+ params.did_create_new_entry ? "yes" : "no");
+ base::debug::SetCrashKeyValue("369661-pageid",
+ base::IntToString(params.page_id));
+ base::debug::SetCrashKeyValue(
+ "369661-maxpageid",
+ base::IntToString(delegate_->GetMaxPageIDForSiteInstance(
+ rfh->GetSiteInstance())));
+ CHECK_EQ(details->type, new_type);
}
// is_in_page must be computed before the entry gets committed.
@@ -936,6 +957,9 @@ NavigationType NavigationControllerImpl::ClassifyNavigation(
// list.
//
// In these cases, there's nothing we can do with them, so ignore.
+ base::debug::SetCrashKeyValue("369661-oldignore",
+ rfh->CommitCountString() +
+ " no page id");
return NAVIGATION_TYPE_NAV_IGNORE;
}
@@ -952,8 +976,12 @@ NavigationType NavigationControllerImpl::ClassifyNavigation(
// navigated on a popup navigated to about:blank (the iframe would be
// written into the popup by script on the main page). For these cases,
// there isn't any navigation stuff we can do, so just ignore it.
- if (!GetLastCommittedEntry())
+ if (!GetLastCommittedEntry()) {
+ base::debug::SetCrashKeyValue("369661-oldignore",
+ rfh->CommitCountString() +
+ " new subframe no last committed");
return NAVIGATION_TYPE_NAV_IGNORE;
+ }
// Valid subframe navigation.
return NAVIGATION_TYPE_NEW_SUBFRAME;
@@ -1004,6 +1032,9 @@ NavigationType NavigationControllerImpl::ClassifyNavigation(
}
GURL url(temp);
rfh->render_view_host()->Send(new ViewMsg_TempCrashWithData(url));
+ base::debug::SetCrashKeyValue("369661-oldignore",
+ rfh->CommitCountString() +
+ " renderer smoking crack");
return NAVIGATION_TYPE_NAV_IGNORE;
}
NavigationEntryImpl* existing_entry = entries_[existing_entry_index].get();
@@ -1076,8 +1107,12 @@ NavigationType NavigationControllerImpl::ClassifyNavigationWithoutPageID(
// navigated on a popup navigated to about:blank (the iframe would be
// written into the popup by script on the main page). For these cases,
// there isn't any navigation stuff we can do, so just ignore it.
- if (!GetLastCommittedEntry())
+ if (!GetLastCommittedEntry()) {
+ base::debug::SetCrashKeyValue("369661-newignore",
+ rfh->CommitCountString() +
+ " new subframe no last committed");
return NAVIGATION_TYPE_NAV_IGNORE;
+ }
// Valid subframe navigation.
return NAVIGATION_TYPE_NEW_SUBFRAME;
@@ -1094,6 +1129,9 @@ NavigationType NavigationControllerImpl::ClassifyNavigationWithoutPageID(
} else {
// We ignore subframes created in non-committed pages; we'd appreciate if
// people stopped doing that.
+ base::debug::SetCrashKeyValue("369661-newignore",
+ rfh->CommitCountString() +
+ " auto subframe no last committed");
return NAVIGATION_TYPE_NAV_IGNORE;
}
}
@@ -1106,8 +1144,12 @@ NavigationType NavigationControllerImpl::ClassifyNavigationWithoutPageID(
// scribble onto an uncommitted page. Again, there isn't any navigation
// stuff that we can do, so ignore it here as well.
NavigationEntry* last_committed = GetLastCommittedEntry();
- if (!last_committed)
+ if (!last_committed) {
+ base::debug::SetCrashKeyValue("369661-newignore",
+ rfh->CommitCountString() +
+ " renderer-initiated no last committed");
return NAVIGATION_TYPE_NAV_IGNORE;
+ }
if (IsURLInPageNavigation(params.url, params.was_within_same_page, rfh)) {
// This is history.replaceState(), which is renderer-initiated yet within
@@ -1155,6 +1197,9 @@ NavigationType NavigationControllerImpl::ClassifyNavigationWithoutPageID(
// to such entries). It could also mean that the renderer is smoking crack.
// TODO(avi): Crash the renderer like we do in the old ClassifyNavigation?
NOTREACHED() << "Could not find nav entry with id " << params.nav_entry_id;
+ base::debug::SetCrashKeyValue("369661-newignore",
+ rfh->CommitCountString() +
+ " renderer smoking crack");
return NAVIGATION_TYPE_NAV_IGNORE;
}
« no previous file with comments | « chrome/common/crash_keys.cc ('k') | content/browser/frame_host/render_frame_host_impl.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698