Fixed flakiness of context_lost tests on GPU bots.

content/browser/frame_host/navigation_controller_impl.cc

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 79e2c7b478ca89c52a53ec2bdc079c29fddcdae3..42ea8e5cf5e8bd4ed8a06a9c1854eed30dfc842a 100644
--- a/content/browser/frame_host/navigation_controller_impl.cc
+++ b/content/browser/frame_host/navigation_controller_impl.cc
@@ -5,12 +5,14 @@
 #include "content/browser/frame_host/navigation_controller_impl.h"
 
 #include "base/bind.h"
+#include "base/command_line.h"
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"  // Temporary
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
+#include "cc/base/switches.h"
 #include "content/browser/browser_url_handler_impl.h"
 #include "content/browser/dom_storage/dom_storage_context_wrapper.h"
 #include "content/browser/dom_storage/session_storage_namespace_impl.h"
@@ -1012,8 +1014,19 @@ void NavigationControllerImpl::RendererDidNavigateToNewPage(
     // update the virtual URL when replaceState is called after a pushState.
     GURL url = params.url;
     bool needs_update = false;
-    BrowserURLHandlerImpl::GetInstance()->RewriteURLIfNecessary(
-        &url, browser_context_, &needs_update);
+    // Short-circuit the URL rewriting -- and consequent duplicate
+    // handling of debug URL handlers -- in the case where this is a
+    // debug URL and it would be handled in the BrowserURLHandlerImpl.
+    // URLs are rewritten twice, once when page navigation begins and
+    // once when the renderer completes the page navigation, and these
+    // debug URLs must be handled exactly once.

[Charlie Reis, 2014/05/09 23:21:51]

Sorry, it took me a while to understand what this comment meant, and thus why
this was correct.  Can we rewrite it to be a little more concrete?  Something
like:

"We call RewriteURLIfNecessary twice: once when page navigation begins in
CreateNavigationEntry, and once here when it commits.  For debug URLs, rewriting
the URL causes an action to occur and thus should not be done a second time."

[Ken Russell (switch to Gerrit), 2014/05/09 23:43:05]

Yes, thanks. Done.

+    bool skip_rewrite =
+        IsDebugURL(url) && base::CommandLine::ForCurrentProcess()->HasSwitch(
+            cc::switches::kEnableGpuBenchmarking);

[jam, 2014/05/09 22:47:46]

i can't tell what kEnableGpuBenchmarking has to do with this code?

[Charlie Reis, 2014/05/09 23:21:51]

+1.  I think we want to skip the second rewriting call for all debug URLs,
right?

[Ken Russell (switch to Gerrit), 2014/05/09 23:43:05]

Most of the time, the browser's debug URL handler will ignore debug URLs. I only
added support for processing them when kEnableGpuBenchmarking is enabled, since
most of the time they are navigated to by the omnibox. See Issue 308675 and
https://codereview.chromium.org/27603004 , and DebugURLHandler in
browser_url_handler_impl.cc.

I was aiming for the minimum behavioral change with this CL. I'd be glad to
expand the effects if you like, but I think it's worth being conservative.

[Charlie Reis, 2014/05/09 23:53:40]

Oh, I misunderstood what was happening.  Yes, let's keep the flag check here,
since it only matters in that case.  (It's admittedly a little awkward to have
this logic here, but maybe we can revisit how you're handling these URLs later.)

Accordingly, let's change the comment above to:

"We call RewriteURLIfNecessary twice: once when page navigation begins in
CreateNavigationEntry, and once here when it commits.  With the
kEnableGpuBenchmarking flag, the rewriting step includes handling debug URLs
which cause an action to occur, and thus we should not rewrite them a second
time."

[Ken Russell (switch to Gerrit), 2014/05/09 23:56:40]

Done.

+    if (!skip_rewrite) {
+      BrowserURLHandlerImpl::GetInstance()->RewriteURLIfNecessary(
+          &url, browser_context_, &needs_update);
+    }
     new_entry->set_update_virtual_url_with_url(needs_update);
 
     // When navigating to a new page, give the browser URL handler a chance to