Implement ScreenOrientationProvider for Chrome Android.

Implement ScreenOrientationProvider for Chrome Android.

This allows screen.lockOrientation() and screen.unlockOrientation() to
work on Chrome Android. It does not work for the Android WebView yet and
there is no permission/security checks yet.

BUG=162827
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=257113

[mlamouri (slow - plz ping), 2014-03-11 23:18:20]

jochen@, let me know if you want someone else to review the java files.

[jochen (gone - plz use gerrit), 2014-03-12 12:42:27]

yes, please have somebody more familiar with android review the java bits

c++ bits lgtm

https://codereview.chromium.org/196193002/diff/1/content/browser/screen_orien...
File content/browser/screen_orientation/screen_orientation_provider_android.h
(right):

https://codereview.chromium.org/196193002/diff/1/content/browser/screen_orien...
content/browser/screen_orientation/screen_orientation_provider_android.h:24:
virtual ~ScreenOrientationProviderAndroid() {}
don't put non-trivial dtors in the header (it's non trivial because it has to
destruct the global java ref)

https://codereview.chromium.org/196193002/diff/1/content/browser/screen_orien...
content/browser/screen_orientation/screen_orientation_provider_android.h:27: };
disallow copy/assign

[mlamouri (slow - plz ping), 2014-03-12 14:16:36]

https://codereview.chromium.org/196193002/diff/1/content/browser/screen_orien...
File content/browser/screen_orientation/screen_orientation_provider_android.h
(right):

https://codereview.chromium.org/196193002/diff/1/content/browser/screen_orien...
content/browser/screen_orientation/screen_orientation_provider_android.h:24:
virtual ~ScreenOrientationProviderAndroid() {}
On 2014/03/12 12:42:27, jochen wrote:
> don't put non-trivial dtors in the header (it's non trivial because it has to
> destruct the global java ref)

Done.

https://codereview.chromium.org/196193002/diff/1/content/browser/screen_orien...
content/browser/screen_orientation/screen_orientation_provider_android.h:27: };
On 2014/03/12 12:42:27, jochen wrote:
> disallow copy/assign

Done.

[mlamouri (slow - plz ping), 2014-03-12 14:17:48]

Michael, can you have a look at the Java bits? Thanks :)

[Michael van Ouwerkerk, 2014-03-12 14:42:54]

https://codereview.chromium.org/196193002/diff/20001/content/browser/screen_o...
File content/browser/screen_orientation/screen_orientation_provider_android.cc
(right):

https://codereview.chromium.org/196193002/diff/20001/content/browser/screen_o...
content/browser/screen_orientation/screen_orientation_provider_android.cc:6:
#include
"content/browser/screen_orientation/screen_orientation_provider_android.h"
This line should sit alone above the rest.

https://codereview.chromium.org/196193002/diff/20001/content/browser/screen_o...
File content/browser/screen_orientation/screen_orientation_provider_android.h
(right):

https://codereview.chromium.org/196193002/diff/20001/content/browser/screen_o...
content/browser/screen_orientation/screen_orientation_provider_android.h:5:
#ifndef CONTENT_BROWSER_SCREEN_ORIENTATION_PROVIDER_ANDROID_H_
Afaik this should be the full path, so
CONTENT_BROWSER_SCREEN_ORIENTATION_SCREEN_ORIENTATION_PROVIDER_ANDROID_H_

That's the precedent I see in geolocation, device_orientation, vibration.

Same comment for all the other files in this directory.

https://codereview.chromium.org/196193002/diff/20001/content/browser/screen_o...
content/browser/screen_orientation/screen_orientation_provider_android.h:14: //
ScreenOrientationProvider for Android.
This comment doesn't add much. Delete it?

https://codereview.chromium.org/196193002/diff/20001/content/public/android/j...
File
content/public/android/java/src/org/chromium/content/browser/ScreenOrientationProvider.java
(right):

https://codereview.chromium.org/196193002/diff/20001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/ScreenOrientationProvider.java:24:
// Has to be kept in sync with:
Ugh. There are ways to automate this. They are ugly, but please take a look at
the email I forwarded to you.

https://codereview.chromium.org/196193002/diff/20001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/ScreenOrientationProvider.java:62:
if (activity == null)
Braces please. http://source.android.com/source/code-style.html

https://codereview.chromium.org/196193002/diff/20001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/ScreenOrientationProvider.java:66:
if (orientation == ActivityInfo.SCREEN_ORIENTATION_UNSPECIFIED)
Braces.

https://codereview.chromium.org/196193002/diff/20001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/ScreenOrientationProvider.java:75:
if (activity == null)
And braces.

[mlamouri (slow - plz ping), 2014-03-12 20:19:55]

PTAL. Also, let me know if you want someone else to review the non-Java changes
that I had to do.

https://codereview.chromium.org/196193002/diff/20001/content/browser/screen_o...
File content/browser/screen_orientation/screen_orientation_provider_android.cc
(right):

https://codereview.chromium.org/196193002/diff/20001/content/browser/screen_o...
content/browser/screen_orientation/screen_orientation_provider_android.cc:6:
#include
"content/browser/screen_orientation/screen_orientation_provider_android.h"
On 2014/03/12 14:42:55, Michael van Ouwerkerk wrote:
> This line should sit alone above the rest.

Done.

https://codereview.chromium.org/196193002/diff/20001/content/browser/screen_o...
File content/browser/screen_orientation/screen_orientation_provider_android.h
(right):

https://codereview.chromium.org/196193002/diff/20001/content/browser/screen_o...
content/browser/screen_orientation/screen_orientation_provider_android.h:5:
#ifndef CONTENT_BROWSER_SCREEN_ORIENTATION_PROVIDER_ANDROID_H_
On 2014/03/12 14:42:55, Michael van Ouwerkerk wrote:
> Afaik this should be the full path, so
> CONTENT_BROWSER_SCREEN_ORIENTATION_SCREEN_ORIENTATION_PROVIDER_ANDROID_H_
> 
> That's the precedent I see in geolocation, device_orientation, vibration.
> 
> Same comment for all the other files in this directory.

Do you mind if I change that for all the files in a follow-up CL?

https://codereview.chromium.org/196193002/diff/20001/content/browser/screen_o...
content/browser/screen_orientation/screen_orientation_provider_android.h:14: //
ScreenOrientationProvider for Android.
On 2014/03/12 14:42:55, Michael van Ouwerkerk wrote:
> This comment doesn't add much. Delete it?

Done.

https://codereview.chromium.org/196193002/diff/20001/content/public/android/j...
File
content/public/android/java/src/org/chromium/content/browser/ScreenOrientationProvider.java
(right):

https://codereview.chromium.org/196193002/diff/20001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/ScreenOrientationProvider.java:24:
// Has to be kept in sync with:
On 2014/03/12 14:42:55, Michael van Ouwerkerk wrote:
> Ugh. There are ways to automate this. They are ugly, but please take a look at
> the email I forwarded to you.

Done.

https://codereview.chromium.org/196193002/diff/20001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/ScreenOrientationProvider.java:62:
if (activity == null)
On 2014/03/12 14:42:55, Michael van Ouwerkerk wrote:
> Braces please. http://source.android.com/source/code-style.html

Done.

https://codereview.chromium.org/196193002/diff/20001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/ScreenOrientationProvider.java:66:
if (orientation == ActivityInfo.SCREEN_ORIENTATION_UNSPECIFIED)
On 2014/03/12 14:42:55, Michael van Ouwerkerk wrote:
> Braces.

Done.

https://codereview.chromium.org/196193002/diff/20001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/ScreenOrientationProvider.java:75:
if (activity == null)
On 2014/03/12 14:42:55, Michael van Ouwerkerk wrote:
> And braces.

Done.

[Michael van Ouwerkerk, 2014-03-13 10:22:54]

Thanks for the fixes, lgtm.

[mlamouri (slow - plz ping), 2014-03-13 10:27:11]

The CQ bit was checked by mlamouri@chromium.org

[commit-bot: I haz the power, 2014-03-13 10:27:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mlamouri@chromium.org/196193002/40001

[commit-bot: I haz the power, 2014-03-13 10:51:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-13 10:51:16]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[mlamouri (slow - plz ping), 2014-03-13 11:12:56]

The CQ bit was checked by mlamouri@chromium.org

[commit-bot: I haz the power, 2014-03-13 11:13:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mlamouri@chromium.org/196193002/60001

[commit-bot: I haz the power, 2014-03-13 12:48:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-13 12:48:19]

Retried try job too often on android_aosp for step(s) compile
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_ao...

[mlamouri (slow - plz ping), 2014-03-13 14:17:09]

benm@chromium.org: Please review changes in 
android_webview/all_webview.gyp

[benm (inactive), 2014-03-13 17:12:18]

On 2014/03/13 14:17:09, Mounir Lamouri wrote:
> benm@chromium.org: Please review changes in 
> android_webview/all_webview.gyp

a_w/ lgtm

[mlamouri (slow - plz ping), 2014-03-13 17:16:48]

The CQ bit was checked by mlamouri@chromium.org

[commit-bot: I haz the power, 2014-03-13 17:16:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mlamouri@chromium.org/196193002/50020

[commit-bot: I haz the power, 2014-03-13 19:16:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-13 19:16:13]

Retried try job too often on android_aosp for step(s) compile
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_ao...

[mlamouri (slow - plz ping), 2014-03-14 10:52:36]

The CQ bit was checked by mlamouri@chromium.org

[commit-bot: I haz the power, 2014-03-14 10:52:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mlamouri@chromium.org/196193002/90001

[commit-bot: I haz the power, 2014-03-14 10:54:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-14 10:54:02]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_clang_dbg

[benm (inactive), 2014-03-14 10:54:46]

still lgtm

https://codereview.chromium.org/196193002/diff/90001/android_webview/all_webv...
File android_webview/all_webview.gyp (right):

https://codereview.chromium.org/196193002/diff/90001/android_webview/all_webv...
android_webview/all_webview.gyp:26:
'../content/content.gyp:screen_orientation_values_java',
nit: alphabetical order

[mlamouri (slow - plz ping), 2014-03-14 10:59:59]

The CQ bit was checked by mlamouri@chromium.org

[commit-bot: I haz the power, 2014-03-14 11:00:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mlamouri@chromium.org/196193002/110001

[commit-bot: I haz the power, 2014-03-14 12:18:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-14 12:18:14]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_rel

[mlamouri (slow - plz ping), 2014-03-14 12:19:06]

The CQ bit was checked by mlamouri@chromium.org

[commit-bot: I haz the power, 2014-03-14 12:19:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mlamouri@chromium.org/196193002/110001

[commit-bot: I haz the power, 2014-03-14 13:40:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-14 13:40:59]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_rel

[mlamouri (slow - plz ping), 2014-03-14 13:52:59]

The CQ bit was checked by mlamouri@chromium.org

[commit-bot: I haz the power, 2014-03-14 13:53:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mlamouri@chromium.org/196193002/110001

[commit-bot: I haz the power, 2014-03-14 15:13:30]

Change committed as 257113