Add start/stop listening Platform methods for Screen Orientation.

Add start/stop listening Platform methods for Screen Orientation.

This will allow platforms that do not support accurate and efficient
screen orientation listening to only spend resources when needed

In practice, the current platform we are targetting is Android 4.0/4.1.

BUG=162827

[mlamouri (slow - plz ping), 2014-07-16 16:57:15]

Note: this is required by https://codereview.chromium.org/398013002/

[abarth-chromium, 2014-07-16 17:12:17]

https://codereview.chromium.org/398003002/diff/1/public/platform/Platform.h
File public/platform/Platform.h (right):

https://codereview.chromium.org/398003002/diff/1/public/platform/Platform.h#n...
public/platform/Platform.h:659: 
The way we've done this for other APIs is to set a listener pointer to a null or
non-null value.

It's a bit strange for these calls to be routed through the platform but for the
notification to come in from the web layer.  Can we signal this information via
the web frame client?  In principle, Platform and WebFrameClient are completely
separate systems.

[mlamouri (slow - plz ping), 2014-07-16 17:21:57]

On 2014/07/16 17:12:17, abarth wrote:
> https://codereview.chromium.org/398003002/diff/1/public/platform/Platform.h
> File public/platform/Platform.h (right):
> 
>
https://codereview.chromium.org/398003002/diff/1/public/platform/Platform.h#n...
> public/platform/Platform.h:659: 
> The way we've done this for other APIs is to set a listener pointer to a null
or
> non-null value.
> 
> It's a bit strange for these calls to be routed through the platform but for
the
> notification to come in from the web layer.  Can we signal this information
via
> the web frame client?  In principle, Platform and WebFrameClient are
completely
> separate systems.

As I see it, Platform calls are global to the platform while WebFrame calls are
specific to a frame. Here, I want to notify the platform that there is a new
listener so Android 4.0/4.1 can be fixed and start polling for the screen
orientation changes. I could go trough the WebFrame but that would be an
unnecessary complexity given that the call isn't specific to the frame.

Basically, what I do in the content/ side is what most Platform consumers do: I
send an IPC, catch it in a BrowserMessageFilter and do something that is global
to the browser process. In that case, count the number of signals and start/stop
polling based on that.

I do not pass a Listener because this is an edge case that is only affecting
some old version of a Platform and changing the entire design of the feature in
order to match that edge case sounds unneeded. Every platform except Android
4.0/4.1 will listen for screen orientation changes from startup because this is
required by Chrome but the bug Android 4.0/4.1 has isn't big enough for Chrome
but make the API incorrect.

[abarth-chromium, 2014-07-16 18:14:14]

On 2014/07/16 at 17:21:57, mlamouri wrote:
> Every platform except Android 4.0/4.1 will listen for screen orientation
changes from startup because this is required by Chrome but the bug Android
4.0/4.1 has isn't big enough for Chrome but make the API incorrect.

Why not just always have Android 4.0/4.1 listen for orientation changes?

[mlamouri (slow - plz ping), 2014-07-16 18:28:04]

On 2014/07/16 18:14:14, abarth wrote:
> On 2014/07/16 at 17:21:57, mlamouri wrote:
> > Every platform except Android 4.0/4.1 will listen for screen orientation
> changes from startup because this is required by Chrome but the bug Android
> 4.0/4.1 has isn't big enough for Chrome but make the API incorrect.
> 
> Why not just always have Android 4.0/4.1 listen for orientation changes?

It does. Somehow. The Android SDK only provides an accurate API for display
changes since API Level 17 (4.2+). For Android 4.0 and Android 4.1, what we are
going to do with the signals sent from Blink is to simply poll by looking at the
Display object and check if it has changed. For obvious reasons, we only want to
do that when we actually care about this level of accuracy. Note that by "level
of accuracy", the issue is that without polling, we are only aware of 90 or 270
degrees change, not 180 degrees.

[abarth-chromium, 2014-07-16 20:26:11]

On 2014/07/16 at 18:28:04, mlamouri wrote:
> On 2014/07/16 18:14:14, abarth wrote:
> > On 2014/07/16 at 17:21:57, mlamouri wrote:
> > > Every platform except Android 4.0/4.1 will listen for screen orientation
> > changes from startup because this is required by Chrome but the bug Android
> > 4.0/4.1 has isn't big enough for Chrome but make the API incorrect.
> > 
> > Why not just always have Android 4.0/4.1 listen for orientation changes?
> 
> It does. Somehow. The Android SDK only provides an accurate API for display
changes since API Level 17 (4.2+). For Android 4.0 and Android 4.1, what we are
going to do with the signals sent from Blink is to simply poll by looking at the
Display object and check if it has changed. For obvious reasons, we only want to
do that when we actually care about this level of accuracy. Note that by "level
of accuracy", the issue is that without polling, we are only aware of 90 or 270
degrees change, not 180 degrees.

Perhaps we should just not support this feature on 4.1 and below.

[mlamouri (slow - plz ping), 2014-07-16 23:25:01]

On 2014/07/16 20:26:11, abarth wrote:
> Perhaps we should just not support this feature on 4.1 and below.

If we consider that Chrome Android is for 4.0+, disabling the feature on 4.0 and
4.1 would mean disabling the feature for ~50% of our users assuming that the
Chrome Android user base matches the Android user base:
https://developer.android.com/about/dashboards/index.html

I think that solution is a good compromise: the amount of Blink code specific to
this is fairly low and we don't have to eagerly poll all the time just for a
feature that will unlikely be used by most web sites.

[Michael van Ouwerkerk, 2014-07-17 10:36:27]

https://codereview.chromium.org/398003002/diff/1/Source/modules/screen_orient...
File Source/modules/screen_orientation/ScreenOrientation.cpp (right):

https://codereview.chromium.org/398003002/diff/1/Source/modules/screen_orient...
Source/modules/screen_orientation/ScreenOrientation.cpp:117:
blink::Platform::current()->stopScreenOrientationListening();
Shouldn't this also be called when page visibility changes to !visible? This
could go in a followup patch if you prefer, but I think you could still add it.

[mlamouri (slow - plz ping), 2014-07-17 16:04:34]

Review comments applied. PTAL.

I also wrote a design doc describing what I'm trying to do with this CL:
https://docs.google.com/document/d/10X8HPVTK9VzB9C5HVxSiElJK5XF4nl49q9UGnztoB...

[Michael van Ouwerkerk, 2014-07-17 16:52:50]

lgtm

[abarth-chromium, 2014-07-17 18:07:47]

Thanks for the doc.

IMHO, we should just ship with the bug.  That's better than polling and burning
the user's battery.

[mlamouri (slow - plz ping), 2014-07-17 19:30:03]

On 2014/07/17 18:07:47, abarth wrote:
> Thanks for the doc.
> 
> IMHO, we should just ship with the bug.  That's better than polling and
burning
> the user's battery.

Given that we will poll only when the API is used on the current tab when Chrome
is in foreground, it's not sure that this will have a big impact on the battery.
It might be barely noticeable compared to the activity generated by a page being
interacted with. It is anyway very hard to quantify without experimenting. Our
plan is to ship with polling and tweak things based on API usage and analysis we
will run.

[abarth-chromium, 2014-07-17 20:08:11]

This CL is also buggy.  If there are multiple ScreenOrientationControllers in a
given process (i.e., if there are multiple pages), they'll fight over this
static.  I don't think it's worthwhile.

[mlamouri (slow - plz ping), 2014-08-06 14:18:37]

superseeded by https://codereview.chromium.org/419643008/