Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2812623004:
Check that the source is valid before accessing it in MSVT::GetSettings. (Closed)
|
Created:
3 years, 8 months ago by Guido Urdaneta Modified:
3 years, 8 months ago Reviewers:
mcasas CC:
chromium-reviews, mlamouri+watch-content_chromium.org, imcheng+watch_chromium.org, feature-media-reviews_chromium.org, posciak+watch_chromium.org, chfremer+watch_chromium.org, jam, avayvod+watch_chromium.org, darin-cc_chromium.org, jasonroberts+watch_google.com, xjz+watch_chromium.org, isheriff+watch_chromium.org, miu+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionCheck that the source is valid before accessing it in MSVT::GetSettings.
Drive-by: DCHECK that the function is called on the correct thread.
BUG=709736
Review-Url: https://codereview.chromium.org/2812623004
Cr-Commit-Position: refs/heads/master@{#463703}
Committed: https://chromium.googlesource.com/chromium/src/+/0cbb4e27d69412dd0f01f2c6583f2b00231c0a38
Patch Set 1 #
Total comments: 2
Messages
Total messages: 17 (11 generated)
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Description was changed from ========== Check that the source is valid before accessing it in MSVT::GetSettings. BUG=709736 ========== to ========== Check that the source is valid before accessing it in MSVT::GetSettings. Drive-by: DCHECK that the function is called on the correct thread. BUG=709736 ==========
The CQ bit was checked by guidou@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...
guidou@chromium.org changed reviewers: + mcasas@chromium.org
Hi, PTAL
lgtm https://codereview.chromium.org/2812623004/diff/40001/content/renderer/media/... File content/renderer/media/media_stream_video_track.cc (right): https://codereview.chromium.org/2812623004/diff/40001/content/renderer/media/... content/renderer/media/media_stream_video_track.cc:397: } nit: maybe nonsense, but what about if (width_) settings.width = width_; if (height_) settings.height = height_;
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2812623004/diff/40001/content/renderer/media/... File content/renderer/media/media_stream_video_track.cc (right): https://codereview.chromium.org/2812623004/diff/40001/content/renderer/media/... content/renderer/media/media_stream_video_track.cc:397: } On 2017/04/11 17:37:00, mcasas wrote: > nit: maybe nonsense, but what about > > if (width_) > settings.width = width_; > if (height_) > settings.height = height_; I think the original intent is to return values only if both are set. Land as is, since the main purpose of this CL is to fix the bug.
The CQ bit was checked by guidou@chromium.org
s/Land/Landing
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1491936053573310,
"parent_rev": "2224b661e60f9b57f117ef437b10a73ba2d19a95", "commit_rev":
"0cbb4e27d69412dd0f01f2c6583f2b00231c0a38"}
Message was sent while issue was closed.
Description was changed from ========== Check that the source is valid before accessing it in MSVT::GetSettings. Drive-by: DCHECK that the function is called on the correct thread. BUG=709736 ========== to ========== Check that the source is valid before accessing it in MSVT::GetSettings. Drive-by: DCHECK that the function is called on the correct thread. BUG=709736 Review-Url: https://codereview.chromium.org/2812623004 Cr-Commit-Position: refs/heads/master@{#463703} Committed: https://chromium.googlesource.com/chromium/src/+/0cbb4e27d69412dd0f01f2c6583f... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:40001) as https://chromium.googlesource.com/chromium/src/+/0cbb4e27d69412dd0f01f2c6583f... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
