Don't leave aborted URLs in the omnibox unless we're on a new tab.

content/browser/frame_host/navigation_controller_impl_unittest.cc

 // Copyright 2013 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 "base/basictypes.h"
 #include "base/bind.h"
 #include "base/file_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/path_service.h"
 #include "base/stl_util.h"
@@ skipping 1028 matching lines @@
   params.showing_repost_interstitial = false;
   main_test_rfh()->OnMessageReceived(
       FrameHostMsg_DidFailProvisionalLoadWithError(0,  // routing_id
                                                    params));
 
   // Because the pending entry is renderer initiated and not visible, we
   // clear it when it fails.
   EXPECT_EQ(-1, controller.GetPendingEntryIndex());
   EXPECT_FALSE(controller.GetPendingEntry());
   EXPECT_EQ(0, controller.GetLastCommittedEntryIndex());
-  EXPECT_EQ(0, delegate->navigation_state_change_count());
+  EXPECT_EQ(1, delegate->navigation_state_change_count());

[Charlie Reis, 2014/04/18 01:23:53]

We're now triggering a state change in DidFail so that the omnibox updates when
we clear the entry.

 
   // The visible entry should be the last committed URL, not the pending one,
   // so that no spoof is possible.
   EXPECT_EQ(kExistingURL, controller.GetVisibleEntry()->GetURL());
 
   contents()->SetDelegate(NULL);
 }
 
 // Ensure that NavigationEntries track which bindings their RenderViewHost had
 // at the time they committed.  http://crbug.com/173672.
@@ skipping 1813 matching lines @@
   // we must revert to showing about:blank to avoid a URL spoof.
   test_rvh()->OnMessageReceived(
         ViewHostMsg_DidAccessInitialDocument(0));
   EXPECT_TRUE(test_rvh()->has_accessed_initial_document());
   EXPECT_FALSE(controller.GetVisibleEntry());
   EXPECT_EQ(url, controller.GetPendingEntry()->GetURL());
 
   notifications.Reset();
 }
 
+// Tests that the URLs for renderer-initiated navigations in new tabs are
+// displayed to the user even after they fail, as long as the initial
+// about:blank page has not been modified.  If so, we must revert to showing
+// about:blank. See http://crbug.com/355537.
+TEST_F(NavigationControllerTest, ShowRendererURLAfterFailUntilModified) {

[Charlie Reis, 2014/04/18 01:23:53]

This is similar to the test above but checks the DidFail case, making sure the
URL stays visible until the underlying page is modified.

+  NavigationControllerImpl& controller = controller_impl();
+  TestNotificationTracker notifications;
+  RegisterForAllNavNotifications(&notifications, &controller);
+
+  const GURL url("http://foo");
+
+  // For renderer-initiated navigations in new tabs (with no committed entries),
+  // we show the pending entry's URL as long as the about:blank page is not
+  // modified.
+  NavigationController::LoadURLParams load_url_params(url);
+  load_url_params.transition_type = PAGE_TRANSITION_LINK;
+  load_url_params.is_renderer_initiated = true;
+  controller.LoadURLWithParams(load_url_params);
+  EXPECT_EQ(url, controller.GetVisibleEntry()->GetURL());
+  EXPECT_EQ(url, controller.GetPendingEntry()->GetURL());
+  EXPECT_TRUE(
+      NavigationEntryImpl::FromNavigationEntry(controller.GetPendingEntry())->
+          is_renderer_initiated());
+  EXPECT_TRUE(controller.IsInitialNavigation());
+  EXPECT_FALSE(test_rvh()->has_accessed_initial_document());
+
+  // There should be no title yet.
+  EXPECT_TRUE(contents()->GetTitle().empty());
+
+  // Suppose it aborts before committing, if it's a 204 or download or due to a
+  // stop or a new navigation from the user.  The URL should remain visible.
+  FrameHostMsg_DidFailProvisionalLoadWithError_Params params;
+  params.error_code = net::ERR_ABORTED;
+  params.error_description = base::string16();
+  params.url = url;
+  params.showing_repost_interstitial = false;
+  main_test_rfh()->OnMessageReceived(
+      FrameHostMsg_DidFailProvisionalLoadWithError(0,  // routing_id
+                                                   params));
+  EXPECT_EQ(url, controller.GetVisibleEntry()->GetURL());
+
+  // If something else later modifies the contents of the about:blank page, then
+  // we must revert to showing about:blank to avoid a URL spoof.
+  test_rvh()->OnMessageReceived(
+        ViewHostMsg_DidAccessInitialDocument(0));
+  EXPECT_TRUE(test_rvh()->has_accessed_initial_document());
+  EXPECT_FALSE(controller.GetVisibleEntry());
+  EXPECT_EQ(url, controller.GetPendingEntry()->GetURL());
+
+  notifications.Reset();
+}
+
 TEST_F(NavigationControllerTest, DontShowRendererURLInNewTabAfterCommit) {
   NavigationControllerImpl& controller = controller_impl();
   TestNotificationTracker notifications;
   RegisterForAllNavNotifications(&notifications, &controller);
 
   const GURL url1("http://foo/eh");
   const GURL url2("http://foo/bee");
 
   // For renderer-initiated navigations in new tabs (with no committed entries),
   // we show the pending entry's URL as long as the about:blank page is not
@@ skipping 1146 matching lines @@
   EXPECT_EQ(1, controller.GetEntryCount());
   EXPECT_EQ(0, controller.GetCurrentEntryIndex());
   EXPECT_EQ(0, controller.GetLastCommittedEntryIndex());
   EXPECT_EQ(-1, controller.GetPendingEntryIndex());
   EXPECT_FALSE(controller.CanGoBack());
   EXPECT_FALSE(controller.CanGoForward());
   EXPECT_EQ(url4, controller.GetVisibleEntry()->GetURL());
 }
 
 }  // namespace content