Fix potential synchronization issues with framelisteners in EglRenderer.

Fix potential synchronization issues with framelisteners in EglRenderer.

Previously, a frame queued before calling addFrameListener could be
passed to the listener. Also fixes an issue where listener could still
be called after removeFrameListener call returned.

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

[sakal, 2016-11-28 08:54:47]

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

[sakal, 2016-11-28 08:54:47]

PTAL

[magjed_webrtc, 2016-11-28 11:32:04]

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

https://codereview.webrtc.org/2529313002/diff/1/webrtc/api/android/java/src/o...
webrtc/api/android/java/src/org/webrtc/EglRenderer.java:375:
renderThreadHandler.post(new Runnable() {
Use postToRenderThread instead.

https://codereview.webrtc.org/2529313002/diff/1/webrtc/api/android/java/src/o...
webrtc/api/android/java/src/org/webrtc/EglRenderer.java:392: public void
removeFrameListener(FrameListener listener) {
Now this is racy if |listener| is posted in addFrameListener but not yet added
to |frameListeners|. I think the most simple solution would be to post to the
render thread here as well, and block until the Runnable is executed. That way,
we can also remove |frameListenerLock|. We need to be careful not to introduce
dead locks though. Can you add to the function comment that we are guaranteed
not to get any callbacks from |listener| once this function returns? Also, can
we add unittests to test this?

[sakal, 2016-11-28 14:37:26]

Patchset #3 (id:40001) has been deleted

[sakal, 2016-11-28 14:37:58]

PTAL

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

https://codereview.webrtc.org/2529313002/diff/1/webrtc/api/android/java/src/o...
webrtc/api/android/java/src/org/webrtc/EglRenderer.java:375:
renderThreadHandler.post(new Runnable() {
On 2016/11/28 at 11:32:04, magjed_webrtc wrote:
> Use postToRenderThread instead.

Done.

https://codereview.webrtc.org/2529313002/diff/1/webrtc/api/android/java/src/o...
webrtc/api/android/java/src/org/webrtc/EglRenderer.java:392: public void
removeFrameListener(FrameListener listener) {
On 2016/11/28 11:32:03, magjed_webrtc wrote:
> Now this is racy if |listener| is posted in addFrameListener but not yet added
> to |frameListeners|. I think the most simple solution would be to post to the
> render thread here as well, and block until the Runnable is executed. That
way,
> we can also remove |frameListenerLock|. We need to be careful not to introduce
> dead locks though. Can you add to the function comment that we are guaranteed
> not to get any callbacks from |listener| once this function returns? Also, can
> we add unittests to test this?

Done.

[magjed_webrtc, 2016-11-28 16:30:45]

The CQ bit was checked by magjed@webrtc.org

[magjed_webrtc, 2016-11-28 16:30:45]

lgtm

[commit-bot: I haz the power, 2016-11-28 16:30:58]

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-28 16:53:45]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1480350645569370,
"parent_rev": "fb542d942b2c9e0f5efaa3ffee165f1679be8206", "commit_rev":
"d2214c92911d964add930a7d0477aea3401a8994"}

[commit-bot: I haz the power, 2016-11-28 16:53:49]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-11-28 16:53:55]

Description was changed from

==========
Fix potential synchronization issues with framelisteners in EglRenderer.

Previously, a frame queued before calling addFrameListener could be
passed to the listener. Also fixes an issue where listener could still
be called after removeFrameListener call returned.

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

to

==========
Fix potential synchronization issues with framelisteners in EglRenderer.

Previously, a frame queued before calling addFrameListener could be
passed to the listener. Also fixes an issue where listener could still
be called after removeFrameListener call returned.

BUG=webrtc:6470

Committed: https://crrev.com/bb58435da0513e080a0f66086a3262ec087c73dc
Cr-Commit-Position: refs/heads/master@{#15275}
==========

[commit-bot: I haz the power, 2016-11-28 16:53:56]

Patchset 4 (id:??) landed as
https://crrev.com/bb58435da0513e080a0f66086a3262ec087c73dc
Cr-Commit-Position: refs/heads/master@{#15275}