844399 - lock fennec in landscape mode for demo purposes

Status: RESOLVED WONTFIX

(Core :: WebRTC, defect)

Description

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: landscape lock patch, v1

This is a temporary workaround to allow us to offer a good demo before the portrait-mode code is landed.

First cut patch at landscape locking from gcp attached.  It seems to cause some crashiness, so next step is to debug and see if we can fix that.

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

Caught a crash in a debug build with this stuff from logcat:

adb| ###!!! ASSERTION: Buffer underran: 'bufferEnd >= mCurrentTime', file /Us...
adb| void mozilla::AndroidBridge::HandleGeckoMessage(const nsAString_internal...
adb| leaving void mozilla::AndroidBridge::HandleGeckoMessage(const nsAString_...
adb| void mozilla::AndroidBridge::HandleGeckoMessage(const nsAString_internal...
adb| leaving void mozilla::AndroidBridge::HandleGeckoMessage(const nsAString_...
adb| WARNING: MediaPipeline Transport failed. This is not properly cleaned up...
adb| WARNING: MediaPipeline Transport failed. This is not properly cleaned up...
adb| WARNING: MediaPipeline Transport failed. This is not properly cleaned up...
adb| WARNING: MediaPipeline Transport failed. This is not properly cleaned up...

For unclear reasons, however, there are no symbols.

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

FWIW, the assertion seems to come from MediaStreamGraphImpl::WillUnderrun

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

Excitingly, it seems much more difficult to provoke the crash in debug builds.  I have, however, seen plenty of instances of the Buffer underran assertion without any crashes, suggesting it's probably unrelated.  I have not seen more of the MediaTransport warnings, for whatever that's worth.

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

I'm now finding it almost impossible to reproduce the crashiness of the landscape-only patch on my own builds.  I am, however, still occasionally seeing the incoming call web content get laid out in a way where the buttons are not displayed at all, making it impossible to answer the call.

Comment 5

[Gian-Carlo Pascutto [:gcp]]

This is smelling like an (existing) race condition to me, that the landscape patch somehow makes more likely to trigger.

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

NOTE: that underrun assertion is bug 839650.  And also note that Try builds default to ASSERTIONS crash.  This can be overridden in one of the files (I think in build/).  It's basically something that sets the default value of the env var that controls this, and changes the default for Try.  This assertion is fairly innocuous in practice.  Note that regular automatic Alder builds *shouldn't* have assert-is-crash.

Comment 7

[Gian-Carlo Pascutto [:gcp]]

I understand we are doing the demos in portrait mode, as those patches were working fine, so putting this to WONTFIX.