Update webrtc::DesktopCapturer clients to implement OnCaptureResult().

Update webrtc::DesktopCapturer clients to implement OnCaptureResult().

OnCaptureCompleted() method in webrtc::DesktopCapturer::Callback is
being replaced with OnCaptureResult(). The new method uses unique_ptr<>
to pass frame ownership, and also gets a Result code that allows to
distinguish between different types of capture errors.

BUG=webrtc:5950
Committed: https://crrev.com/546cbaa96e356990ae1bb801d720025ca814812c
Cr-Commit-Position: refs/heads/master@{#400494}

[Sergey Ulanov, 2016-06-09 18:39:11]

sergeyu@chromium.org changed reviewers:
+ wez@chromium.org

[Sergey Ulanov, 2016-06-09 18:40:11]

Description was changed from

==========
Update webrtc::DesktopCapturer clients to implement OnCaptureResult().

OnCaptureCompleted() method in webrtc::DesktopCapturer::Callback is
being replaced with OnCaptureResult(). The new method uses unique_ptr<>
to pass frame ownership, and also gets a Result code that allows to
distinguish between different types of capture errors.

BUG=webrtc:5950
==========

to

==========
Update webrtc::DesktopCapturer clients to implement OnCaptureResult().

OnCaptureCompleted() method in webrtc::DesktopCapturer::Callback is
being replaced with OnCaptureResult(). The new method uses unique_ptr<>
to pass frame ownership, and also gets a Result code that allows to
distinguish between different types of capture errors.

BUG=webrtc:5950
==========

[Sergey Ulanov, 2016-06-09 18:40:11]

sergeyu@chromium.org changed reviewers:
+ tsepez@chromium.org

[Sergey Ulanov, 2016-06-09 18:40:26]

+tsepez@ for remoting/host/chromoting_messages.h

[Tom Sepez, 2016-06-09 19:08:09]

messages LGTM

[Sergey Ulanov, 2016-06-14 18:12:18]

sergeyu@chromium.org changed reviewers:
+ jamiewalch@chromium.org

[Sergey Ulanov, 2016-06-14 18:12:18]

+jamiewalch@
Jamie, looks like Wez is busy, can you please take a look?

[Jamie, 2016-06-14 21:22:44]

lgtm

https://codereview.chromium.org/2050353002/diff/20001/remoting/host/chromotin...
File remoting/host/chromoting_messages.h (right):

https://codereview.chromium.org/2050353002/diff/20001/remoting/host/chromotin...
remoting/host/chromoting_messages.h:177:
webrtc::DesktopCapturer::Result::ERROR_PERMANENT)
Relying on ERROR_PERMANENT as being the max value is not very future proof.
Maybe add an explicit MAX_VALUE to the enum so that this doesn't need to change
if it's ever updated?

https://codereview.chromium.org/2050353002/diff/20001/remoting/host/desktop_s...
File remoting/host/desktop_session_proxy.cc (right):

https://codereview.chromium.org/2050353002/diff/20001/remoting/host/desktop_s...
remoting/host/desktop_session_proxy.cc:497: for (auto& rect :
serialized_frame.dirty_region) {
s/auto/const auto/?

https://codereview.chromium.org/2050353002/diff/20001/remoting/protocol/video...
File remoting/protocol/video_frame_pump.cc (right):

https://codereview.chromium.org/2050353002/diff/20001/remoting/protocol/video...
remoting/protocol/video_frame_pump.cc:124: // TODO(sergeyu): Handle
ERROR_PERMANENT result.
Should this and similar TODOs have tracking bugs?

[Sergey Ulanov, 2016-06-15 20:30:11]

https://codereview.chromium.org/2050353002/diff/20001/remoting/host/chromotin...
File remoting/host/chromoting_messages.h (right):

https://codereview.chromium.org/2050353002/diff/20001/remoting/host/chromotin...
remoting/host/chromoting_messages.h:177:
webrtc::DesktopCapturer::Result::ERROR_PERMANENT)
On 2016/06/14 21:22:44, Jamie wrote:
> Relying on ERROR_PERMANENT as being the max value is not very future proof.
> Maybe add an explicit MAX_VALUE to the enum so that this doesn't need to
change
> if it's ever updated?

Done.

https://codereview.chromium.org/2050353002/diff/20001/remoting/host/desktop_s...
File remoting/host/desktop_session_proxy.cc (right):

https://codereview.chromium.org/2050353002/diff/20001/remoting/host/desktop_s...
remoting/host/desktop_session_proxy.cc:497: for (auto& rect :
serialized_frame.dirty_region) {
On 2016/06/14 21:22:44, Jamie wrote:
> s/auto/const auto/?

Done.

https://codereview.chromium.org/2050353002/diff/20001/remoting/protocol/video...
File remoting/protocol/video_frame_pump.cc (right):

https://codereview.chromium.org/2050353002/diff/20001/remoting/protocol/video...
remoting/protocol/video_frame_pump.cc:124: // TODO(sergeyu): Handle
ERROR_PERMANENT result.
On 2016/06/14 21:22:44, Jamie wrote:
> Should this and similar TODOs have tracking bugs?

I removed the TODO in this file - it will be removed anyway in the future.
WebrtcFrameScheduler::OnCaptureResult() is the only place where this should be
fixed. I don't think it's worth opening a bug for it: it would be P3 anyway.
Also WebrtcFrameScheduler will need work and I'll probably address that TODO
there while making other changes.

[Sergey Ulanov, 2016-06-17 00:30:13]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-06-17 00:30:14]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
jamiewalch@chromium.org
Link to the patchset: https://codereview.chromium.org/2050353002/#ps40001
(title: "addressed feedback")

[commit-bot: I haz the power, 2016-06-17 00:31:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2050353002/40001

[commit-bot: I haz the power, 2016-06-17 00:42:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-17 00:42:39]

Try jobs failed on following builders:
  chromeos_x86-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)

[Sergey Ulanov, 2016-06-17 01:02:38]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-06-17 01:02:39]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
jamiewalch@chromium.org
Link to the patchset: https://codereview.chromium.org/2050353002/#ps60001
(title: "desktop_capture_device_unittest compilation")

[commit-bot: I haz the power, 2016-06-17 01:03:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2050353002/60001

[commit-bot: I haz the power, 2016-06-17 01:16:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-17 01:16:59]

Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[Wez, 2016-06-17 05:57:16]

https://codereview.chromium.org/2050353002/diff/1/chrome/browser/media/native...
File chrome/browser/media/native_desktop_media_list.cc (right):

https://codereview.chromium.org/2050353002/diff/1/chrome/browser/media/native...
chrome/browser/media/native_desktop_media_list.cc:233: current_frame_ =
std::move(frame);
You're changing the semantics such that the desktop-media list will show stale
content if a capture fails - is that desirable? Seems it really depends on why
the capture failed?

[Sergey Ulanov, 2016-06-17 14:52:08]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-06-17 14:52:09]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
jamiewalch@chromium.org
Link to the patchset: https://codereview.chromium.org/2050353002/#ps80001
(title: "fix tests")

[commit-bot: I haz the power, 2016-06-17 14:52:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2050353002/80001

[Sergey Ulanov, 2016-06-17 14:54:58]

The CQ bit was unchecked by sergeyu@chromium.org

[Sergey Ulanov, 2016-06-17 14:59:54]

https://codereview.chromium.org/2050353002/diff/1/chrome/browser/media/native...
File chrome/browser/media/native_desktop_media_list.cc (right):

https://codereview.chromium.org/2050353002/diff/1/chrome/browser/media/native...
chrome/browser/media/native_desktop_media_list.cc:233: current_frame_ =
std::move(frame);
On 2016/06/17 05:57:16, Wez wrote:
> You're changing the semantics such that the desktop-media list will show stale
> content if a capture fails - is that desirable? Seems it really depends on why
> the capture failed?

You are right. current_frame_ may actually be from a different window, so we
don't want to keep it here. Removed if.

[Sergey Ulanov, 2016-06-17 15:00:22]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-06-17 15:00:22]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
jamiewalch@chromium.org
Link to the patchset: https://codereview.chromium.org/2050353002/#ps100001
(title: ".")

[commit-bot: I haz the power, 2016-06-17 15:00:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2050353002/100001

[commit-bot: I haz the power, 2016-06-17 15:13:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-17 15:13:36]

Try jobs failed on following builders:
  chromeos_x86-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Sergey Ulanov, 2016-06-17 19:48:16]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-06-17 19:48:16]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
jamiewalch@chromium.org
Link to the patchset: https://codereview.chromium.org/2050353002/#ps120001
(title: "fix chromeos")

[commit-bot: I haz the power, 2016-06-17 19:48:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2050353002/120001

[commit-bot: I haz the power, 2016-06-17 20:39:44]

Description was changed from

==========
Update webrtc::DesktopCapturer clients to implement OnCaptureResult().

OnCaptureCompleted() method in webrtc::DesktopCapturer::Callback is
being replaced with OnCaptureResult(). The new method uses unique_ptr<>
to pass frame ownership, and also gets a Result code that allows to
distinguish between different types of capture errors.

BUG=webrtc:5950
==========

to

==========
Update webrtc::DesktopCapturer clients to implement OnCaptureResult().

OnCaptureCompleted() method in webrtc::DesktopCapturer::Callback is
being replaced with OnCaptureResult(). The new method uses unique_ptr<>
to pass frame ownership, and also gets a Result code that allows to
distinguish between different types of capture errors.

BUG=webrtc:5950
==========

[commit-bot: I haz the power, 2016-06-17 20:39:46]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-06-17 20:41:15]

Description was changed from

==========
Update webrtc::DesktopCapturer clients to implement OnCaptureResult().

OnCaptureCompleted() method in webrtc::DesktopCapturer::Callback is
being replaced with OnCaptureResult(). The new method uses unique_ptr<>
to pass frame ownership, and also gets a Result code that allows to
distinguish between different types of capture errors.

BUG=webrtc:5950
==========

to

==========
Update webrtc::DesktopCapturer clients to implement OnCaptureResult().

OnCaptureCompleted() method in webrtc::DesktopCapturer::Callback is
being replaced with OnCaptureResult(). The new method uses unique_ptr<>
to pass frame ownership, and also gets a Result code that allows to
distinguish between different types of capture errors.

BUG=webrtc:5950

Committed: https://crrev.com/546cbaa96e356990ae1bb801d720025ca814812c
Cr-Commit-Position: refs/heads/master@{#400494}
==========

[commit-bot: I haz the power, 2016-06-17 20:41:16]

Patchset 7 (id:??) landed as
https://crrev.com/546cbaa96e356990ae1bb801d720025ca814812c
Cr-Commit-Position: refs/heads/master@{#400494}