Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2298103002:
Disable Microsoft GS Wavetable Synth in Windows 10 Web MIDI backend (Closed)
|
Created:
4 years, 3 months ago by Shao-Chuan Lee Modified:
4 years, 3 months ago Reviewers:
Takashi Toyoshima CC:
chromium-reviews, feature-media-reviews_chromium.org, toyoshim+midi_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionDisable Microsoft GS Wavetable Synth in Windows 10 Web MIDI backend
Disabling the Microsoft software synthesizer due to security reasons.
See http://crbug.com/499279 for details.
R=toyoshim@chromium.org
Committed: https://crrev.com/d434fccf5140c2b1fb0005c185c56530b9c4fa11
Cr-Commit-Position: refs/heads/master@{#415590}
Patch Set 1 #
Total comments: 6
Patch Set 2 : using FALSE for boolean #Messages
Total messages: 15 (5 generated)
https://codereview.chromium.org/2298103002/diff/1/media/midi/midi_manager_win... File media/midi/midi_manager_winrt.cc (right): https://codereview.chromium.org/2298103002/diff/1/media/midi/midi_manager_win... media/midi/midi_manager_winrt.cc:520: // events in destructor. Should be StopWatcher() instead of destructor. May I fix it here? This seems too small to have a separate CL..
lgtm with one nit pick. https://codereview.chromium.org/2298103002/diff/1/media/midi/midi_manager_win... File media/midi/midi_manager_winrt.cc (right): https://codereview.chromium.org/2298103002/diff/1/media/midi/midi_manager_win... media/midi/midi_manager_winrt.cc:130: return result != 0; |result| is bool from the beginning, right? https://codereview.chromium.org/2298103002/diff/1/media/midi/midi_manager_win... media/midi/midi_manager_winrt.cc:520: // events in destructor. OK, let's fix this in a separate CL.
https://codereview.chromium.org/2298103002/diff/1/media/midi/midi_manager_win... File media/midi/midi_manager_winrt.cc (right): https://codereview.chromium.org/2298103002/diff/1/media/midi/midi_manager_win... media/midi/midi_manager_winrt.cc:130: return result != 0; On 2016/08/31 07:10:21, toyoshim wrote: > |result| is bool from the beginning, right? 'boolean' is actually unsigned char, casting it to bool with '!= 0' here.
The CQ bit was checked by shaochuan@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2298103002/diff/1/media/midi/midi_manager_win... File media/midi/midi_manager_winrt.cc (right): https://codereview.chromium.org/2298103002/diff/1/media/midi/midi_manager_win... media/midi/midi_manager_winrt.cc:130: return result != 0; Wow, it isn't standard bool. OK, so as we discussed offline, let's initialize it with FALSE so that we can easily notice that it's MS specific type, and probably, it's better in terms of removing implicit cast.
The CQ bit was unchecked by shaochuan@chromium.org
https://codereview.chromium.org/2298103002/diff/1/media/midi/midi_manager_win... File media/midi/midi_manager_winrt.cc (right): https://codereview.chromium.org/2298103002/diff/1/media/midi/midi_manager_win... media/midi/midi_manager_winrt.cc:130: return result != 0; On 2016/08/31 07:30:00, toyoshim wrote: > Wow, it isn't standard bool. OK, so as we discussed offline, let's initialize it > with FALSE so that we can easily notice that it's MS specific type, and > probably, it's better in terms of removing implicit cast. Done. https://msdn.microsoft.com/en-us/library/windows/desktop/aa366740(v=vs.85).aspx
The CQ bit was checked by shaochuan@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from toyoshim@chromium.org Link to the patchset: https://codereview.chromium.org/2298103002/#ps20001 (title: "using FALSE for boolean")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
still lgtm
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Disable Microsoft GS Wavetable Synth in Windows 10 Web MIDI backend Disabling the Microsoft software synthesizer due to security reasons. See http://crbug.com/499279 for details. R=toyoshim@chromium.org ========== to ========== Disable Microsoft GS Wavetable Synth in Windows 10 Web MIDI backend Disabling the Microsoft software synthesizer due to security reasons. See http://crbug.com/499279 for details. R=toyoshim@chromium.org Committed: https://crrev.com/d434fccf5140c2b1fb0005c185c56530b9c4fa11 Cr-Commit-Position: refs/heads/master@{#415590} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/d434fccf5140c2b1fb0005c185c56530b9c4fa11 Cr-Commit-Position: refs/heads/master@{#415590} |
