Separate Settings and Help from History and Extensions when settings-in-a-window is enabled.

With --enable-settings-window, only Settings and Help should be shown in the left nav bar when opening settings or help. Likewise, history and extensions should be the only two nav items when loading those pages.

This is a temporary change to unblock Settings in a Window. Going forward, we will remove the left nav entirely from the Settings window, so the logic for having nav item groups here is only temporary. See issue 294023013 for how this will be done after the branch.

BUG=375425
R=dbeam@chromium.org,sky@chromium.org
TBR=estade@chromium.org
NOTRY=true
# win_chromium_x64_rel flaky failures

Review URL: https://codereview.chromium.org/313363004

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

chrome/browser/resources/uber/uber_frame.css

@@ -41,7 +41,8 @@ li.selected {
   pointer-events: none;
 }
 
-#helpNavItem {
+/* Separates the Help nav item if there are at least 3 items. */
+li:not([hidden]) ~ li:not([hidden]) ~ #helpNavItem {
   margin-top: 27px;
 }
 

chrome/browser/resources/uber/uber_frame.html

@@ -16,19 +16,20 @@
 
 <h1 i18n-content="shortProductName"></h1>
 <ul>
-  <li i18n-values="controls:historyHost;override:overridesHistory">
+  <li i18n-values="controls:historyHost;override:overridesHistory;
+                   group:historyGroup" hidden>
     <button class="custom-appearance"
         i18n-content="historyDisplayName"></button>
   </li>
-  <li i18n-values="controls:extensionsHost">
+  <li i18n-values="controls:extensionsHost;group:extensionsGroup" hidden>
     <button class="custom-appearance"
         i18n-content="extensionsDisplayName"></button>
   </li>
-  <li i18n-values="controls:settingsHost">
+  <li i18n-values="controls:settingsHost;group:settingsGroup" hidden>
     <button class="custom-appearance"
         i18n-content="settingsDisplayName"></button>
   </li>
-  <li id="helpNavItem" i18n-values="controls:helpHost">
+  <li id="helpNavItem" i18n-values="controls:helpHost;group:helpGroup" hidden>
     <button class="custom-appearance"
         i18n-content="helpDisplayName"></button>
   </li>

chrome/browser/resources/uber/uber_frame.js

@@ -77,14 +77,22 @@ cr.define('uber_frame', function() {
     var navItem =
         document.querySelector('li[controls="' + params.pageId + '"]');
     setSelection(navItem);
+    showNavItems();
+  }
+
+  /**
+   * @return {Element} The currently selected nav item, if any.
+   */
+  function getSelectedNavItem() {
+    return document.querySelector('li.selected');
   }
 
   /**
    * Sets selection on the given nav item.
-   * @param {boolean} newSelection The item to be selected.
+   * @param {Element} newSelection The item to be selected.
    */
   function setSelection(newSelection) {
-    var lastSelectedNavItem = document.querySelector('li.selected');
+    var lastSelectedNavItem = getSelectedNavItem();
     if (lastSelectedNavItem !== newSelection) {
       newSelection.classList.add('selected');
       if (lastSelectedNavItem)
@@ -92,6 +100,20 @@ cr.define('uber_frame', function() {
     }
   }
 
+  /**
+   * Shows nav items belonging to the same group as the selected item.
+   */
+  function showNavItems() {
+    var navItems = document.querySelectorAll('li');
+    var selectedNavItem = getSelectedNavItem();
+    assert(selectedNavItem);
+
+    var selectedGroup = selectedNavItem.getAttribute('group');
+    for (var i = 0; i < navItems.length; ++i) {
+      navItems[i].hidden = navItems[i].getAttribute('group') != selectedGroup;
+    }
+  }
+
   /**
    * Adjusts this frame's content to scrolls from the outer frame. This is done
    * to obscure text in RTL as a user scrolls over the content of this frame (as
@@ -124,7 +146,7 @@ cr.define('uber_frame', function() {
   }
 
   /**
-   * @return {Object} The currently selected iframe container.
+   * @return {Element} The currently selected iframe container.
    * @private
    */
   function getSelectedIframe() {

chrome/browser/ui/chrome_pages.cc

@@ -108,7 +108,7 @@ void ShowHelpImpl(Browser* browser,
   ShowSingletonTab(browser, url);
 }
 
-bool SettingsWindowEnabled() {
+bool IsSettingsWindowEnabled() {
   return CommandLine::ForCurrentProcess()->HasSwitch(
       ::switches::kEnableSettingsWindow);
 }
@@ -129,11 +129,6 @@ void ShowBookmarkManagerForNode(Browser* browser, int64 node_id) {
 
 void ShowHistory(Browser* browser) {
   content::RecordAction(UserMetricsAction("ShowHistory"));
-  if (SettingsWindowEnabled()) {
-    SettingsWindowManager::GetInstance()->ShowChromePageForProfile(
-        browser->profile(), GURL(kChromeUIHistoryURL));
-    return;
-  }
   NavigateParams params(
       GetSingletonTabNavigateParams(browser, GURL(kChromeUIHistoryURL)));
   params.path_behavior = NavigateParams::IGNORE_AND_NAVIGATE;
@@ -156,11 +151,6 @@ void ShowDownloads(Browser* browser) {
 void ShowExtensions(Browser* browser,
                     const std::string& extension_to_highlight) {
   content::RecordAction(UserMetricsAction("ShowExtensions"));
-  if (SettingsWindowEnabled()) {
-    SettingsWindowManager::GetInstance()->ShowChromePageForProfile(
-        browser->profile(), GURL(kChromeUIExtensionsURL));
-    return;
-  }
   NavigateParams params(
       GetSingletonTabNavigateParams(browser, GURL(kChromeUIExtensionsURL)));
   params.path_behavior = NavigateParams::IGNORE_AND_NAVIGATE;
@@ -244,7 +234,7 @@ void ShowSettings(Browser* browser) {
 }
 
 void ShowSettingsSubPage(Browser* browser, const std::string& sub_page) {
-  if (SettingsWindowEnabled()) {
+  if (IsSettingsWindowEnabled()) {
     ShowSettingsSubPageForProfile(browser->profile(), sub_page);
     return;
   }
@@ -253,7 +243,7 @@ void ShowSettingsSubPage(Browser* browser, const std::string& sub_page) {
 
 void ShowSettingsSubPageForProfile(Profile* profile,
                                    const std::string& sub_page) {
-  if (SettingsWindowEnabled()) {
+  if (IsSettingsWindowEnabled()) {
     content::RecordAction(base::UserMetricsAction("ShowOptions"));
     SettingsWindowManager::GetInstance()->ShowChromePageForProfile(
         profile, GetSettingsUrl(sub_page));

chrome/browser/ui/settings_window_manager.h

@@ -30,7 +30,7 @@ class SettingsWindowManager {
   void AddObserver(SettingsWindowManagerObserver* observer);
   void RemoveObserver(SettingsWindowManagerObserver* observer);
 
-  // Shows a chrome:// page (e.g. Settings, History) in an an existing system
+  // Shows a chrome:// page (e.g. Settings, About) in an an existing system
   // Browser window for |profile| or creates a new one.
   void ShowChromePageForProfile(Profile* profile, const GURL& gurl);
 

chrome/browser/ui/settings_window_manager_browsertest.cc

@@ -183,29 +183,25 @@ IN_PROC_BROWSER_TEST_F(SettingsWindowManagerTest, SettingsWindowMultiProfile) {
 }
 #endif
 
-IN_PROC_BROWSER_TEST_F(SettingsWindowManagerTest, OpenSettingsChromePages) {
+IN_PROC_BROWSER_TEST_F(SettingsWindowManagerTest, OpenChromePages) {
+  EXPECT_EQ(1u, chrome::GetTotalBrowserCount());
+
+  // History should open in the existing browser window.
+  chrome::ShowHistory(browser());
   EXPECT_EQ(1u, chrome::GetTotalBrowserCount());
 
   // Settings should open a new browser window.
   chrome::ShowSettings(browser());
   EXPECT_EQ(2u, chrome::GetTotalBrowserCount());
 
-  // History should open a new browser window.
-  CloseNonDefaultBrowsers();
-  EXPECT_EQ(1u, chrome::GetTotalBrowserCount());
-  chrome::ShowHistory(browser());
-  EXPECT_EQ(2u, chrome::GetTotalBrowserCount());
-
-  // Extensions should open a new browser window.
+  // Extensions should open in an existing browser window.
   CloseNonDefaultBrowsers();
   EXPECT_EQ(1u, chrome::GetTotalBrowserCount());
   std::string extension_to_highlight;  // none
   chrome::ShowExtensions(browser(), extension_to_highlight);
-  EXPECT_EQ(2u, chrome::GetTotalBrowserCount());
-
-  // Downloads should NOT open a new browser window.
-  CloseNonDefaultBrowsers();
   EXPECT_EQ(1u, chrome::GetTotalBrowserCount());
+
+  // Downloads should open in an existing browser window.
   chrome::ShowDownloads(browser());
   EXPECT_EQ(1u, chrome::GetTotalBrowserCount());
 }

chrome/browser/ui/webui/uber/uber_ui.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/ui/webui/uber/uber_ui.h"
 
+#include "base/command_line.h"
 #include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/chrome_notification_types.h"
@@ -12,6 +13,7 @@
 #include "chrome/browser/ui/webui/chrome_web_ui_controller_factory.h"
 #include "chrome/browser/ui/webui/extensions/extensions_ui.h"
 #include "chrome/browser/ui/webui/options/options_ui.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/manifest_url_handler.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/navigation_controller.h"
@@ -33,6 +35,11 @@ using content::WebContents;
 
 namespace {
 
+bool IsSettingsWindowEnabled() {
+  return CommandLine::ForCurrentProcess()->HasSwitch(
+      ::switches::kEnableSettingsWindow);
+}
+
 content::WebUIDataSource* CreateUberHTMLSource() {
   content::WebUIDataSource* source =
       content::WebUIDataSource::Create(chrome::kChromeUIUberHost);
@@ -102,19 +109,27 @@ content::WebUIDataSource* CreateUberFrameHTMLSource(Profile* profile) {
   source->AddLocalizedString("shortProductName", IDS_SHORT_PRODUCT_NAME);
 #endif  // defined(OS_CHROMEOS)
 
+  // Group settings and help separately if settings in a window is enabled.
+  base::string16 settings_group(ASCIIToUTF16("settings_group"));
+  base::string16 other_group(ASCIIToUTF16(
+      IsSettingsWindowEnabled() ? "other_group" : "settings_group"));
   source->AddString("extensionsHost",
                     ASCIIToUTF16(chrome::kChromeUIExtensionsHost));
   source->AddLocalizedString("extensionsDisplayName",
                              IDS_MANAGE_EXTENSIONS_SETTING_WINDOWS_TITLE);
+  source->AddString("extensionsGroup", other_group);
   source->AddString("helpHost",
                     ASCIIToUTF16(chrome::kChromeUIHelpHost));
   source->AddLocalizedString("helpDisplayName", IDS_ABOUT_TITLE);
+  source->AddString("helpGroup", settings_group);
   source->AddString("historyHost",
                     ASCIIToUTF16(chrome::kChromeUIHistoryHost));
   source->AddLocalizedString("historyDisplayName", IDS_HISTORY_TITLE);
+  source->AddString("historyGroup", other_group);
   source->AddString("settingsHost",
                     ASCIIToUTF16(chrome::kChromeUISettingsHost));
   source->AddLocalizedString("settingsDisplayName", IDS_SETTINGS_TITLE);
+  source->AddString("settingsGroup", settings_group);
   bool overridesHistory = HasExtensionType(profile,
       chrome::kChromeUIHistoryHost);
   source->AddString("overridesHistory",