Fix status bar visibility issue on pre-Marshmallow devices

Fix status bar visibility issue on pre-Marshmallow devices

When the OSK is dismissed on Marshmallow, the OS will call
onSystemUiVisibilityChange which will hide any remaining system UI such
as the status or navigation bar.

On pre-marshallow OSes this method is not called if the user has
interacted with the system UI (such as pulling the notification UI
down).  When this occurs, the user is left in a state where the system
UI acts 'sticky', that is all user interactions are directed to the
system UI instead of our app.

This change will manually hide the system UI on older OSes to prevent
this issue from occurring.

BUG=581931
Committed: https://crrev.com/1771a0d1b813877eebefca92a0f8c5077de6dd80
Cr-Commit-Position: refs/heads/master@{#372404}

[joedow, 2016-01-29 17:42:25]

joedow@chromium.org changed reviewers:
+ lambroslambrou@chromium.org

[joedow, 2016-01-29 17:42:25]

PTAL!

Thanks,
Joe

[Lambros, 2016-01-29 19:09:57]

lgtm

https://codereview.chromium.org/1643273002/diff/1/remoting/android/java/src/o...
File remoting/android/java/src/org/chromium/chromoting/Desktop.java (right):

https://codereview.chromium.org/1643273002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/Desktop.java:80: /** Flag to
indicate whether to manually hide the system UI when the OSK is dismissed. */
Avoid OSK abbreviation, maybe just "keyboard" or "Soft Input UI" instead?

https://codereview.chromium.org/1643273002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/Desktop.java:449: // OSK is
dismissed if the user has interacted with the status bar.
This comment seems to contradict the first sentence in the comment above? Are
you saying that the OS will not call onSystemUiVisibilityChange() until the
keyboard has been dismissed; and on pre-M versions, it doesn't even get called
at all?

I think the problem is slightly different than what the comment indicates. You
say that we don't get the full-screen notification, but I think perhaps the
system doesn't even go into fullscreen. That's the real problem isn't it - the
system is out of full-screen, but our toolbar gets hidden, so the user has no
way of revealing the toolbar?

https://codereview.chromium.org/1643273002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/Desktop.java:555: if
(mHideSystemUIOnOskDismiss) {
Why do you need to check this flag again? You already check it in the previous
if statement.

[joedow, 2016-01-29 19:31:17]

https://codereview.chromium.org/1643273002/diff/1/remoting/android/java/src/o...
File remoting/android/java/src/org/chromium/chromoting/Desktop.java (right):

https://codereview.chromium.org/1643273002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/Desktop.java:80: /** Flag to
indicate whether to manually hide the system UI when the OSK is dismissed. */
On 2016/01/29 19:09:57, Lambros wrote:
> Avoid OSK abbreviation, maybe just "keyboard" or "Soft Input UI" instead?
> 

Done.

https://codereview.chromium.org/1643273002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/Desktop.java:449: // OSK is
dismissed if the user has interacted with the status bar.
On 2016/01/29 19:09:57, Lambros wrote:
> This comment seems to contradict the first sentence in the comment above? Are
> you saying that the OS will not call onSystemUiVisibilityChange() until the
> keyboard has been dismissed; and on pre-M versions, it doesn't even get called
> at all?
> 
> I think the problem is slightly different than what the comment indicates. You
> say that we don't get the full-screen notification, but I think perhaps the
> system doesn't even go into fullscreen. That's the real problem isn't it - the
> system is out of full-screen, but our toolbar gets hidden, so the user has no
> way of revealing the toolbar?

It doesn't contradict the sentence, it is documenting a complication due to the
older OS behavior.  onSystemUiVisibilityChange() is called for all OSes except
when the user interacts with the stats bar on pre-Marshmallow OSes.

For the second comment, I don't think it is a fullscreen issue.  It seems to me
that the OS acts like it is in immersive sticky mode even though that flag is
not specified.

https://codereview.chromium.org/1643273002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/Desktop.java:555: if
(mHideSystemUIOnOskDismiss) {
On 2016/01/29 19:09:57, Lambros wrote:
> Why do you need to check this flag again? You already check it in the previous
> if statement.

I am checking it here because this task is appended to the work queue.  *If* a
pending call to 'showActionBar()' were ahead of this task then the show would be
negated by this call.  Instead I check the value of the flag inside the queued
task to ensure it is still the intended behavior when the task is executed.

If the task were queued multiple times (due to a bug) we would still only
execute the hide once.

[joedow, 2016-01-29 19:31:21]

The CQ bit was checked by joedow@chromium.org

[joedow, 2016-01-29 19:31:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
lambroslambrou@chromium.org
Link to the patchset: https://codereview.chromium.org/1643273002/#ps20001
(title: "Addressing CR feedback")

[commit-bot: I haz the power, 2016-01-29 19:33:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1643273002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1643273002/20001

[commit-bot: I haz the power, 2016-01-29 20:27:35]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-29 20:28:35]

Description was changed from

==========
Fix status bar visibility issue on pre-Marshmallow devices

When the OSK is dismissed on Marshmallow, the OS will call
onSystemUiVisibilityChange which will hide any remaining system UI such
as the status or navigation bar.

On pre-marshallow OSes this method is not called if the user has
interacted with the system UI (such as pulling the notification UI
down).  When this occurs, the user is left in a state where the system
UI acts 'sticky', that is all user interactions are directed to the
system UI instead of our app.

This change will manually hide the system UI on older OSes to prevent
this issue from occurring.

BUG=581931
==========

to

==========
Fix status bar visibility issue on pre-Marshmallow devices

When the OSK is dismissed on Marshmallow, the OS will call
onSystemUiVisibilityChange which will hide any remaining system UI such
as the status or navigation bar.

On pre-marshallow OSes this method is not called if the user has
interacted with the system UI (such as pulling the notification UI
down).  When this occurs, the user is left in a state where the system
UI acts 'sticky', that is all user interactions are directed to the
system UI instead of our app.

This change will manually hide the system UI on older OSes to prevent
this issue from occurring.

BUG=581931

Committed: https://crrev.com/1771a0d1b813877eebefca92a0f8c5077de6dd80
Cr-Commit-Position: refs/heads/master@{#372404}
==========

[commit-bot: I haz the power, 2016-01-29 20:28:35]

Patchset 2 (id:??) landed as
https://crrev.com/1771a0d1b813877eebefca92a0f8c5077de6dd80
Cr-Commit-Position: refs/heads/master@{#372404}