776742 - FramebufferNativeWindow doesn't implement ANativeWindow::cancelBuffer

Status: RESOLVED FIXED

Description

[Marshall Culpepper [:marshall_law]]

On the emulator, When the b2g process tries to exit, a segfault happens when the emulator's EGL code tries to clean up it's internal buffers by calling eglCancelBuffer() -- unfortunately, ANativeWindow::cancelBuffer is not set by the FramebufferNativeWindow.

This is technically an AOSP bug, but we don't currently maintain forks of either platform_frameworks_base or platform_development (where this would be ideally fixed).

In the interest of maintainability, it is easy enough to set the cancelBuffer function pointer to a no-op so we can avoid segfaulting for now..

Comment 1

[Marshall Culpepper [:marshall_law]]

Created attachment 645106 [details] [diff] [review]
cancelBuffer - v1

Comment 2

[Marshall Culpepper [:marshall_law]]

I meant to say this happens in the EGL surface destructor. You can see the code in question here:

egl_window_surface_t destructor:
https://github.com/android/platform_development/blob/master/tools/emulator/opengl/system/egl/egl.cpp#L297

notice the missing set of cancelBuffer in FramebufferNativeWindow.cpp:
https://github.com/android/platform_frameworks_base/blob/ics-plus-aosp/libs/ui/FramebufferNativeWindow.cpp#L139

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 645106 [details] [diff] [review]
cancelBuffer - v1

>diff --git a/widget/gonk/nsWindow.cpp b/widget/gonk/nsWindow.cpp

>+/* static */ int
>+nsWindow::CancelBufferNoop(ANativeWindow* aWindow, ANativeWindowBuffer* aBuffer)

Let's just make this a static function in this compilation unit,
not part of nsWindow.

r=me with that change.

Comment 4

[:Ehsan Akhgari (needinfo please, extremely long backlog, Away 8/18)]

I backed out this patch because of build bustage:

https://hg.mozilla.org/integration/mozilla-inbound/rev/3e038a64e7de

Example build failure: https://tbpl.mozilla.org/php/getParsedLog.php?id=13810239&tree=Mozilla-Inbound

Comment 5

[Marshall Culpepper [:marshall_law]]

Looks like this is environment related -- this definitely compiles locally in the B2G emulator config. Phil mentioned that the Tinderboxes aren't currently setup for ICS: Bug 776045

Comment 6

[Marshall Culpepper [:marshall_law]]

Created attachment 649292 [details] [diff] [review]
cancelBuffer - v1 (rebased)

Comment 7

[Marshall Culpepper [:marshall_law]]

I've confirmed this build error

Comment 8

[Marshall Culpepper [:marshall_law]]

* I've confirmed this build error occurs because of the GB toolchain in Tinderbox. ICS based toolchains are building this patch just fine..

Comment 9

[Marshall Culpepper [:marshall_law]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6ee7b6582fbd

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/6ee7b6582fbd