Improve tablet mode detection on Windows 10+

base/win/win_util.cc

Index: base/win/win_util.cc
diff --git a/base/win/win_util.cc b/base/win/win_util.cc
index a1863c7043e642bd93b968fdc342cb70d6c90785..5485fde914999387aa1ee214438f96a19d03c2b2 100644
--- a/base/win/win_util.cc
+++ b/base/win/win_util.cc
@@ -6,20 +6,24 @@
 
 #include <aclapi.h>
 #include <cfgmgr32.h>
+#include <hstring.h>
 #include <lm.h>
 #include <powrprof.h>
 #include <shellapi.h>
 #include <shlobj.h>
 #include <shobjidl.h>  // Must be before propkey.
 #include <initguid.h>
+#include <inspectable.h>
 #include <propkey.h>
 #include <propvarutil.h>
 #include <psapi.h>
+#include <roapi.h>
 #include <sddl.h>
 #include <setupapi.h>
 #include <signal.h>
 #include <stddef.h>
 #include <stdlib.h>
+#include <winstring.h>
 
 #include "base/base_switches.h"
 #include "base/command_line.h"
@@ -33,6 +37,7 @@
 #include "base/threading/thread_restrictions.h"
 #include "base/win/registry.h"
 #include "base/win/scoped_co_mem.h"
+#include "base/win/scoped_comptr.h"
 #include "base/win/scoped_handle.h"
 #include "base/win/scoped_propvariant.h"
 #include "base/win/windows_version.h"
@@ -107,6 +112,119 @@ POWER_PLATFORM_ROLE GetPlatformRole() {
   return PowerDeterminePlatformRoleEx(POWER_PLATFORM_ROLE_V2);
 }
 
+// Windows 10 stuff defined while we wait for Windows 10 headers.
+// These definitions need to be removed once we have the new headers.

[jschuh, 2016/03/11 22:01:44]

How close are we to landing the Win10 SDK? Because I'd prefer to avoid this if
possible.

If we can't avoid it, then please add a TODO with a bug and put this behind an
"#if VER_PRODUCTBUILD > X" macro and a "#pragma message()" in the else case so
we notice that the version rolled forward.

[Will Harris, 2016/03/11 22:05:06]

I thought we had landed Win10 SDK since it was needed for the new process
mitigations.

SDK in my build dir is 10.0.10586.0 - is that not recent enough?

[ananta, 2016/03/11 23:14:14]

Removed these definitions. Thanks to Will for letting me know that we can use
the win10 sdk includes

+namespace ABI {
+namespace Windows {
+namespace UI {
+namespace ViewManagement {
+
+enum UserInteractionMode {
+  UserInteractionMode_Mouse = 0,
+  UserInteractionMode_Touch = 1
+};
+
+interface __declspec(uuid("C63657F6-8850-470D-88F8-455E16EA2C26"))
+    IUIViewSettings : public IInspectable {
+  public:
+  virtual HRESULT STDMETHODCALLTYPE get_UserInteractionMode(
+      UserInteractionMode* value) = 0;
+};
+
+extern const __declspec(selectany) IID & IID_IUIViewSettings =
+    __uuidof(IUIViewSettings);
+
+}  // namespace ViewManagement
+}  // namespace UI
+}  // namespace Windows
+}  // namespace ABI
+
+interface __declspec(uuid("3694dbf9-8f68-44be-8ff5-195c98ede8a6"))
+    IUIViewSettingsInterop : public IInspectable {
+ public:
+  virtual HRESULT STDMETHODCALLTYPE GetForWindow(HWND window, REFIID riid,
+      void** ppv) = 0;
+};
+
+// Uses the Windows 10 WRL API's to query the current system state. The API's
+// we are using in the function below are supported in Win32 apps as per msdn.
+// It looks like the API implementation is buggy at least on Surface 4 causing
+// it to always return UserInteractionMode_Touch which as per documentation
+// indicates tablet mode.
+bool IsWindows10TabletDevice() {
+  if (GetVersion() < VERSION_WIN10)
+    return false;
+
+  typedef HRESULT (WINAPI* GetActivationFactory)(HSTRING class_id,

[jschuh, 2016/03/11 22:01:45]

ROGetActivationFactoryFunction

[Will Harris, 2016/03/11 22:05:06]

consider using typedef decltype if the prototype is declared in one of the
headers.

[ananta, 2016/03/11 23:14:14]

Thanks. Removed these

+                                                 REFIID  iid,
+                                                 void** factory);
+
+  typedef HRESULT (WINAPI* CreateString)(LPCWSTR source,

[jschuh, 2016/03/11 22:01:45]

WindowsCreateStringFunction

[ananta, 2016/03/11 23:14:14]

Done

+                                         UINT32 length,
+                                         HSTRING* target);
+
+  static GetActivationFactory get_factory = nullptr;
+  static CreateString create_string = nullptr;
+
+  if (!get_factory) {
+    DCHECK_EQ(create_string, static_cast<CreateString>(nullptr));
+
+    HMODULE combase_dll = ::LoadLibrary(L"combase.dll");
+    if (!combase_dll)
+      return false;
+
+    get_factory = reinterpret_cast<GetActivationFactory>(
+        ::GetProcAddress(combase_dll, "RoGetActivationFactory"));
+    if (!get_factory) {
+      CHECK(false);
+      return false;
+    }
+
+    create_string = reinterpret_cast<CreateString>(
+        ::GetProcAddress(combase_dll, "WindowsCreateString"));
+    if (!create_string) {
+      CHECK(false);
+      return false;
+    }
+  }
+
+  HRESULT hr = E_FAIL;
+  // This HSTRING is allocated on the heap and is leaked.
+  static HSTRING view_settings_guid = NULL;

[jschuh, 2016/03/11 22:01:45]

Is there a race here where you could allocate the string more than once?

[ananta, 2016/03/11 23:14:14]

No. This only gets called from the UI thread

+  if (!view_settings_guid) {
+    hr = create_string(L"Windows.UI.ViewManagement.UIViewSettings",
+                       static_cast<UINT32>(
+                          wcslen(L"Windows.UI.ViewManagement.UIViewSettings")),

[jschuh, 2016/03/11 22:01:45]

Declare this as a static constant so you don't have to repeat it, and use
base::arraysize() - 1 to get the length.

[ananta, 2016/03/11 23:14:14]

We use the defined constant from the include file now

+                       &view_settings_guid);
+    if (FAILED(hr))
+      return false;
+  }
+
+  base::win::ScopedComPtr<IUIViewSettingsInterop> view_settings_interop;
+  hr = get_factory(view_settings_guid,
+                   __uuidof(IUIViewSettingsInterop),
+                    view_settings_interop.ReceiveVoid());
+  if (FAILED(hr))
+    return false;
+
+  base::win::ScopedComPtr<ABI::Windows::UI::ViewManagement::IUIViewSettings>
+      view_settings;
+  // TODO(ananta)
+  // Avoid using GetForegroundWindow here and pass in the HWND of the window
+  // intiating the request to display the keyboard.
+  hr = view_settings_interop->GetForWindow(
+      ::GetForegroundWindow(),
+      __uuidof(ABI::Windows::UI::ViewManagement::IUIViewSettings),
+      view_settings.ReceiveVoid());
+  if (FAILED(hr))
+    return false;
+
+  ABI::Windows::UI::ViewManagement::UserInteractionMode mode =
+      ABI::Windows::UI::ViewManagement::UserInteractionMode_Mouse;
+  view_settings->get_UserInteractionMode(&mode);

[jschuh, 2016/03/11 22:01:45]

Are we sure the overhead is acceptable in doing this on every gesture event? It
seems like we should really just be setting the tablet/keyboard state at at key
times like start, focus change, or when we get state change events. And then we
should globally track that state.

[ananta, 2016/03/11 23:14:14]

There should not be much overhead here. The functions just check a state
variable to determine the mode. I am not worried about the cost of the QI's etc.

[jschuh, 2016/03/11 23:50:26]

Okay. It just seems like we keep piling more logic in this call stack when all
we really want is a boolean that's tied to state changes. How about filing a bug
for someone to refactor it that way, if we ever get around to it?

+  return mode == ABI::Windows::UI::ViewManagement::UserInteractionMode_Touch;
+}
+
 }  // namespace
 
 // Returns true if a physical keyboard is detected on Windows 8 and up.
@@ -141,10 +259,11 @@ bool IsKeyboardPresentOnSlate(std::string* reason) {
     }
   }
 
+  // If it is a tablet device we assume that there is no keyboard attached.
   if (IsTabletDevice(reason)) {
     if (reason)
       *reason += "Tablet device.\n";
-    return true;
+    return false;
   } else {
     if (reason) {
       *reason += "Not a tablet device";
@@ -391,6 +510,9 @@ bool IsTabletDevice(std::string* reason) {
     return false;
   }
 
+  if (IsWindows10TabletDevice())
+    return true;
+
   if (GetSystemMetrics(SM_MAXIMUMTOUCHES) == 0) {
     if (reason) {
       *reason += "Device does not support touch.\n";
@@ -414,8 +536,9 @@ bool IsTabletDevice(std::string* reason) {
   bool slate_power_profile = (role == PlatformRoleSlate);
 
   bool is_tablet = false;
-
+  bool is_tablet_pc = false;
   if (mobile_power_profile || slate_power_profile) {
+    is_tablet_pc = !GetSystemMetrics(SM_TABLETPC);
     is_tablet = !GetSystemMetrics(SM_CONVERTIBLESLATEMODE);
     if (!is_tablet) {
       if (reason) {
@@ -433,7 +556,7 @@ bool IsTabletDevice(std::string* reason) {
     if (reason)
       *reason += "Device role is not mobile or slate.\n";
   }
-  return is_tablet;
+  return is_tablet && is_tablet_pc;
 }
 
 bool DisplayVirtualKeyboard() {