ExtensionNavigationThrottle: Enforce the same rules on redirect as we

chrome/browser/extensions/extension_navigation_throttle_unittest.cc

 // Copyright 2016 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/memory/ptr_util.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "components/crx_file/id_util.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/navigation_handle.h"
@@ skipping 44 matching lines @@
     ChromeRenderViewHostTestHarness::SetUp();
     original_client_ = content::SetBrowserClientForTesting(&client_);
     AddExtension();
   }
 
   void TearDown() override {
     content::SetBrowserClientForTesting(original_client_);
     ChromeRenderViewHostTestHarness::TearDown();
   }
 
-  // Checks that trying to navigate the given |host| to |url| results in the
-  // |expected_result|.
+  // Checks that trying to navigate the given |host| to |extension_url| results
+  // in the |expected_result|.
   void CheckTestCase(content::RenderFrameHost* host,
-                     const GURL& url,
+                     const GURL& extension_url,
                      NavigationThrottle::ThrottleCheckResult expected_result) {
+    // First subtest: direct navigation to |extension_url|.
     std::unique_ptr<content::NavigationHandle> handle =
-        content::NavigationHandle::CreateNavigationHandleForTesting(url, host);
+        content::NavigationHandle::CreateNavigationHandleForTesting(
+            extension_url, host);
     EXPECT_EQ(expected_result,
               handle->CallWillStartRequestForTesting(
                   /*is_post=*/false, content::Referrer(),
                   /*has_user_gesture=*/false, ui::PAGE_TRANSITION_LINK,
                   /*is_external_protocol=*/false))
-        << url;
+        << extension_url;
+
+    // Reset the handle for a second subtest: server redirect to
+    // |extension_url|. Currently, the same rules apply, but we cancel rather
+    // than block such requests.
+    GURL http_url("https://example.com");
+    handle = content::NavigationHandle::CreateNavigationHandleForTesting(
+        http_url, host);
+    NavigationThrottle::ThrottleCheckResult expected_redirect_result =

[Devlin, 2017/05/18 15:22:44]

This is a bit strange, because now |expected_result| isn't always the actual
expected result.  I would suggest either:
a) passing in a bool "expect_succeed", and then constructing a
direct_navigation_expected_result = success ? PROCEED : BLOCK_REQUEST and a
redirect_navigation_expected_result = success ? PROCEED : CANCEL, or
b) passing in both a direct and redirect expected result.

Since it looks like the expected result is currently always either proceed or
block, a) would probably be sufficient and is a little less verbose.

[ncarter (slow), 2017/05/22 22:55:59]

Really, it's just a limitation of the throttle system in non-plznavigate that we
need to round the BLOCKs down to CANCELs currently.

My hope is that in the future, we'll be able to use the same errors for
WillRedirect as we do for WillStartRequest.

I've added some TODO comments w/ bug links to clarify what's going on.

+        (expected_result == NavigationThrottle::PROCEED)
+            ? NavigationThrottle::PROCEED
+            : NavigationThrottle::CANCEL;
+    EXPECT_EQ(NavigationThrottle::PROCEED,
+              handle->CallWillStartRequestForTesting(
+                  /*is_post=*/false, content::Referrer(),
+                  /*has_user_gesture=*/false, ui::PAGE_TRANSITION_LINK,
+                  /*is_external_protocol=*/false))
+        << http_url;
+    EXPECT_EQ(expected_redirect_result,
+              handle->CallWillRedirectRequestForTesting(
+                  extension_url,
+                  /*new_method_is_post=*/false, http_url,
+                  /*new_is_external_protocol=*/false))
+        << extension_url;
   }
 
   const Extension* extension() { return extension_.get(); }
   content::WebContentsTester* web_contents_tester() {
     return content::WebContentsTester::For(web_contents());
   }
   content::RenderFrameHostTester* render_frame_host_tester(
       content::RenderFrameHost* host) {
     return content::RenderFrameHostTester::For(host);
   }
@@ skipping 157 matching lines @@
   GURL unknown_filesystem(
       base::StringPrintf("filesystem:chrome-extension://%s/temporary/foo.html",
                          second_id.c_str()));
   CheckTestCase(main_rfh(), disabled_filesystem,
                 NavigationThrottle::BLOCK_REQUEST);
   CheckTestCase(main_rfh(), unknown_filesystem,
                 NavigationThrottle::BLOCK_REQUEST);
 }
 
 }  // namespace extensions