RB-B P1 issue: Add debug aliases for state_ and urls to debug CHECK failures

chrome/browser/loader/safe_browsing_resource_throttle.cc

Index: chrome/browser/loader/safe_browsing_resource_throttle.cc
diff --git a/chrome/browser/loader/safe_browsing_resource_throttle.cc b/chrome/browser/loader/safe_browsing_resource_throttle.cc
index 23a27088ef4fe9f2a774c7f7516457b8d5fddca2..396226f09ea60c8ef70cf82ea2c5ef8ebba828e2 100644
--- a/chrome/browser/loader/safe_browsing_resource_throttle.cc
+++ b/chrome/browser/loader/safe_browsing_resource_throttle.cc
@@ -7,6 +7,7 @@
 #include <iterator>
 #include <utility>
 
+#include "base/debug/alias.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/trace_event/trace_event.h"
@@ -232,10 +233,27 @@ void SafeBrowsingResourceThrottle::OnCheckBrowseUrlResult(
     const GURL& url,
     safe_browsing::SBThreatType threat_type,
     const safe_browsing::ThreatMetadata& metadata) {
-  CHECK_EQ(state_, STATE_CHECKING_URL);
-  CHECK(url.is_valid());
-  CHECK(url_being_checked_.is_valid());
-  CHECK_EQ(url, url_being_checked_);
+  // TODO(vakh): The following base::debug::Alias() calls should be removed and
+  // CHECKs corrected after http://crbug.com/660293 is fixed.
+  base::debug::Alias(&state_);
+  base::debug::Alias(&url);
+  base::debug::Alias(&url_being_checked_);

[Scott Hess - ex-Googler, 2016/12/02 04:19:27]

I am not sure that these work.  They'll annotate to pin the main object, but not
the spec_.

Actually, looking at the Alias() implementation ... I have no idea why it works
to do what I have used it for in the past.  Like sql/connection.cc in
ReportDiagnosticInfo().  In that function, I put the thing I want to see in the
crash dump into a stack variable, then Alias() that, then potentially dump
(well, in that case I force a dump, but I've used it successfully in
potentially-dump cases before).  It may be that the stack placement is
important.  I don't know what parts of the heap come up in crash dumps.

+  std::string alias_str = base::StringPrintf(
+      "OnCheckBrowseUrlResult: state_: %d, url: %s, url_being_checked_: %s",
+      state_, url.spec().c_str(), url_being_checked_.spec().c_str());

[Scott Hess - ex-Googler, 2016/12/02 04:19:27]

In fact, I'm not even sure this will work, because I'm not sure you'll get the
string's heap part.  If you want to be safe, I'd snprintf() into a stack buffer.
 I'm not sure that's correct, but I'm sure it will work, and for something like
this you'll probably rather have results than correctness :-).

+  base::debug::Alias(&alias_str);
+  if (!url_being_checked_.is_valid()) {
+    CHECK(false);
+  }
+  if (!url.is_valid()) {
+    CHECK(false);
+  }
+  if (url != url_being_checked_) {
+    CHECK(false);
+  }
+  if (state_ != STATE_CHECKING_URL) {
+    CHECK(false);
+  }
 
   timer_.Stop();  // Cancel the timeout timer.
   threat_type_ = threat_type;