Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(76)

Issue 1535643006: Refactor the IsKeyboardPresentOnSlate function. (Closed)

Created:
5 years ago by ananta
Modified:
4 years, 11 months ago
Reviewers:
jschuh, sky
CC:
chromium-reviews, grt+watch_chromium.org, wfh+watch_chromium.org, vmpstr+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Refactor the IsKeyboardPresentOnSlate function. Currently this function shares the same logic as the IsTabletDevice function. One of the big differences was the IsTabletDevice function allows calls from Windows XP. Changed that to Windows 8+ and changed the IsKeyboardPresentOnSlate function to call IsTabletDevice. The IsTabletDevice function now takes in a reason argument which is updated with the reason info as to why the device was deemed to be a tablet or not. We now use the PowerDeterminePlatformRoleEx API to determine the platform role. As per msdn this function returns better results on Windows 8+. Next step would be to see if this function reliably works for tablets and slate mode. If yes then we can possibly flip the switch kDisableUsbKeyboardDetect to kEnableUsbKeyboardDetect which would ensure that the OSK pops up by default. BUG=497381 TBR=sky Committed: https://crrev.com/44366f4e8c4970cb49d47c3b38063a5ae01b989a Cr-Commit-Position: refs/heads/master@{#368241}

Patch Set 1 #

Total comments: 4

Patch Set 2 : Address review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+64 lines, -41 lines) Patch
M base/win/win_util.h View 1 chunk +8 lines, -3 lines 0 comments Download
M base/win/win_util.cc View 1 5 chunks +55 lines, -37 lines 0 comments Download
M chrome/browser/chrome_browser_main_win.cc View 1 chunk +1 line, -1 line 0 comments Download

Depends on Patchset:

Messages

Total messages: 16 (7 generated)
ananta
5 years ago (2015-12-19 02:33:41 UTC) #2
jschuh
Sorry for the delay. https://codereview.chromium.org/1535643006/diff/1/base/win/win_util.cc File base/win/win_util.cc (right): https://codereview.chromium.org/1535643006/diff/1/base/win/win_util.cc#newcode105 base/win/win_util.cc:105: HMODULE power_prof_module = ::GetModuleHandle(L"powrprof.dll"); Skip ...
4 years, 11 months ago (2016-01-07 01:18:40 UTC) #3
ananta
https://codereview.chromium.org/1535643006/diff/1/base/win/win_util.cc File base/win/win_util.cc (right): https://codereview.chromium.org/1535643006/diff/1/base/win/win_util.cc#newcode105 base/win/win_util.cc:105: HMODULE power_prof_module = ::GetModuleHandle(L"powrprof.dll"); On 2016/01/07 01:18:40, jschuh (very ...
4 years, 11 months ago (2016-01-07 22:41:05 UTC) #4
jschuh
lgtm
4 years, 11 months ago (2016-01-08 00:27:26 UTC) #5
ananta
TBR'ing sky for chrome\browser owners.
4 years, 11 months ago (2016-01-08 00:39:57 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1535643006/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1535643006/20001
4 years, 11 months ago (2016-01-08 01:41:08 UTC) #11
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 11 months ago (2016-01-08 01:45:49 UTC) #13
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/44366f4e8c4970cb49d47c3b38063a5ae01b989a Cr-Commit-Position: refs/heads/master@{#368241}
4 years, 11 months ago (2016-01-08 01:46:52 UTC) #15
sky
4 years, 11 months ago (2016-01-08 16:26:39 UTC) #16
Message was sent while issue was closed.
LGTM

Powered by Google App Engine
This is Rietveld 408576698