Add an experimental keyboard lock, where Esc must be held to exit.

chrome/browser/ui/exclusive_access/exclusive_access_manager.cc

Index: chrome/browser/ui/exclusive_access/exclusive_access_manager.cc
diff --git a/chrome/browser/ui/exclusive_access/exclusive_access_manager.cc b/chrome/browser/ui/exclusive_access/exclusive_access_manager.cc
index 666d11e6970660990014e60c2a88d65f567d018b..852e7f2c40ea1b6b96df5c43d8174f503dc30d4f 100644
--- a/chrome/browser/ui/exclusive_access/exclusive_access_manager.cc
+++ b/chrome/browser/ui/exclusive_access/exclusive_access_manager.cc
@@ -19,11 +19,19 @@
 
 using content::WebContents;
 
+namespace {
+
+// Time in milliseconds to hold the Esc key in order to exit full screen.
+const int kHoldEscapeTimeMs = 1500;
+
+}
+
 ExclusiveAccessManager::ExclusiveAccessManager(
     ExclusiveAccessContext* exclusive_access_context)
     : exclusive_access_context_(exclusive_access_context),
       fullscreen_controller_(this),
-      mouse_lock_controller_(this) {
+      mouse_lock_controller_(this),
+      hold_timer_(true, false) {
 }
 
 ExclusiveAccessManager::~ExclusiveAccessManager() {
@@ -58,6 +66,8 @@ ExclusiveAccessManager::GetExclusiveAccessExitBubbleType() const {
   if (fullscreen_controller_.IsUserAcceptedFullscreen()) {
     if (fullscreen_controller_.IsPrivilegedFullscreenForTab())
       return EXCLUSIVE_ACCESS_BUBBLE_TYPE_NONE;
+    if (IsExperimentalKeyboardLockUIEnabled())

[msw, 2016/03/23 07:20:54]

Shouldn't this key off of something other than the experiment being enabled
(like the page actually engaging this keyboard lock mode?), or is this just a
hacky way of testing the mode anytime a site enters fullscreen with the
experiment enabled?

[dominickn, 2016/03/23 10:07:03]

This is a hacky way of testing the mode for now - we want to get some feedback
from UI. Plus, much of this code will soon disappear when
simplified-fullscreen-ui is the default on all platforms, so it'll be good to
wait until after then before introducing more complexity.

+      return EXCLUSIVE_ACCESS_BUBBLE_TYPE_KEYBOARD_LOCK_EXIT_INSTRUCTION;
     if (mouse_lock_controller_.IsMouseLocked())
       return EXCLUSIVE_ACCESS_BUBBLE_TYPE_FULLSCREEN_MOUSELOCK_EXIT_INSTRUCTION;
     if (mouse_lock_controller_.IsMouseLockRequested())
@@ -91,6 +101,11 @@ GURL ExclusiveAccessManager::GetExclusiveAccessBubbleURL() const {
 }
 
 // static
+bool ExclusiveAccessManager::IsExperimentalKeyboardLockUIEnabled() {
+  return base::FeatureList::IsEnabled(features::kExperimentalKeyboardLockUI);
+}
+
+// static
 bool ExclusiveAccessManager::IsSimplifiedFullscreenUIEnabled() {
   return base::FeatureList::IsEnabled(features::kSimplifiedFullscreenUI);
 }
@@ -117,10 +132,31 @@ bool ExclusiveAccessManager::HandleUserKeyPress(
     return false;
   }
 
-  bool handled = false;
-  handled = fullscreen_controller_.HandleUserPressedEscape();
-  handled |= mouse_lock_controller_.HandleUserPressedEscape();
-  return handled;
+  if (IsExperimentalKeyboardLockUIEnabled()) {
+    if (event.type == content::NativeWebKeyboardEvent::KeyUp &&

[msw, 2016/03/23 07:20:54]

Shouldn't this check if the key pressed/released is esc?

[dominickn, 2016/03/23 10:07:03]

Only Esc will get this far - all other keys are captured in the if statement
above.

[msw, 2016/03/23 17:07:32]

Acknowledged.

+        hold_timer_.IsRunning()) {
+      // Seeing a key up event on Esc with the hold timer running cancels the
+      // timer and doesn't exit. This means the user pressed Esc, but not long
+      // enough to trigger an exit
+      hold_timer_.Stop();
+    } else if (event.type == content::NativeWebKeyboardEvent::RawKeyDown &&
+              !hold_timer_.IsRunning()) {
+      // Seeing a key down event on Esc when the hold timer is stopped starts
+      // the timer. When the timer reaches 0, the callback will trigger an exit
+      // from fullscreen/mouselock.
+      hold_timer_.Start(
+          FROM_HERE, base::TimeDelta::FromMilliseconds(kHoldEscapeTimeMs),
+          base::Bind(&ExclusiveAccessManager::HandleUserHeldEscape,
+                     base::Unretained(this)));
+    }
+    // We never handle the keyboard event.
+    return false;
+  } else {

[msw, 2016/03/23 07:20:55]

nit: no else after return (just put the old code below the conditional block)

[dominickn, 2016/03/23 10:07:03]

Done.

+    bool handled = false;
+    handled = fullscreen_controller_.HandleUserPressedEscape();
+    handled |= mouse_lock_controller_.HandleUserPressedEscape();

[msw, 2016/03/23 07:20:55]

Should this feature add a corresponding key_lock_controller?

[dominickn, 2016/03/23 10:07:03]

At this stage, we want to test how the hold to escape interaction feels. I feel
like adding a full new controller should come as we scope out the feature more
(and refactor. But if you prefer, I can add a new controller now. Thoughts?

[msw, 2016/03/23 17:07:32]

Nope; now I understand the hackyness level; this is fine.

+    return handled;
+  }
 }
 
 void ExclusiveAccessManager::OnUserInput() {
@@ -159,6 +195,7 @@ void ExclusiveAccessManager::RecordBubbleReshownUMA(
       break;
     case EXCLUSIVE_ACCESS_BUBBLE_TYPE_FULLSCREEN_BUTTONS:
     case EXCLUSIVE_ACCESS_BUBBLE_TYPE_FULLSCREEN_EXIT_INSTRUCTION:
+    case EXCLUSIVE_ACCESS_BUBBLE_TYPE_KEYBOARD_LOCK_EXIT_INSTRUCTION:
     case EXCLUSIVE_ACCESS_BUBBLE_TYPE_BROWSER_FULLSCREEN_EXIT_INSTRUCTION:
     case EXCLUSIVE_ACCESS_BUBBLE_TYPE_EXTENSION_FULLSCREEN_EXIT_INSTRUCTION:
       // Only fullscreen in effect.
@@ -181,3 +218,8 @@ void ExclusiveAccessManager::RecordBubbleReshownUMA(
   if (mouselock)
     mouse_lock_controller_.RecordBubbleReshownUMA();
 }
+
+void ExclusiveAccessManager::HandleUserHeldEscape() {
+  fullscreen_controller_.HandleUserPressedEscape();
+  mouse_lock_controller_.HandleUserPressedEscape();
+}