Fix Material Settings Help and Settings redirect

chrome/browser/browser_about_handler_unittest.cc

Index: chrome/browser/browser_about_handler_unittest.cc
diff --git a/chrome/browser/browser_about_handler_unittest.cc b/chrome/browser/browser_about_handler_unittest.cc
index 5e51f0e4731dde41236118629cc5e6ad9938a2a5..49129c6c55fd7bd6fd7fd6998ed6a6b90c371c4a 100644
--- a/chrome/browser/browser_about_handler_unittest.cc
+++ b/chrome/browser/browser_about_handler_unittest.cc
@@ -7,9 +7,12 @@
 #include <stddef.h>
 
 #include <memory>

[Dan Beam, 2016/06/21 05:24:45]

nit: #include <utility>

for std::move()

[michaelpg, 2016/06/21 19:19:33]

Done.

+#include <vector>
 
+#include "base/feature_list.h"
 #include "base/macros.h"
 #include "base/message_loop/message_loop.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/browser/navigation_controller.h"
@@ -26,16 +29,31 @@ using content::NavigationEntry;
 using content::Referrer;
 
 class BrowserAboutHandlerTest : public testing::Test {
+ protected:
+  struct AboutURLTestData {

[Dan Beam, 2016/06/21 05:24:45]

nit: why does this need to be inside BrowserAboutHandlerTest

[michaelpg, 2016/06/21 19:19:33]

ummm dunno, is an anonymous namespace better?

[Dan Beam, 2016/06/21 19:22:50]

i would say so because shorter names, yes

+    GURL test_url;
+    GURL result_url;

[Lei Zhang, 2016/06/21 07:54:00]

Maybe expected_result_url?

[michaelpg, 2016/06/21 19:19:33]

Done, also renamed TestData => TestCase[s]

+  };
+
+  void TestWillHandleBrowserAboutURL(
+      const std::vector<AboutURLTestData>& test_data) {
+    TestingProfile profile;
+
+    for (const auto& test_datum : test_data) {
+      GURL url(test_datum.test_url);
+      WillHandleBrowserAboutURL(&url, &profile);
+      EXPECT_EQ(test_datum.result_url, url);
+    }
+  }
+
+ private:
   content::TestBrowserThreadBundle thread_bundle_;
 };
 
 TEST_F(BrowserAboutHandlerTest, WillHandleBrowserAboutURL) {
   std::string chrome_prefix(content::kChromeUIScheme);
   chrome_prefix.append(url::kStandardSchemeSeparator);
-  struct AboutURLTestData {
-    GURL test_url;
-    GURL result_url;
-  } test_data[] = {
+  std::vector<BrowserAboutHandlerTest::AboutURLTestData> test_data({
       {
         GURL("http://google.com"),
         GURL("http://google.com")
@@ -72,14 +90,60 @@ TEST_F(BrowserAboutHandlerTest, WillHandleBrowserAboutURL) {
         GURL(chrome_prefix + "host/path?query#ref"),
         GURL(chrome_prefix + "host/path?query#ref"),
       }
-  };
-  TestingProfile profile;
+  });
+  TestWillHandleBrowserAboutURL(test_data);
+}
 
-  for (size_t i = 0; i < arraysize(test_data); ++i) {
-    GURL url(test_data[i].test_url);
-    WillHandleBrowserAboutURL(&url, &profile);
-    EXPECT_EQ(test_data[i].result_url, url);
-  }
+TEST_F(BrowserAboutHandlerTest, WillHandleBrowserAboutURLForOptions) {
+  std::string chrome_prefix(content::kChromeUIScheme);
+  chrome_prefix.append(url::kStandardSchemeSeparator);
+  GURL settings_url_result;
+  GURL help_url_result;
+#if defined(OS_CHROMEOS)
+  settings_url_result =

[Dan Beam, 2016/06/21 05:24:45]

this test might be slightly easier to read if the whole thing was keyed of the
OS

[michaelpg, 2016/06/21 19:19:33]

changed to 2 separate tests, does seem more readable

+      GURL(chrome_prefix + chrome::kChromeUISettingsFrameHost);
+  help_url_result = GURL(chrome_prefix + chrome::kChromeUISettingsFrameHost +
+                         "/" + chrome::kChromeUIHelpHost);
+#else
+  settings_url_result = GURL(chrome_prefix + chrome::kChromeUIUberHost + "/" +
+                             chrome::kChromeUISettingsHost + "/");
+  help_url_result = GURL(chrome_prefix + chrome::kChromeUIUberHost + "/" +
+                         chrome::kChromeUIHelpHost + "/");
+#endif
+
+  std::vector<BrowserAboutHandlerTest::AboutURLTestData> test_data({
+      {
+        GURL(chrome_prefix + chrome::kChromeUISettingsHost),
+        settings_url_result
+      },
+      {
+        GURL(chrome_prefix + chrome::kChromeUIHelpHost),
+        help_url_result
+      }
+  });
+  TestWillHandleBrowserAboutURL(test_data);
+}
+
+TEST_F(BrowserAboutHandlerTest, WillHandleBrowserAboutURLForMDSettings) {
+  std::unique_ptr<base::FeatureList> feature_list(new base::FeatureList);
+  feature_list->InitializeFromCommandLine(
+      features::kMaterialDesignSettingsFeature.name, "");
+  base::FeatureList::ClearInstanceForTesting();
+  base::FeatureList::SetInstance(std::move(feature_list));
+
+  std::string chrome_prefix(content::kChromeUIScheme);
+  chrome_prefix.append(url::kStandardSchemeSeparator);
+  std::vector<BrowserAboutHandlerTest::AboutURLTestData> test_data({
+      {
+        GURL(chrome_prefix + chrome::kChromeUISettingsHost),
+        GURL(chrome_prefix + chrome::kChromeUISettingsHost)
+      },
+      {
+        GURL(chrome_prefix + chrome::kChromeUIHelpHost),
+        GURL(chrome_prefix + chrome::kChromeUIHelpHost)
+      }
+  });
+  TestWillHandleBrowserAboutURL(test_data);
 }
 
 // Ensure that minor BrowserAboutHandler fixup to a URL does not cause us to