Fix Material Settings Help and Settings redirect

chrome/browser/browser_about_handler_unittest.cc

 // Copyright (c) 2012 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 "chrome/browser/browser_about_handler.h"
 
 #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"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/common/referrer.h"
 #include "content/public/test/test_browser_thread.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
 using content::BrowserThread;
 using content::NavigationController;
 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")
       },
       {
         GURL(url::kAboutBlankURL),
         GURL(url::kAboutBlankURL)
       },
       {
         GURL(chrome_prefix + chrome::kChromeUIDefaultHost),
@@ skipping 16 matching lines @@
         GURL(chrome_prefix + chrome::kChromeUISyncInternalsHost)
       },
       {
         GURL(chrome_prefix + chrome::kChromeUIInvalidationsHost),
         GURL(chrome_prefix + chrome::kChromeUIInvalidationsHost)
       },
       {
         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
 // keep a separate virtual URL, which would not be updated on redirects.
 // See https://crbug.com/449829.
 TEST_F(BrowserAboutHandlerTest, NoVirtualURLForFixup) {
   GURL url("view-source:http://.foo");
 
   // Fixup will remove the dot and add a slash.
   GURL fixed_url("view-source:http://foo/");
 
   // Rewriters will remove the view-source prefix and expect it to stay in the
   // virtual URL.
   GURL rewritten_url("http://foo/");
 
   TestingProfile profile;
   std::unique_ptr<NavigationEntry> entry(
       NavigationController::CreateNavigationEntry(
           url, Referrer(), ui::PAGE_TRANSITION_RELOAD, false, std::string(),
           &profile));
   EXPECT_EQ(fixed_url, entry->GetVirtualURL());
   EXPECT_EQ(rewritten_url, entry->GetURL());
 }