ash: Disable window caption buttons in immersive mode

Set them invisible when immersive mode is enabled, but show them when we reveal the toolbar.

Also ensure the caption buttons don't reserve too much horizontal space when the immersive mode button is invisible. Otherwise the tab strip doesn't have enough space.

BUG=168855
TEST=added browser_tests BrowserNonClientFrameViewAshTest.ImmersiveMode

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

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

7 files changed, 112 insertions, 10 deletions

diff --git a/ash/wm/frame_painter.cc b/ash/wm/frame_painter.cc
index 5ccda29..b680069 100644
--- a/ash/wm/frame_painter.cc
+++ b/ash/wm/frame_painter.cc
@@ -322,7 +322,7 @@ gfx::Size FramePainter::GetMinimumSize(views::NonClientFrameView* view) {
   int title_width = GetTitleOffsetX() +
       size_button_->width() + kSizeButtonOffsetX +
       close_button_->width() + kCloseButtonOffsetX;
-  if (immersive_button_)
+  if (immersive_button_ && immersive_button_->visible())
     title_width += immersive_button_->width() + kImmersiveButtonOffsetX;
   if (title_width > min_size.width())
     min_size.set_width(title_width);
@@ -338,7 +338,7 @@ int FramePainter::GetRightInset() const {
   gfx::Size size_button_size = size_button_->GetPreferredSize();
   int inset = close_size.width() + kCloseButtonOffsetX +
       size_button_size.width() + kSizeButtonOffsetX;
-  if (immersive_button_) {
+  if (immersive_button_ && immersive_button_->visible()) {
     gfx::Size immersive_size = immersive_button_->GetPreferredSize();
     inset += immersive_size.width() + kImmersiveButtonOffsetX;
   }
diff --git a/chrome/browser/chromeos/memory/oom_priority_manager.cc b/chrome/browser/chromeos/memory/oom_priority_manager.cc
index 81f4cdb..3973811 100644
--- a/chrome/browser/chromeos/memory/oom_priority_manager.cc
+++ b/chrome/browser/chromeos/memory/oom_priority_manager.cc
@@ -31,6 +31,7 @@
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/common/url_constants.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
@@ -74,8 +75,9 @@ const int kAdjustmentIntervalSeconds = 10;
 const int kSuspendThresholdSeconds = kAdjustmentIntervalSeconds * 4;
 
 // The default interval in milliseconds to wait before setting the score of
-// currently focused tab.
-const int kFocusedTabScoreAdjustIntervalMs = 500;
+// currently focused tab. Must be be long enough that a user who is flipping
+// through tabs with Ctrl-Tab does not mark each every tab as "focused".
+const int kFocusedTabScoreAdjustIntervalMs = 2000;
 
 // Returns a unique ID for a WebContents.  Do not cast back to a pointer, as
 // the WebContents could be deleted if the user closed the tab.
@@ -83,6 +85,30 @@ int64 IdFromWebContents(WebContents* web_contents) {
   return reinterpret_cast<int64>(web_contents);
 }
 
+// Returns true if the URL for |contents| indicates an internal Chrome web UI
+// page that can be easily reloaded and hence makes a good choice to discard.
+bool IsReloadableUI(WebContents* web_contents) {
+  // There are many internal URLs, but only look for the ones that users are
+  // likely to have open. Most of the benefit is the from NTP URL.
+  const char* kReloadableUrlPrefixes[] = {
+      chrome::kChromeUIDownloadsURL,
+      chrome::kChromeUIHistoryURL,
+      chrome::kChromeUINewTabURL,
+      chrome::kChromeUISettingsURL,
+  };
+  GURL url = web_contents->GetURL();
+  LOG(ERROR) << "JAMESDEBUG url " << url.spec();
+  // Prefix-match against the table above. Use strncmp to avoid allocating
+  // memory to convert the URL prefix constants into std::strings.
+  for (size_t i = 0; i < arraysize(kReloadableUrlPrefixes); ++i) {
+    if (!strncmp(url.spec().c_str(),
+                 kReloadableUrlPrefixes[i],
+                 strlen(kReloadableUrlPrefixes[i])))
+      return true;
+  }
+  return false;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // OomMemoryDetails logs details about all Chrome processes during an out-of-
 // memory event in an attempt to identify the culprit, then discards a tab and
@@ -133,6 +159,7 @@ void OomMemoryDetails::OnDetailsAvailable() {
 
 OomPriorityManager::TabStats::TabStats()
   : is_app(false),
+    is_reloadable_ui(false),
     is_pinned(false),
     is_selected(false),
     is_discarded(false),
@@ -198,6 +225,7 @@ std::vector<string16> OomPriorityManager::GetTabTitles() {
     str += base::IntToString16(score);
     str += ASCIIToUTF16(")");
     str += ASCIIToUTF16(it->is_app ? " app" : "");
+    str += ASCIIToUTF16(it->is_reloadable_ui ? " discardable_ui" : "");
     str += ASCIIToUTF16(it->is_pinned ? " pinned" : "");
     str += ASCIIToUTF16(it->is_discarded ? " discarded" : "");
     titles.push_back(str);
@@ -327,18 +355,23 @@ int OomPriorityManager::GetTabCount() const {
 // than |second|.
 bool OomPriorityManager::CompareTabStats(TabStats first,
                                          TabStats second) {
-  // Being currently selected is most important.
+  // Being currently selected is most important to protect.
   if (first.is_selected != second.is_selected)
-    return first.is_selected == true;
+    return first.is_selected;
+
+  // Tab with internal web UI like NTP or Settings are good choices to discard,
+  // so protect non-Web UI and let the other conditionals finish the sort.
+  if (first.is_reloadable_ui != second.is_reloadable_ui)
+    return !first.is_reloadable_ui;
 
-  // Being pinned is second most important.
+  // Being pinned is important to protect.
   if (first.is_pinned != second.is_pinned)
-    return first.is_pinned == true;
+    return first.is_pinned;
 
   // Being an app is important too, as you're the only visible surface in the
   // window and we don't want to discard that.
   if (first.is_app != second.is_app)
-    return first.is_app == true;
+    return first.is_app;
 
   // TODO(jamescook): Incorporate sudden_termination_allowed into the sort
   // order.  We don't do this now because pages with unload handlers set
@@ -468,6 +501,7 @@ OomPriorityManager::TabStatsList OomPriorityManager::GetTabStatsOnUIThread() {
       if (!contents->IsCrashed()) {
         TabStats stats;
         stats.is_app = is_browser_for_app;
+        stats.is_reloadable_ui = IsReloadableUI(contents);
         stats.is_pinned = model->IsTabPinned(i);
         stats.is_selected = browser_active && model->IsTabSelected(i);
         stats.is_discarded = model->IsTabDiscarded(i);
diff --git a/chrome/browser/chromeos/memory/oom_priority_manager.h b/chrome/browser/chromeos/memory/oom_priority_manager.h
index bad0a40..a4a56a3 100644
--- a/chrome/browser/chromeos/memory/oom_priority_manager.h
+++ b/chrome/browser/chromeos/memory/oom_priority_manager.h
@@ -64,6 +64,7 @@ class OomPriorityManager : public content::NotificationObserver {
     TabStats();
     ~TabStats();
     bool is_app;  // browser window is an app
+    bool is_reloadable_ui;  // Reloadable web UI page, like NTP or Settings.
     bool is_pinned;
     bool is_selected;  // selected in the currently active browser window
     bool is_discarded;
diff --git a/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc b/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
index 0c560c1..66ddf16 100644
--- a/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
+++ b/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
@@ -197,6 +197,16 @@ void BrowserNonClientFrameViewAsh::GetWindowMask(const gfx::Size& size,
 }
 
 void BrowserNonClientFrameViewAsh::ResetWindowControls() {
+  // Hide the caption buttons in immersive mode because it's confusing when
+  // the user hovers or clicks in the top-right of the screen and hits one.
+  // Only show them during a reveal.
+  ImmersiveModeController* controller =
+      browser_view()->immersive_mode_controller();
+  bool show_buttons = !controller->enabled() || controller->IsRevealed();
+  immersive_button_->SetVisible(show_buttons);
+  size_button_->SetVisible(show_buttons);
+  close_button_->SetVisible(show_buttons);
+
   size_button_->SetState(views::CustomButton::STATE_NORMAL);
   // The close button isn't affected by this constraint.
 }
diff --git a/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h b/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
index e1d3102..445dab9 100644
--- a/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
+++ b/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
@@ -68,6 +68,7 @@ class BrowserNonClientFrameViewAsh
 
  private:
   FRIEND_TEST_ALL_PREFIXES(BrowserNonClientFrameViewAshTest, WindowHeader);
+  FRIEND_TEST_ALL_PREFIXES(BrowserNonClientFrameViewAshTest, ImmersiveMode);
 
   // Distance between top of window and client area.
   int NonClientTopBorderHeight(bool force_restored) const;
diff --git a/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc b/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
index 4d3b3d9..da5b29f 100644
--- a/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
+++ b/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
@@ -7,8 +7,10 @@
 #include "ash/ash_constants.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/views/immersive_mode_controller.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "ui/base/hit_test.h"
+#include "ui/views/controls/button/image_button.h"
 #include "ui/views/widget/widget.h"
 
 using views::Widget;
@@ -17,7 +19,8 @@ typedef InProcessBrowserTest BrowserNonClientFrameViewAshTest;
 
 IN_PROC_BROWSER_TEST_F(BrowserNonClientFrameViewAshTest, WindowHeader) {
   // We know we're using Views, so static cast.
-  Widget* widget = static_cast<BrowserView*>(browser()->window())->GetWidget();
+  BrowserView* browser_view = static_cast<BrowserView*>(browser()->window());
+  Widget* widget = browser_view->GetWidget();
   // We know we're using Ash, so static cast.
   BrowserNonClientFrameViewAsh* frame_view =
       static_cast<BrowserNonClientFrameViewAsh*>(
@@ -69,3 +72,44 @@ IN_PROC_BROWSER_TEST_F(BrowserNonClientFrameViewAshTest, WindowHeader) {
   app_widget->SetBounds(gfx::Rect(15, 15, 250, 250));
   EXPECT_FALSE(app_frame_view->UseShortHeader());
 }
+
+IN_PROC_BROWSER_TEST_F(BrowserNonClientFrameViewAshTest, ImmersiveMode) {
+  // We know we're using Views, so static cast.
+  BrowserView* browser_view = static_cast<BrowserView*>(browser()->window());
+  Widget* widget = browser_view->GetWidget();
+  // We know we're using Ash, so static cast.
+  BrowserNonClientFrameViewAsh* frame_view =
+      static_cast<BrowserNonClientFrameViewAsh*>(
+          widget->non_client_view()->frame_view());
+
+  // Normal window does not have immersive mode button.
+  EXPECT_FALSE(frame_view->immersive_button_->visible());
+
+  // Maximized window shows immersive mode button.
+  widget->Maximize();
+  EXPECT_TRUE(frame_view->immersive_button_->visible());
+
+  // Entering immersive mode hides the caption buttons.
+  browser_view->immersive_mode_controller()->SetEnabled(true);
+  EXPECT_FALSE(frame_view->immersive_button_->visible());
+  EXPECT_FALSE(frame_view->size_button_->visible());
+  EXPECT_FALSE(frame_view->close_button_->visible());
+
+  // An immersive reveal shows the buttons.
+  browser_view->immersive_mode_controller()->StartRevealForTest();
+  EXPECT_TRUE(frame_view->immersive_button_->visible());
+  EXPECT_TRUE(frame_view->size_button_->visible());
+  EXPECT_TRUE(frame_view->close_button_->visible());
+
+  // Ending reveal hides them again.
+  browser_view->immersive_mode_controller()->CancelReveal();
+  EXPECT_FALSE(frame_view->immersive_button_->visible());
+  EXPECT_FALSE(frame_view->size_button_->visible());
+  EXPECT_FALSE(frame_view->close_button_->visible());
+
+  // Exiting immersive mode makes them visible again.
+  browser_view->immersive_mode_controller()->SetEnabled(false);
+  EXPECT_TRUE(frame_view->immersive_button_->visible());
+  EXPECT_TRUE(frame_view->size_button_->visible());
+  EXPECT_TRUE(frame_view->close_button_->visible());
+}
diff --git a/chrome/browser/ui/views/immersive_mode_controller.cc b/chrome/browser/ui/views/immersive_mode_controller.cc
index 301dc25..99f505e 100644
--- a/chrome/browser/ui/views/immersive_mode_controller.cc
+++ b/chrome/browser/ui/views/immersive_mode_controller.cc
@@ -310,6 +310,10 @@ void ImmersiveModeController::SetEnabled(bool enabled) {
   }
 #endif
 
+  // Ensure window caption buttons are shown/hidden appropriately.
+  browser_view_->frame()->non_client_view()->frame_view()->
+      ResetWindowControls();
+
   // Always ensure tab strip is in correct state.
   browser_view_->tabstrip()->SetImmersiveStyle(enabled_);
   browser_view_->Layout();
@@ -408,6 +412,10 @@ void ImmersiveModeController::StartReveal() {
     return;
   revealed_ = true;
 
+  // Reveal shows the window caption buttons.
+  browser_view_->frame()->non_client_view()->frame_view()->
+      ResetWindowControls();
+
   // Recompute the bounds of the views when painted normally.
   browser_view_->tabstrip()->SetImmersiveStyle(false);
   browser_view_->Layout();
@@ -468,6 +476,10 @@ void ImmersiveModeController::EndReveal(Animate animate, Layout layout) {
   }
 
   if (layout == LAYOUT_YES) {
+    // Ending reveal hides the window caption buttons.
+    browser_view_->frame()->non_client_view()->frame_view()->
+        ResetWindowControls();
+
     browser_view_->tabstrip()->SetImmersiveStyle(enabled_);
     browser_view_->Layout();
   }