Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1880673002:
Pass PointerType from Mojo EventPtr to WebInputEvent. (Closed)
|
Created:
4 years, 8 months ago by mustaq Modified:
4 years, 8 months ago Reviewers:
sky CC:
chromium-reviews, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, darin (slow to review), ben+mojo_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionPass PointerType from Mojo EventPtr to WebInputEvent.
BUG=593375
Committed: https://crrev.com/bbf25032a5f5849240b3e17721a0a4edb0de9e81
Cr-Commit-Position: refs/heads/master@{#387102}
Patch Set 1 #
Total comments: 5
Patch Set 2 : #Patch Set 3 : #Messages
Total messages: 18 (6 generated)
mustaq@chromium.org changed reviewers: + sky@chromium.org
ptal
https://codereview.chromium.org/1880673002/diff/1/mojo/converters/blink/blink... File mojo/converters/blink/blink_input_events_type_converters.cc (right): https://codereview.chromium.org/1880673002/diff/1/mojo/converters/blink/blink... mojo/converters/blink/blink_input_events_type_converters.cc:67: mus::mojom::PointerKind pointerKind) { pointer_kind https://codereview.chromium.org/1880673002/diff/1/mojo/converters/blink/blink... mojo/converters/blink/blink_input_events_type_converters.cc:75: default: Can you get rid of the default statement? That way there is a compile error if a new one is added.
ptal https://codereview.chromium.org/1880673002/diff/1/mojo/converters/blink/blink... File mojo/converters/blink/blink_input_events_type_converters.cc (right): https://codereview.chromium.org/1880673002/diff/1/mojo/converters/blink/blink... mojo/converters/blink/blink_input_events_type_converters.cc:67: mus::mojom::PointerKind pointerKind) { On 2016/04/12 20:38:01, sky wrote: > pointer_kind Done. https://codereview.chromium.org/1880673002/diff/1/mojo/converters/blink/blink... mojo/converters/blink/blink_input_events_type_converters.cc:75: default: On 2016/04/12 20:38:01, sky wrote: > Can you get rid of the default statement? That way there is a compile error if a > new one is added. Done.
LGTM
The CQ bit was checked by mustaq@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1880673002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1880673002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win8_chromium_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)
https://codereview.chromium.org/1880673002/diff/1/mojo/converters/blink/blink... File mojo/converters/blink/blink_input_events_type_converters.cc (right): https://codereview.chromium.org/1880673002/diff/1/mojo/converters/blink/blink... mojo/converters/blink/blink_input_events_type_converters.cc:75: default: On 2016/04/13 14:09:04, mustaq wrote: > On 2016/04/12 20:38:01, sky wrote: > > Can you get rid of the default statement? That way there is a compile error if > a > > new one is added. > > Done. Yikes! The win8_chromium_ng compiler isn't smart enough to like it: https://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng... Any workaround? Or bring the default case back?
What about a return value after the switch? On Wed, Apr 13, 2016 at 11:35 AM, <mustaq@chromium.org> wrote: > > https://codereview.chromium.org/1880673002/diff/1/mojo/converters/blink/blink... > File mojo/converters/blink/blink_input_events_type_converters.cc > (right): > > https://codereview.chromium.org/1880673002/diff/1/mojo/converters/blink/blink... > mojo/converters/blink/blink_input_events_type_converters.cc:75: default: > On 2016/04/13 14:09:04, mustaq wrote: >> On 2016/04/12 20:38:01, sky wrote: >> > Can you get rid of the default statement? That way there is a > compile error if >> a >> > new one is added. >> >> Done. > > Yikes! The win8_chromium_ng compiler isn't smart enough to like it: > > https://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng... > > Any workaround? Or bring the default case back? > > https://codereview.chromium.org/1880673002/ -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On 2016/04/13 20:38:07, sky wrote: > What about a return value after the switch? > > On Wed, Apr 13, 2016 at 11:35 AM, <mailto:mustaq@chromium.org> wrote: > > > > Yikes! The win8_chromium_ng compiler isn't smart enough to like it: > > Done. I thought muting the win8 compiler through a flag would be a better option, but not sure how to configure the bot this way. I will commit now, thanks.
The CQ bit was checked by mustaq@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org Link to the patchset: https://codereview.chromium.org/1880673002/#ps40001 (title: " ")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1880673002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1880673002/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Pass PointerType from Mojo EventPtr to WebInputEvent. BUG=593375 ========== to ========== Pass PointerType from Mojo EventPtr to WebInputEvent. BUG=593375 Committed: https://crrev.com/bbf25032a5f5849240b3e17721a0a4edb0de9e81 Cr-Commit-Position: refs/heads/master@{#387102} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/bbf25032a5f5849240b3e17721a0a4edb0de9e81 Cr-Commit-Position: refs/heads/master@{#387102} |
