Make releaseEglSurface in EglRenderer asynchronous.

Make releaseEglSurface in EglRenderer asynchronous.

BUG=webrtc:6470
Committed: https://crrev.com/28ec6bdce85edbd63d0483fd34ed3c0c31eadca6
Cr-Commit-Position: refs/heads/master@{#14993}

[sakal, 2016-11-08 09:48:47]

Description was changed from

==========
Make releaseEglSurface in EglRenderer asynchronous.

BUG=webrtc:6470
==========

to

==========
Make releaseEglSurface in EglRenderer asynchronous.

BUG=webrtc:6470
==========

[sakal, 2016-11-08 09:48:47]

sakal@webrtc.org changed reviewers:
+ magjed@webrtc.org

[sakal, 2016-11-08 09:49:07]

PTAL

[magjed_webrtc, 2016-11-08 12:41:11]

https://codereview.webrtc.org/2483143002/diff/1/webrtc/api/android/java/src/o...
File webrtc/api/android/java/src/org/webrtc/EglRenderer.java (right):

https://codereview.webrtc.org/2483143002/diff/1/webrtc/api/android/java/src/o...
webrtc/api/android/java/src/org/webrtc/EglRenderer.java:443: if
(renderThreadHandler != null) {
nit: Can you change the order to this instead:
if (renderThreadHandler == null) {
  completionCallback.run();
  return;
}
...
post(...
   ...
   completionCallback.run();
);

[sakal, 2016-11-09 08:18:26]

https://codereview.webrtc.org/2483143002/diff/1/webrtc/api/android/java/src/o...
File webrtc/api/android/java/src/org/webrtc/EglRenderer.java (right):

https://codereview.webrtc.org/2483143002/diff/1/webrtc/api/android/java/src/o...
webrtc/api/android/java/src/org/webrtc/EglRenderer.java:443: if
(renderThreadHandler != null) {
On 2016/11/08 12:41:10, magjed_webrtc wrote:
> nit: Can you change the order to this instead:
> if (renderThreadHandler == null) {
>   completionCallback.run();
>   return;
> }
> ...
> post(...
>    ...
>    completionCallback.run();
> );

I agree the layout you are proposing would be cleaner. However, even though in
practice this will likely not be an issue, I'd prefer not to keep the lock while
calling the completionCallback to avoid causing potential deadlocks.
Implementing this in the layout you are proposing would require adding variables
to the method, which I would like to avoid.

[magjed_webrtc, 2016-11-09 08:39:06]

lgtm

https://codereview.webrtc.org/2483143002/diff/1/webrtc/api/android/java/src/o...
File webrtc/api/android/java/src/org/webrtc/EglRenderer.java (right):

https://codereview.webrtc.org/2483143002/diff/1/webrtc/api/android/java/src/o...
webrtc/api/android/java/src/org/webrtc/EglRenderer.java:443: if
(renderThreadHandler != null) {
On 2016/11/09 08:18:26, sakal wrote:
> On 2016/11/08 12:41:10, magjed_webrtc wrote:
> > nit: Can you change the order to this instead:
> > if (renderThreadHandler == null) {
> >   completionCallback.run();
> >   return;
> > }
> > ...
> > post(...
> >    ...
> >    completionCallback.run();
> > );
> 
> I agree the layout you are proposing would be cleaner. However, even though in
> practice this will likely not be an issue, I'd prefer not to keep the lock
while
> calling the completionCallback to avoid causing potential deadlocks.
> Implementing this in the layout you are proposing would require adding
variables
> to the method, which I would like to avoid.

I see, valid point.

[sakal, 2016-11-09 09:07:34]

The CQ bit was checked by sakal@webrtc.org

[commit-bot: I haz the power, 2016-11-09 09:07:42]

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, 2016-11-09 09:47:14]

Description was changed from

==========
Make releaseEglSurface in EglRenderer asynchronous.

BUG=webrtc:6470
==========

to

==========
Make releaseEglSurface in EglRenderer asynchronous.

BUG=webrtc:6470
==========

[commit-bot: I haz the power, 2016-11-09 09:47:15]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-11-09 09:47:27]

Description was changed from

==========
Make releaseEglSurface in EglRenderer asynchronous.

BUG=webrtc:6470
==========

to

==========
Make releaseEglSurface in EglRenderer asynchronous.

BUG=webrtc:6470

Committed: https://crrev.com/28ec6bdce85edbd63d0483fd34ed3c0c31eadca6
Cr-Commit-Position: refs/heads/master@{#14993}
==========

[commit-bot: I haz the power, 2016-11-09 09:47:28]

Patchset 1 (id:??) landed as
https://crrev.com/28ec6bdce85edbd63d0483fd34ed3c0c31eadca6
Cr-Commit-Position: refs/heads/master@{#14993}