Bring RFH/RVH unit tests closer to reality of how RF/RV are initialized

content/test/web_contents_observer_sanity_checker.cc

Index: content/test/web_contents_observer_sanity_checker.cc
diff --git a/content/test/web_contents_observer_sanity_checker.cc b/content/test/web_contents_observer_sanity_checker.cc
index 11e8674cc902626c49c2fb8fe24a2d2f33d60640..96cf1fc1dca486d4ab69251408dff6fa28a11beb 100644
--- a/content/test/web_contents_observer_sanity_checker.cc
+++ b/content/test/web_contents_observer_sanity_checker.cc
@@ -4,6 +4,7 @@
 
 #include "content/test/web_contents_observer_sanity_checker.h"
 
+#include "base/debug/stack_trace.h"
 #include "base/strings/stringprintf.h"
 #include "content/common/frame_messages.h"
 #include "content/public/browser/render_frame_host.h"
@@ -19,6 +20,10 @@ namespace {
 const char kWebContentsObserverSanityCheckerKey[] =
     "WebContentsObserverSanityChecker";
 
+// Set this variable to true to collect stack traces on each RenderFrameCreated
+// and RenderFrameDeleted call, so duplicate calls can be easily debugged.
+static bool g_collect_stack_traces = true;

[ncarter (slow), 2015/05/26 21:44:14]

If you've got to edit the code to turn this off, it's just as easy for the user
to comment out that part of the log message. So I'd argue we probably don't need
this global, unless we need to turn it off programmatically for other reasons
e.g. it doesn't work on all platforms.

I've envisioned that there may be a test in the future that isn't compatible
with some part of this sanity checker, and if that ever happens, we'll provide a
way to suppress the sanity checker as a whole rather than suppressing just one
feature of it like this.

[nasko, 2015/05/26 22:25:49]

As we chatted, I'll land this in a separate CL and make it based on a command
line flag.

+
 }  // namespace
 
 // static
@@ -39,8 +44,20 @@ void WebContentsObserverSanityChecker::RenderFrameCreated(
   deleted_routes_.erase(routing_pair);
 
   if (frame_exists) {
+    std::string trace;
+    if (g_collect_stack_traces) {
+      trace = "\nPrevious creation stack trace:\n" +
+        render_frame_created_stacks_[routing_pair];
+    }
     CHECK(false) << "RenderFrameCreated called more than once for routing pair:"
-                 << Format(render_frame_host);
+                 << Format(render_frame_host) << trace;

[ncarter (slow), 2015/05/26 21:44:14]

This is pretty sweet!

[nasko, 2015/05/26 22:25:49]

It indeed is very helpful in tracking down issues. I will land the full support
(including deletions tracking) in a separate CL.

+  }
+
+  if (g_collect_stack_traces) {
+    base::debug::StackTrace stack;
+    render_frame_created_stacks_.insert(
+        std::make_pair(routing_pair, stack.ToString()));
+    render_frame_deleted_stacks_.erase(routing_pair);
   }
 }