Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1591083002:
blimp: Fix input events sent from the client to the engine (Closed)
|
Created:
4 years, 11 months ago by Khushal Modified:
4 years, 11 months ago CC:
anandc+watch-blimp_chromium.org, chromium-reviews, dtrainor+watch-blimp_chromium.org, kmarshall+watch-blimp_chromium.org, maniscalco+watch-blimp_chromium.org, marcinjb+watch-blimp_chromium.org, nyquist+watch-blimp_chromium.org, sriramsr+watch-blimp_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptionblimp: Fix input events sent from the client to the engine
The BlimpInputHandlerWrapper incorrectly sets the input events handles by
the compositor as consumed, causing duplicate events being sent to the
compositor and the renderer on the engine.
Don't send events handled or dropped by the compositor to the engine.
Committed: https://crrev.com/8807296b1722d6b1fbf5f4c50240d6aa9be8aff8
Cr-Commit-Position: refs/heads/master@{#369850}
Patch Set 1 #
Total comments: 4
Patch Set 2 : Addressed wez's comments. #Messages
Total messages: 19 (9 generated)
Description was changed from ========== blimp: Fix input events sent from the client to the engine The BlimpInputHandlerWrapper incorrectly sets the input events handles by the compositor as consumed, causing duplicate events being sent to the compositor and the renderer on the engine. Don't send events handled or dropped by the compositor to the engine. ========== to ========== blimp: Fix input events sent from the client to the engine The BlimpInputHandlerWrapper incorrectly sets the input events handles by the compositor as consumed, causing duplicate events being sent to the compositor and the renderer on the engine. Don't send events handled or dropped by the compositor to the engine. ==========
khushalsagar@chromium.org changed reviewers: + dtrainor@chromium.org
lgtm
The CQ bit was checked by khushalsagar@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1591083002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1591083002/1
wez@chromium.org changed reviewers: + wez@chromium.org
https://codereview.chromium.org/1591083002/diff/1/blimp/client/input/blimp_in... File blimp/client/input/blimp_input_handler_wrapper.cc (right): https://codereview.chromium.org/1591083002/diff/1/blimp/client/input/blimp_in... blimp/client/input/blimp_input_handler_wrapper.cc:47: bool consumed; Initialize consumed to false here! https://codereview.chromium.org/1591083002/diff/1/blimp/client/input/blimp_in... blimp/client/input/blimp_input_handler_wrapper.cc:55: consumed = false; nit: This case should have a break; even though it's the final one, to guard against later addition of a new case breaking things.
The CQ bit was unchecked by wez@chromium.org
wez, could you take another look? https://codereview.chromium.org/1591083002/diff/1/blimp/client/input/blimp_in... File blimp/client/input/blimp_input_handler_wrapper.cc (right): https://codereview.chromium.org/1591083002/diff/1/blimp/client/input/blimp_in... blimp/client/input/blimp_input_handler_wrapper.cc:47: bool consumed; On 2016/01/15 20:58:18, Wez wrote: > Initialize consumed to false here! Done. https://codereview.chromium.org/1591083002/diff/1/blimp/client/input/blimp_in... blimp/client/input/blimp_input_handler_wrapper.cc:55: consumed = false; On 2016/01/15 20:58:18, Wez wrote: > nit: This case should have a break; even though it's the final one, to guard > against later addition of a new case breaking things. Done.
lgtm
Thanks!
The CQ bit was checked by khushalsagar@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dtrainor@chromium.org Link to the patchset: https://codereview.chromium.org/1591083002/#ps20001 (title: "Addressed wez's comments.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1591083002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1591083002/20001
Message was sent while issue was closed.
Description was changed from ========== blimp: Fix input events sent from the client to the engine The BlimpInputHandlerWrapper incorrectly sets the input events handles by the compositor as consumed, causing duplicate events being sent to the compositor and the renderer on the engine. Don't send events handled or dropped by the compositor to the engine. ========== to ========== blimp: Fix input events sent from the client to the engine The BlimpInputHandlerWrapper incorrectly sets the input events handles by the compositor as consumed, causing duplicate events being sent to the compositor and the renderer on the engine. Don't send events handled or dropped by the compositor to the engine. ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== blimp: Fix input events sent from the client to the engine The BlimpInputHandlerWrapper incorrectly sets the input events handles by the compositor as consumed, causing duplicate events being sent to the compositor and the renderer on the engine. Don't send events handled or dropped by the compositor to the engine. ========== to ========== blimp: Fix input events sent from the client to the engine The BlimpInputHandlerWrapper incorrectly sets the input events handles by the compositor as consumed, causing duplicate events being sent to the compositor and the renderer on the engine. Don't send events handled or dropped by the compositor to the engine. Committed: https://crrev.com/8807296b1722d6b1fbf5f4c50240d6aa9be8aff8 Cr-Commit-Position: refs/heads/master@{#369850} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/8807296b1722d6b1fbf5f4c50240d6aa9be8aff8 Cr-Commit-Position: refs/heads/master@{#369850} |
