Update XServerPixelBuffer to handle shmget() errors properly.

Update XServerPixelBuffer to handle shmget() errors properly.

When screen/window capturers fail to use shared memory, they are
supposed to fall back to XGetImage(). It's slower, but should still
allow to capture desktop content. This wasn't implemented correclty in
XScreenPixelBuffer - it was failing completely when shmget() returns an
error.

BUG=705146
Review-Url: https://codereview.webrtc.org/2775793003
Cr-Commit-Position: refs/heads/master@{#17401}
Committed: https://chromium.googlesource.com/external/webrtc/+/858b0f4703e043e1b90b95c1351aa8663c11fd7c

[Sergey Ulanov, 2017-03-24 23:27:01]

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

[Hzj_jie, 2017-03-25 00:30:31]

https://codereview.webrtc.org/2775793003/diff/1/webrtc/modules/desktop_captur...
File webrtc/modules/desktop_capture/x11/x_server_pixel_buffer.cc (right):

https://codereview.webrtc.org/2775793003/diff/1/webrtc/modules/desktop_captur...
webrtc/modules/desktop_capture/x11/x_server_pixel_buffer.cc:158:
shm_segment_info_ = nullptr;
It looks like in Release() function, there are bunch of shm_segment_info_
cleanup statements, instead of just delete. And AFAICT, these fields are
initialized even when using_shm == false. I would suggest to abstract the
shm_segment_info_ cleanup logic into a standalone function and call it both here
and in Release().

[Sergey Ulanov, 2017-03-27 19:23:06]

https://codereview.webrtc.org/2775793003/diff/1/webrtc/modules/desktop_captur...
File webrtc/modules/desktop_capture/x11/x_server_pixel_buffer.cc (right):

https://codereview.webrtc.org/2775793003/diff/1/webrtc/modules/desktop_captur...
webrtc/modules/desktop_capture/x11/x_server_pixel_buffer.cc:158:
shm_segment_info_ = nullptr;
On 2017/03/25 00:30:30, Hzj_jie wrote:
> It looks like in Release() function, there are bunch of shm_segment_info_
> cleanup statements, instead of just delete. And AFAICT, these fields are
> initialized even when using_shm == false. I would suggest to abstract the
> shm_segment_info_ cleanup logic into a standalone function and call it both
here
> and in Release().

Yes, good point. Fixed.
Also update this code to set shm_segment_info_->shmaddr and x_image_->data to
nullptr in unitialized state instead of -1 (shmat() returns (void*)-1 to
indicate error).

[Hzj_jie, 2017-03-27 19:27:53]

On 2017/03/27 19:23:06, Sergey Ulanov wrote:
>
https://codereview.webrtc.org/2775793003/diff/1/webrtc/modules/desktop_captur...
> File webrtc/modules/desktop_capture/x11/x_server_pixel_buffer.cc (right):
> 
>
https://codereview.webrtc.org/2775793003/diff/1/webrtc/modules/desktop_captur...
> webrtc/modules/desktop_capture/x11/x_server_pixel_buffer.cc:158:
> shm_segment_info_ = nullptr;
> On 2017/03/25 00:30:30, Hzj_jie wrote:
> > It looks like in Release() function, there are bunch of shm_segment_info_
> > cleanup statements, instead of just delete. And AFAICT, these fields are
> > initialized even when using_shm == false. I would suggest to abstract the
> > shm_segment_info_ cleanup logic into a standalone function and call it both
> here
> > and in Release().
> 
> Yes, good point. Fixed.
> Also update this code to set shm_segment_info_->shmaddr and x_image_->data to
> nullptr in unitialized state instead of -1 (shmat() returns (void*)-1 to
> indicate error).

lgtm.

[Sergey Ulanov, 2017-03-27 20:03:48]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2017-03-27 20:03:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-03-27 20:47:32]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490645028980960,
"parent_rev": "46f901537a72fcfb4e95af1042b3ea5687e9b055", "commit_rev":
"858b0f4703e043e1b90b95c1351aa8663c11fd7c"}

[commit-bot: I haz the power, 2017-03-27 20:47:35]

Description was changed from

==========
Update XServerPixelBuffer to handle shmget() errors properly.

When screen/window capturers fail to use shared memory, they are
supposed to fall back to XGetImage(). It's slower, but should still
allow to capture desktop content. This wasn't implemented correclty in
XScreenPixelBuffer - it was failing completely when shmget() returns an
error.

BUG=705146
==========

to

==========
Update XServerPixelBuffer to handle shmget() errors properly.

When screen/window capturers fail to use shared memory, they are
supposed to fall back to XGetImage(). It's slower, but should still
allow to capture desktop content. This wasn't implemented correclty in
XScreenPixelBuffer - it was failing completely when shmget() returns an
error.

BUG=705146

Review-Url: https://codereview.webrtc.org/2775793003
Cr-Commit-Position: refs/heads/master@{#17401}
Committed:
https://chromium.googlesource.com/external/webrtc/+/858b0f4703e043e1b90b95c13...
==========

[commit-bot: I haz the power, 2017-03-27 20:47:36]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/external/webrtc/+/858b0f4703e043e1b90b95c13...