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

content/test/web_contents_observer_sanity_checker.cc

 // Copyright 2014 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/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"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/site_instance.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
 
 namespace content {
 
 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
 void WebContentsObserverSanityChecker::Enable(WebContents* web_contents) {
   if (web_contents->GetUserData(&kWebContentsObserverSanityCheckerKey))
     return;
   web_contents->SetUserData(&kWebContentsObserverSanityCheckerKey,
                             new WebContentsObserverSanityChecker(web_contents));
 }
 
 void WebContentsObserverSanityChecker::RenderFrameCreated(
     RenderFrameHost* render_frame_host) {
   CHECK(!web_contents_destroyed_);
   std::pair<int, int> routing_pair =
       std::make_pair(render_frame_host->GetProcess()->GetID(),
                      render_frame_host->GetRoutingID());
   bool frame_exists = !live_routes_.insert(routing_pair).second;
   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);
   }
 }
 
 void WebContentsObserverSanityChecker::RenderFrameDeleted(
     RenderFrameHost* render_frame_host) {
   CHECK(!web_contents_destroyed_);
   std::pair<int, int> routing_pair =
       std::make_pair(render_frame_host->GetProcess()->GetID(),
                      render_frame_host->GetRoutingID());
   bool was_live = !!live_routes_.erase(routing_pair);
@@ skipping 194 matching lines @@
 
 std::string WebContentsObserverSanityChecker::Format(
     RenderFrameHost* render_frame_host) {
   return base::StringPrintf(
       "(%d, %d -> %s)", render_frame_host->GetProcess()->GetID(),
       render_frame_host->GetRoutingID(),
       render_frame_host->GetSiteInstance()->GetSiteURL().spec().c_str());
 }
 
 }  // namespace content