Do not discount a MANUAL_SUBFRAME load just because it involved

some redirects.

R=brettw
BUG=21353
TEST=none

Review URL: http://codereview.chromium.org/246073

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@27887 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 45 insertions, 24 deletions

diff --git a/chrome/browser/session_history_uitest.cc b/chrome/browser/session_history_uitest.cc
index 8d76583..473dc8b 100644
--- a/chrome/browser/session_history_uitest.cc
+++ b/chrome/browser/session_history_uitest.cc
@@ -23,10 +23,6 @@ const wchar_t kDocRoot[] = L"chrome/test/data";
 class SessionHistoryTest : public UITest {
  protected:
   SessionHistoryTest() : UITest() {
-    FilePath path(test_data_directory_);
-    path = path.AppendASCII("session_history");
-
-    url_prefix_ = UTF8ToWide(net::FilePathToFileURL(path).spec());
   }
 
   virtual void SetUp() {
@@ -98,7 +94,6 @@ class SessionHistoryTest : public UITest {
   }
 
  protected:
-  wstring url_prefix_;
   scoped_refptr<BrowserProxy> window_;
   scoped_refptr<TabProxy> tab_;
 };
@@ -518,4 +513,21 @@ TEST_F(SessionHistoryTest, HistorySearchXSS) {
   EXPECT_EQ(L"History", GetTabTitle());
 }
 
+TEST_F(SessionHistoryTest, LocationChangeInSubframe) {
+  scoped_refptr<HTTPTestServer> server =
+      HTTPTestServer::CreateServer(kDocRoot, NULL);
+  ASSERT_TRUE(server.get());
+
+  ASSERT_TRUE(tab_->NavigateToURL(server->TestServerPage(
+      "files/session_history/location_redirect.html")));
+  EXPECT_EQ(L"Default Title", GetTabTitle());
+
+  ASSERT_TRUE(tab_->NavigateToURL(GURL(
+      "javascript:void(frames[0].navigate())")));
+  EXPECT_EQ(L"foo", GetTabTitle());
+
+  ASSERT_TRUE(tab_->GoBack());
+  EXPECT_EQ(L"Default Title", GetTabTitle());
+}
+
 }  // namespace
diff --git a/chrome/browser/tab_contents/navigation_controller.cc b/chrome/browser/tab_contents/navigation_controller.cc
index fddf3de..a1fa599 100644
--- a/chrome/browser/tab_contents/navigation_controller.cc
+++ b/chrome/browser/tab_contents/navigation_controller.cc
@@ -711,10 +711,6 @@ void NavigationController::RendererDidNavigateNewSubframe(
     // This is not user-initiated. Ignore.
     return;
   }
-  if (IsRedirect(params)) {
-    // This is redirect. Ignore.
-    return;
-  }
 
   // Manual subframe navigations just get the current entry cloned so the user
   // can go back or forward to it. The actual subframe information will be
diff --git a/chrome/test/data/session_history/location_redirect.html b/chrome/test/data/session_history/location_redirect.html
new file mode 100644
index 0000000..07f0f2a
--- /dev/null
+++ b/chrome/test/data/session_history/location_redirect.html
@@ -0,0 +1,5 @@
+<title>Default Title</title>
+<style>
+iframe { width: 98%; height: 96%; }
+</style>
+<iframe src="location_redirect_frame1.html"></iframe>
diff --git a/chrome/test/data/session_history/location_redirect_frame1.html b/chrome/test/data/session_history/location_redirect_frame1.html
new file mode 100644
index 0000000..3b9c05e
--- /dev/null
+++ b/chrome/test/data/session_history/location_redirect_frame1.html
@@ -0,0 +1,5 @@
+<script>
+function navigate() {
+  location = "location_redirect_frame2.html";
+}
+</script>
diff --git a/chrome/test/data/session_history/location_redirect_frame2.html b/chrome/test/data/session_history/location_redirect_frame2.html
new file mode 100644
index 0000000..21e040e
--- /dev/null
+++ b/chrome/test/data/session_history/location_redirect_frame2.html
@@ -0,0 +1,3 @@
+<script>
+parent.document.title = "foo";
+</script>
diff --git a/webkit/glue/webframeloaderclient_impl.cc b/webkit/glue/webframeloaderclient_impl.cc
index 9ceda5b..ea6b62e 100644
--- a/webkit/glue/webframeloaderclient_impl.cc
+++ b/webkit/glue/webframeloaderclient_impl.cc
@@ -476,22 +476,22 @@ void WebFrameLoaderClient::dispatchWillPerformClientRedirect(
     double fire_date) {
   // Tells dispatchDidStartProvisionalLoad that if it sees this item it is a
   // redirect and the source item should be added as the start of the chain.
-  if (webframe_->client()) {
-    expected_client_redirect_src_ = webframe_->url();
-    expected_client_redirect_dest_ = webkit_glue::KURLToGURL(url);
-
-    // TODO(timsteele): bug 1135512. Webkit does not properly notify us of
-    // cancelling http > file client redirects. Since the FrameLoader's policy
-    // is to never carry out such a navigation anyway, the best thing we can do
-    // for now to not get confused is ignore this notification.
-    if (expected_client_redirect_dest_.SchemeIsFile() &&
-       (expected_client_redirect_src_.SchemeIs("http") ||
-        expected_client_redirect_src_.SchemeIsSecure())) {
-      expected_client_redirect_src_ = GURL();
-      expected_client_redirect_dest_ = GURL();
-      return;
-    }
+  expected_client_redirect_src_ = webframe_->url();
+  expected_client_redirect_dest_ = webkit_glue::KURLToGURL(url);
+
+  // TODO(timsteele): bug 1135512. Webkit does not properly notify us of
+  // cancelling http > file client redirects. Since the FrameLoader's policy
+  // is to never carry out such a navigation anyway, the best thing we can do
+  // for now to not get confused is ignore this notification.
+  if (expected_client_redirect_dest_.SchemeIsFile() &&
+     (expected_client_redirect_src_.SchemeIs("http") ||
+      expected_client_redirect_src_.SchemeIsSecure())) {
+    expected_client_redirect_src_ = GURL();
+    expected_client_redirect_dest_ = GURL();
+    return;
+  }
 
+  if (webframe_->client()) {
     webframe_->client()->willPerformClientRedirect(
         webframe_,
         expected_client_redirect_src_,