Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1314483003:
Improve VideoCaptureImpl unittest. (Closed)
|
Created:
5 years, 4 months ago by msu.koo Modified:
5 years, 3 months ago CC:
chromium-reviews, mlamouri+watch-content_chromium.org, posciak+watch_chromium.org, jam, mcasas+watch_chromium.org, feature-media-reviews_chromium.org, darin-cc_chromium.org, mkwst+moarreviews-renderer_chromium.org, wjia+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionImprove VideoCaptureImpl unittest.
This patch includes 2 improvements on VideoCaptureImpl unittest.
- Added "BufferReceived" unittest to check when buffer is received.
- Added "BufferReceivedAfterStop" unittest to check when buffer is received after stop.
BUG=522145
Committed: https://crrev.com/393d2fca30fc7340abd7257f31b2d2884266cd1d
Cr-Commit-Position: refs/heads/master@{#348337}
Patch Set 1 #Patch Set 2 : #
Total comments: 6
Patch Set 3 : #
Total comments: 10
Patch Set 4 : #
Total comments: 16
Patch Set 5 : #
Total comments: 5
Patch Set 6 : #
Total comments: 1
Patch Set 7 : #Patch Set 8 : #Messages
Total messages: 35 (12 generated)
msu.koo@samsung.com changed reviewers: + ajose@chromium.org, mcasas@chromium.org
Please take a look, and let me know your comments. :)
Looks good! I think right now only the mailbox_holder.mailbox.IsZero() path of OnBufferReceieved is being tested, consider adding a test for the else path too? https://codereview.chromium.org/1314483003/diff/20001/content/renderer/media/... File content/renderer/media/video_capture_impl_unittest.cc (right): https://codereview.chromium.org/1314483003/diff/20001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:114: int buffer_received_; s/buffer_received/buffers_received/ ? Also consider making this private and adding a getter. https://codereview.chromium.org/1314483003/diff/20001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:328: } EXPECT_EQ(this->video_capture_impl_->buffer_received_, 1); ?
https://codereview.chromium.org/1314483003/diff/20001/content/renderer/media/... File content/renderer/media/video_capture_impl_unittest.cc (right): https://codereview.chromium.org/1314483003/diff/20001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:38: class MockVideoCaptureImpl : public VideoCaptureImpl { This is odd: A test class that Mocks the object to test? :) VideoCaptureImpl is basically a billboard routing messages between an impl of VideoCaptureMessageFilter::Delegate and controlled from VideoCaptureImplManager via a few methods StartCapture() etc. All that maintaining a |state_|. I think this whole test should be refactored into setting expectations on mocked functions, i.e. create a series of MOCK_METHODX()s to be acting as a VideoCaptureStateUpdateCB, VideoCaptureDeliverFrameCB, etc, plug them in via StartCapture() and company, then per test, set EXPECT()s and ping the class by exercising the various IPC messages, possibly observing |state_| changes. WDYT?
Thank you for your valuable comments. I addressed your comments and left my opinions. Please take a look. :) https://codereview.chromium.org/1314483003/diff/20001/content/renderer/media/... File content/renderer/media/video_capture_impl_unittest.cc (right): https://codereview.chromium.org/1314483003/diff/20001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:38: class MockVideoCaptureImpl : public VideoCaptureImpl { On 2015/08/25 00:14:58, mcasas wrote: > This is odd: A test class that Mocks the object to test? :) > > VideoCaptureImpl is basically a billboard routing messages > between an impl of VideoCaptureMessageFilter::Delegate > and controlled from VideoCaptureImplManager via a few > methods StartCapture() etc. All that maintaining a |state_|. > > I think this whole test should be refactored into setting > expectations on mocked functions, i.e. create a series of > MOCK_METHODX()s to be acting as a > VideoCaptureStateUpdateCB, VideoCaptureDeliverFrameCB, > etc, plug them in via StartCapture() and company, then > per test, set EXPECT()s and ping the class by exercising > the various IPC messages, possibly observing |state_| > changes. WDYT? Thank you for your comments. I agree with your opinion, and I think it's better to address your comments as different patch-set because this patch-set was intended to test "bufferAdded, and bufferAddedAfterStop" https://codereview.chromium.org/1314483003/diff/20001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:114: int buffer_received_; On 2015/08/24 17:46:31, ajose wrote: > s/buffer_received/buffers_received/ ? > Also consider making this private and adding a getter. Done. https://codereview.chromium.org/1314483003/diff/20001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:328: } On 2015/08/24 17:46:31, ajose wrote: > EXPECT_EQ(this->video_capture_impl_->buffer_received_, 1); ? I think this line is not required on this unittest because buffer-received will be handled outside of VideoCaptureImpl except the case when the capturing is stopped(This case is tested in "BufferReceivedAfterStop".)
https://codereview.chromium.org/1314483003/diff/40001/content/renderer/media/... File content/renderer/media/video_capture_impl_unittest.cc (right): https://codereview.chromium.org/1314483003/diff/40001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:110: const int GetReceivedBufferCounts() { Fits in one line, and it's the method that should be const (not the return value). int GetReceivedBufferCounts() const { return buffers_received_; } https://codereview.chromium.org/1314483003/diff/40001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:116: int buffers_received_; s/int/size_t/ ? https://codereview.chromium.org/1314483003/diff/40001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:176: gfx::Rect(size.width(), size.height()), mailbox_holder); gfx::Rect(size) [1] [1] https://code.google.com/p/chromium/codesearch#chromium/src/ui/gfx/geometry/re... https://codereview.chromium.org/1314483003/diff/40001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:333: TEST_F(VideoCaptureImplTest, ARGBBufferReceived) { Consider making this test and the previous one parameterised. See for instance [1], TEST_P is your friend. [1] https://code.google.com/p/chromium/codesearch#chromium/src/media/capture/vide... https://codereview.chromium.org/1314483003/diff/40001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:340: size_t i420_frame_size = media::VideoFrame::AllocationSize( s/i420_frame_size/argb_frame_size/ nit: make it const.
Thank you for your review. You comment are addressed on this patch. PTAL https://codereview.chromium.org/1314483003/diff/40001/content/renderer/media/... File content/renderer/media/video_capture_impl_unittest.cc (right): https://codereview.chromium.org/1314483003/diff/40001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:110: const int GetReceivedBufferCounts() { On 2015/08/25 16:43:45, mcasas wrote: > Fits in one line, and it's the method that should be const > (not the return value). > > int GetReceivedBufferCounts() const { return buffers_received_; } Done. https://codereview.chromium.org/1314483003/diff/40001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:116: int buffers_received_; On 2015/08/25 16:43:45, mcasas wrote: > s/int/size_t/ ? Done. https://codereview.chromium.org/1314483003/diff/40001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:176: gfx::Rect(size.width(), size.height()), mailbox_holder); On 2015/08/25 16:43:46, mcasas wrote: > gfx::Rect(size) [1] > > [1] > https://code.google.com/p/chromium/codesearch#chromium/src/ui/gfx/geometry/re... Done. https://codereview.chromium.org/1314483003/diff/40001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:333: TEST_F(VideoCaptureImplTest, ARGBBufferReceived) { On 2015/08/25 16:43:45, mcasas wrote: > Consider making this test and the previous one parameterised. > See for instance [1], TEST_P is your friend. > > [1] > https://code.google.com/p/chromium/codesearch#chromium/src/media/capture/vide... > Done. https://codereview.chromium.org/1314483003/diff/40001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:340: size_t i420_frame_size = media::VideoFrame::AllocationSize( On 2015/08/25 16:43:45, mcasas wrote: > s/i420_frame_size/argb_frame_size/ > nit: make it const. Done.
Could you take a look on this patchset?
Almost there. A few comments. https://codereview.chromium.org/1314483003/diff/60001/content/renderer/media/... File content/renderer/media/video_capture_impl_unittest.cc (right): https://codereview.chromium.org/1314483003/diff/60001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:53: }; Prefer struct Bla { // ... } kBlas[] = { // ... }; for one-off struts, so is easier to see that it's only used in one place. https://codereview.chromium.org/1314483003/diff/60001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:56: BufferReceivedTestArg(0, media::PIXEL_FORMAT_I420, |buffer_id| and |coded_size| are common among the entries, so set them explicitly in the test itself. And I'd call the struct BufferFormat, simpler :) https://codereview.chromium.org/1314483003/diff/60001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:136: const media::VideoCaptureParams& capture_params() const { unused now? (both method and member variable). https://codereview.chromium.org/1314483003/diff/60001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:140: int GetReceivedBufferCounts() const { return received_buffer_counts_; } s/GetReceivedBufferCounts()/received_buffer_count()/ [1] http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Function_Names https://codereview.chromium.org/1314483003/diff/60001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:144: int received_buffer_counts_; s/received_buffer_counts_/received_buffer_count_/ https://codereview.chromium.org/1314483003/diff/60001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:313: testing::ValuesIn(kBufferFormats)); You can also bring |kBufferFormats| here . https://codereview.chromium.org/1314483003/diff/60001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:322: size_t i420_frame_size = media::VideoFrame::AllocationSize( const
Patchset #5 (id:80001) has been deleted
PTAL :) https://codereview.chromium.org/1314483003/diff/60001/content/renderer/media/... File content/renderer/media/video_capture_impl_unittest.cc (right): https://codereview.chromium.org/1314483003/diff/60001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:53: }; On 2015/08/31 17:33:26, mcasas wrote: > Prefer > struct Bla { > // ... > } kBlas[] = { > // ... > }; > > for one-off struts, so is easier to see that it's only > used in one place. Moved |kBufferFormats| to INSTANTIATE_TEST_CASE_P as you commented below. https://codereview.chromium.org/1314483003/diff/60001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:56: BufferReceivedTestArg(0, media::PIXEL_FORMAT_I420, I basically agree with your opinion, but these variables may be possibly reused when expending this unittest. I think it's not bad to leave them as it is, WDYT? On 2015/08/31 17:33:26, mcasas wrote: > |buffer_id| and |coded_size| are common among > the entries, so set them explicitly in the > test itself. > > And I'd call the struct BufferFormat, simpler :) https://codereview.chromium.org/1314483003/diff/60001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:136: const media::VideoCaptureParams& capture_params() const { On 2015/08/31 17:33:26, mcasas wrote: > unused now? (both method and member variable). Done. https://codereview.chromium.org/1314483003/diff/60001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:140: int GetReceivedBufferCounts() const { return received_buffer_counts_; } On 2015/08/31 17:33:26, mcasas wrote: > s/GetReceivedBufferCounts()/received_buffer_count()/ > > [1] > http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Function_Names Done. https://codereview.chromium.org/1314483003/diff/60001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:144: int received_buffer_counts_; On 2015/08/31 17:33:26, mcasas wrote: > s/received_buffer_counts_/received_buffer_count_/ Done. https://codereview.chromium.org/1314483003/diff/60001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:313: testing::ValuesIn(kBufferFormats)); On 2015/08/31 17:33:26, mcasas wrote: > You can also bring |kBufferFormats| here . Done. https://codereview.chromium.org/1314483003/diff/60001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:322: size_t i420_frame_size = media::VideoFrame::AllocationSize( On 2015/08/31 17:33:26, mcasas wrote: > const Done.
LGTM % comments below https://codereview.chromium.org/1314483003/diff/60001/content/renderer/media/... File content/renderer/media/video_capture_impl_unittest.cc (right): https://codereview.chromium.org/1314483003/diff/60001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:56: BufferReceivedTestArg(0, media::PIXEL_FORMAT_I420, On 2015/09/01 05:25:01, esnada wrote: > I basically agree with your opinion, but these variables may be possibly reused > when expending this unittest. > I think it's not bad to leave them as it is, WDYT? > > On 2015/08/31 17:33:26, mcasas wrote: > > |buffer_id| and |coded_size| are common among > > the entries, so set them explicitly in the > > test itself. > > > > And I'd call the struct BufferFormat, simpler :) > Ah, we'll extend it when the moment comes, no dead code and no duplicated code in Chromium! https://codereview.chromium.org/1314483003/diff/100001/content/renderer/media... File content/renderer/media/video_capture_impl_unittest.cc (right): https://codereview.chromium.org/1314483003/diff/100001/content/renderer/media... content/renderer/media/video_capture_impl_unittest.cc:96: void DevicePauseCapture(int device_id) {} Suggest mocking these two fellas since they do nothing and have no scoped_ptr<> args (keep the comment): MOCK_METHOD3(DeviceStartCapture, void(int device_id, media::VideoCaptureSessionId session_id, const media::VideoCaptureParams& params)); MOCK_METHOD1(DevicePauseCapture, void(int device_id)); So we also get alarmed/warned if these methods get pinged when they shouldn't, since non-explicit EXPECTs mean .Times(0). As a side note I guess it should be done with all these methods, i.e. add on ctor sth like ON_CALL(bla)..WillByDefault(Invoke(foo())); allowing for finer method call control. https://codereview.chromium.org/1314483003/diff/100001/content/renderer/media... content/renderer/media/video_capture_impl_unittest.cc:307: testing::ValuesIn(kBufferFormats)); I meant defining it inside the INSTANTIATE_... so we don't create unnecessary symbols: INSTANTIATE_TEST_CASE_P( I420AndARGB, VideoCaptureImplTest, testing::ValuesIn( BufferReceivedTestArg( media::PIXEL_FORMAT_I420, media::VIDEO_CAPTURE_PIXEL_FORMAT_I420, gpu::MailboxHolder()), BufferReceivedTestArg( media::PIXEL_FORMAT_ARGB, media::VIDEO_CAPTURE_PIXEL_FORMAT_ARGB, gpu::MailboxHolder(gpu::Mailbox::Generate(), 0, 0)))); (Took the liberty of removing |buffer_id|and |coded_size|). But on second thoughts and after searching the code base, this doesn't seem popular at all, so please revert it to defining |kBufferFormats| after the struct, apologies! (And make it |static|).
Thank you for your comments. Hope this round will be final :) https://codereview.chromium.org/1314483003/diff/60001/content/renderer/media/... File content/renderer/media/video_capture_impl_unittest.cc (right): https://codereview.chromium.org/1314483003/diff/60001/content/renderer/media/... content/renderer/media/video_capture_impl_unittest.cc:56: BufferReceivedTestArg(0, media::PIXEL_FORMAT_I420, On 2015/09/01 18:09:53, mcasas wrote: > On 2015/09/01 05:25:01, esnada wrote: > > I basically agree with your opinion, but these variables may be possibly > reused > > when expending this unittest. > > I think it's not bad to leave them as it is, WDYT? > > > > On 2015/08/31 17:33:26, mcasas wrote: > > > |buffer_id| and |coded_size| are common among > > > the entries, so set them explicitly in the > > > test itself. > > > > > > And I'd call the struct BufferFormat, simpler :) > > > > Ah, we'll extend it when the moment comes, > no dead code and no duplicated code in Chromium! Done. https://codereview.chromium.org/1314483003/diff/100001/content/renderer/media... File content/renderer/media/video_capture_impl_unittest.cc (right): https://codereview.chromium.org/1314483003/diff/100001/content/renderer/media... content/renderer/media/video_capture_impl_unittest.cc:96: void DevicePauseCapture(int device_id) {} It's better approach but I think changing these functions is not under this patch's scope. How about handling it separately? On 2015/09/01 18:09:53, mcasas wrote: > Suggest mocking these two fellas since they do nothing > and have no scoped_ptr<> args (keep the comment): > > MOCK_METHOD3(DeviceStartCapture, > void(int device_id, > media::VideoCaptureSessionId session_id, > const media::VideoCaptureParams& params)); > MOCK_METHOD1(DevicePauseCapture, void(int device_id)); > > So we also get alarmed/warned if these > methods get pinged when they shouldn't, > since non-explicit EXPECTs mean .Times(0). > > As a side note I guess it should be done with > all these methods, i.e. add on ctor sth like > > ON_CALL(bla)..WillByDefault(Invoke(foo())); > > allowing for finer method call control. https://codereview.chromium.org/1314483003/diff/100001/content/renderer/media... content/renderer/media/video_capture_impl_unittest.cc:307: testing::ValuesIn(kBufferFormats)); On 2015/09/01 18:09:53, mcasas wrote: > I meant defining it inside the INSTANTIATE_... > so we don't create unnecessary symbols: > > INSTANTIATE_TEST_CASE_P( > I420AndARGB, > VideoCaptureImplTest, > testing::ValuesIn( > BufferReceivedTestArg( > media::PIXEL_FORMAT_I420, > media::VIDEO_CAPTURE_PIXEL_FORMAT_I420, > gpu::MailboxHolder()), > BufferReceivedTestArg( > media::PIXEL_FORMAT_ARGB, > media::VIDEO_CAPTURE_PIXEL_FORMAT_ARGB, > gpu::MailboxHolder(gpu::Mailbox::Generate(), 0, 0)))); > > (Took the liberty of removing |buffer_id|and |coded_size|). > > But on second thoughts and after searching the > code base, this doesn't seem popular at all, so > please revert it to defining |kBufferFormats| after > the struct, apologies! (And make it |static|). Done.
still LGTM (see comments below). https://codereview.chromium.org/1314483003/diff/100001/content/renderer/media... File content/renderer/media/video_capture_impl_unittest.cc (right): https://codereview.chromium.org/1314483003/diff/100001/content/renderer/media... content/renderer/media/video_capture_impl_unittest.cc:96: void DevicePauseCapture(int device_id) {} On 2015/09/02 08:38:10, esnada wrote: > It's better approach but I think changing these functions is not under this > patch's scope. How about handling it separately? > > On 2015/09/01 18:09:53, mcasas wrote: > > Suggest mocking these two fellas since they do nothing > > and have no scoped_ptr<> args (keep the comment): > > > > MOCK_METHOD3(DeviceStartCapture, > > void(int device_id, > > media::VideoCaptureSessionId session_id, > > const media::VideoCaptureParams& params)); > > MOCK_METHOD1(DevicePauseCapture, void(int device_id)); > > > > So we also get alarmed/warned if these > > methods get pinged when they shouldn't, > > since non-explicit EXPECTs mean .Times(0). > > > > As a side note I guess it should be done with > > all these methods, i.e. add on ctor sth like > > > > ON_CALL(bla)..WillByDefault(Invoke(foo())); > > > > allowing for finer method call control. > SGTM, you can do that in a CL afterwards but usually we add a TODO() here. https://codereview.chromium.org/1314483003/diff/120001/content/renderer/media... File content/renderer/media/video_capture_impl_unittest.cc (right): https://codereview.chromium.org/1314483003/diff/120001/content/renderer/media... content/renderer/media/video_capture_impl_unittest.cc:52: media::VIDEO_CAPTURE_PIXEL_FORMAT_I420, Heh incidentally there has been another CL unifying VideoPixelFormat and VideoCapturePixelFormat, so you'll need to rebase and then you'll only need one of |pixel_format| and |capture_pixel_format|.
On 2015/09/02 16:51:26, mcasas wrote: > still LGTM (see comments below). > > https://codereview.chromium.org/1314483003/diff/100001/content/renderer/media... > File content/renderer/media/video_capture_impl_unittest.cc (right): > > https://codereview.chromium.org/1314483003/diff/100001/content/renderer/media... > content/renderer/media/video_capture_impl_unittest.cc:96: void > DevicePauseCapture(int device_id) {} > On 2015/09/02 08:38:10, esnada wrote: > > It's better approach but I think changing these functions is not under this > > patch's scope. How about handling it separately? > > > > On 2015/09/01 18:09:53, mcasas wrote: > > > Suggest mocking these two fellas since they do nothing > > > and have no scoped_ptr<> args (keep the comment): > > > > > > MOCK_METHOD3(DeviceStartCapture, > > > void(int device_id, > > > media::VideoCaptureSessionId session_id, > > > const media::VideoCaptureParams& params)); > > > MOCK_METHOD1(DevicePauseCapture, void(int device_id)); > > > > > > So we also get alarmed/warned if these > > > methods get pinged when they shouldn't, > > > since non-explicit EXPECTs mean .Times(0). > > > > > > As a side note I guess it should be done with > > > all these methods, i.e. add on ctor sth like > > > > > > ON_CALL(bla)..WillByDefault(Invoke(foo())); > > > > > > allowing for finer method call control. > > > > SGTM, you can do that in a CL afterwards > but usually we add a TODO() here. > > https://codereview.chromium.org/1314483003/diff/120001/content/renderer/media... > File content/renderer/media/video_capture_impl_unittest.cc (right): > > https://codereview.chromium.org/1314483003/diff/120001/content/renderer/media... > content/renderer/media/video_capture_impl_unittest.cc:52: > media::VIDEO_CAPTURE_PIXEL_FORMAT_I420, > Heh incidentally there has been another CL > unifying VideoPixelFormat and VideoCapturePixelFormat, > so you'll need to rebase and then you'll only need > one of |pixel_format| and |capture_pixel_format|. lgtm % rebase, thanks for your work!
The CQ bit was checked by msu.koo@samsung.com
The CQ bit was unchecked by msu.koo@samsung.com
The CQ bit was checked by msu.koo@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1314483003/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1314483003/120001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
The CQ bit was checked by msu.koo@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1314483003/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1314483003/140001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by msu.koo@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1314483003/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1314483003/160001
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 msu.koo@samsung.com
The patchset sent to the CQ was uploaded after l-g-t-m from mcasas@chromium.org, ajose@chromium.org Link to the patchset: https://codereview.chromium.org/1314483003/#ps160001 (title: " ")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1314483003/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1314483003/160001
Message was sent while issue was closed.
Committed patchset #8 (id:160001)
Message was sent while issue was closed.
Patchset 8 (id:??) landed as https://crrev.com/393d2fca30fc7340abd7257f31b2d2884266cd1d Cr-Commit-Position: refs/heads/master@{#348337}
Message was sent while issue was closed.
Patchset 8 (id:??) landed as https://crrev.com/393d2fca30fc7340abd7257f31b2d2884266cd1d Cr-Commit-Position: refs/heads/master@{#348337} |
