|
|
Created:
4 years, 5 months ago by Will Shackleton Modified:
4 years, 5 months ago CC:
chromium-reviews, tdresser+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionDisabled xinput2 until edge cases can be fixed.
Forces all input devices to use xinput1, the previous path,
until all the kinks are worked out with the new xinput2 path.
BUG=616308
Committed: https://crrev.com/f7e731e7822967ab2d22530ea8d8487cf623ae3b
Cr-Commit-Position: refs/heads/master@{#402966}
Patch Set 1 #Patch Set 2 : Changed xinput to xinput2 #
Total comments: 1
Patch Set 3 : Changed comment to TODO and removed if #Messages
Total messages: 23 (9 generated)
Description was changed from ========== Disabled xinput2 until edge cases can be fixed. I disabled xinput2 to get M52 working in time. Once I get back from holiday we can look at re-enabling for a later release. We could also make xinput2 a chrome://flags flag to allow mitigation in case of more strange edge cases. BUG=593453 ========== to ========== Disabled xinput2 until edge cases can be fixed. I disabled xinput2 to get M52 working in time. Once I get back from holiday we can look at re-enabling for a later release. We could also make xinput2 a chrome://flags flag to allow mitigation in case of more strange edge cases. BUG=593453 ==========
w.shackleton@gmail.com changed reviewers: + bokan@chromium.org, sadrul@chromium.org
https://codereview.chromium.org/2108283002/diff/20001/ui/events/devices/x11/d... File ui/events/devices/x11/device_data_manager_x11.cc (right): https://codereview.chromium.org/2108283002/diff/20001/ui/events/devices/x11/d... ui/events/devices/x11/device_data_manager_x11.cc:776: if (xinput2_disabled) { No need for the variable and if statement. Just return. Make the comment a TODO and add a link to crbug.com/616308
On 2016/06/29 18:34:25, bokan wrote: > https://codereview.chromium.org/2108283002/diff/20001/ui/events/devices/x11/d... > File ui/events/devices/x11/device_data_manager_x11.cc (right): > > https://codereview.chromium.org/2108283002/diff/20001/ui/events/devices/x11/d... > ui/events/devices/x11/device_data_manager_x11.cc:776: if (xinput2_disabled) { > No need for the variable and if statement. Just return. Make the comment a TODO > and add a link to crbug.com/616308 Done. I thought clang would complain at that. Compiling is slow for me but I won't commit before checking it compiles and does the job.
I tried this out locally with touchpad/pointer and looks like its back to non-smooth scrolling as expected (our animated smooth scroll actually makes this mode "almost smooth") so lgtm from me. Sadrul still needs to stamp for OWNER approval.
Also, please use bug 616308 in the CL description.
Description was changed from ========== Disabled xinput2 until edge cases can be fixed. I disabled xinput2 to get M52 working in time. Once I get back from holiday we can look at re-enabling for a later release. We could also make xinput2 a chrome://flags flag to allow mitigation in case of more strange edge cases. BUG=593453 ========== to ========== Disabled xinput2 until edge cases can be fixed. I disabled xinput2 to get M52 working in time. Once I get back from holiday we can look at re-enabling for a later release. We could also make xinput2 a chrome://flags flag to allow mitigation in case of more strange edge cases. BUG=616308 ==========
On 2016/06/29 18:48:53, bokan wrote: > Also, please use bug 616308 in the CL description. Done
The CQ bit was checked by bokan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm I would remove 2nd and 3rd paragraphs from the CL description.
Description was changed from ========== Disabled xinput2 until edge cases can be fixed. I disabled xinput2 to get M52 working in time. Once I get back from holiday we can look at re-enabling for a later release. We could also make xinput2 a chrome://flags flag to allow mitigation in case of more strange edge cases. BUG=616308 ========== to ========== Disabled xinput2 until edge cases can be fixed. Forces all input devices to use xinput1, the previous path, until all the kinks are worked out with the new xinput2 path. BUG=616308 ==========
Thanks Sadrul, I've updated the description and I'll try to get this committed tonight since time is of the essence.
On 2016/06/29 21:50:26, bokan wrote: > Thanks Sadrul, I've updated the description and I'll try to get this committed > tonight since time is of the essence. Thanks sadrul and bokan. I'll be away from computers until the 11th but can answer emails.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by bokan@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Disabled xinput2 until edge cases can be fixed. Forces all input devices to use xinput1, the previous path, until all the kinks are worked out with the new xinput2 path. BUG=616308 ========== to ========== Disabled xinput2 until edge cases can be fixed. Forces all input devices to use xinput1, the previous path, until all the kinks are worked out with the new xinput2 path. BUG=616308 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
CQ bit was unchecked.
Message was sent while issue was closed.
Description was changed from ========== Disabled xinput2 until edge cases can be fixed. Forces all input devices to use xinput1, the previous path, until all the kinks are worked out with the new xinput2 path. BUG=616308 ========== to ========== Disabled xinput2 until edge cases can be fixed. Forces all input devices to use xinput1, the previous path, until all the kinks are worked out with the new xinput2 path. BUG=616308 Committed: https://crrev.com/f7e731e7822967ab2d22530ea8d8487cf623ae3b Cr-Commit-Position: refs/heads/master@{#402966} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/f7e731e7822967ab2d22530ea8d8487cf623ae3b Cr-Commit-Position: refs/heads/master@{#402966} |