[Android] Implement ScreenOrientation.unlock align with spec

[Android] Implement ScreenOrientation.unlock align with spec

This is for compliance with ScreenOrientation
(https://w3c.github.io/screen-orientation).

When unlock screen orientation, it should lock the orientation
to default orientation.
Previous implementation is to set the orientation to "unspecified",
which is not fully aligned with spec.

Instead, read the configuration from the activity metadata in
AndroidManifest.xml and set the orientation accordingly.

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

[wang16, 2014-07-16 06:02:55]

This is for compliance with ScreenOrientation 
(https://w3c.github.io/screen-orientation). 

When unlock screen orientation, it should lock the orientation 
to default orientation. 
Previous implementation is to set the orientation to "unspecified", 
which is not fully aligned with spec. 

Instead, read the configuration from the activity metadata in 
AndroidManifest.xml and set the orientation accordingly. 

Please help review, thanks.

[jochen (gone - plz use gerrit), 2014-07-16 08:58:48]

please get an OWNER closer to this file to review this

[mlamouri (slow - plz ping), 2014-07-16 09:13:43]

Thanks a lot, this patch is great! :)

https://codereview.chromium.org/399553002/diff/1/content/public/android/java/...
File
content/public/android/java/src/org/chromium/content/browser/ScreenOrientationProvider.java
(right):

https://codereview.chromium.org/399553002/diff/1/content/public/android/java/...
content/public/android/java/src/org/chromium/content/browser/ScreenOrientationProvider.java:85:
}
I would prefer if the try-catch was only wrapping the lines that need to be
wrapped in there. Could you do:

int defaultOrientation = ActivityInfo.SCREEN_ORIENTATION_UNSPECIFIED;

try {
    ActivityInfo info = activity.getPackageManager().getActivityInfo(
            activity.getComponentName(), PackageManager.GET_META_DATA);
    defaultOrientation = info.screenOrientation;
} catch (PackageManager.NameNotFoundException) {
    // Do nothing, defaultOrientation should be SCREEN_ORIENTATION_UNSPECIFIED.
} finally {
  activity.setRequestedOrientation(defaultOrientation);
}

Note that I think having the full exception name would be better (witch
PackageManager prefix). I guess you might not need for in the import in that
case.

[wang16, 2014-07-17 01:31:58]

On 2014/07/16 09:13:43, Mounir Lamouri wrote:
> Thanks a lot, this patch is great! :)
> 
>
https://codereview.chromium.org/399553002/diff/1/content/public/android/java/...
> File
>
content/public/android/java/src/org/chromium/content/browser/ScreenOrientationProvider.java
> (right):
> 
>
https://codereview.chromium.org/399553002/diff/1/content/public/android/java/...
>
content/public/android/java/src/org/chromium/content/browser/ScreenOrientationProvider.java:85:
> }
> I would prefer if the try-catch was only wrapping the lines that need to be
> wrapped in there. Could you do:
> 
> int defaultOrientation = ActivityInfo.SCREEN_ORIENTATION_UNSPECIFIED;
> 
> try {
>     ActivityInfo info = activity.getPackageManager().getActivityInfo(
>             activity.getComponentName(), PackageManager.GET_META_DATA);
>     defaultOrientation = info.screenOrientation;
> } catch (PackageManager.NameNotFoundException) {
>     // Do nothing, defaultOrientation should be
SCREEN_ORIENTATION_UNSPECIFIED.
> } finally {
>   activity.setRequestedOrientation(defaultOrientation);
> }
> 
> Note that I think having the full exception name would be better (witch
> PackageManager prefix). I guess you might not need for in the import in that
> case.

Hi,

Thanks for the comments. I've updated the patch set accordingly, thanks.

[wang16, 2014-07-17 01:33:45]

On 2014/07/16 08:58:48, jochen wrote:
> please get an OWNER closer to this file to review this

Yes, Mounir is the owner. :)

[mlamouri (slow - plz ping), 2014-07-17 08:54:18]

lgtm

[xwang, 2014-07-18 04:46:35]

The CQ bit was checked by xingnan.wang@chromium.org

[xwang, 2014-07-18 04:46:36]

The CQ bit was unchecked by xingnan.wang@chromium.org

[wang16, 2014-07-18 07:05:03]

On 2014/07/17 08:54:18, Mounir Lamouri wrote:
> lgtm

The trybots seem not working correctly, the failure doesn't have link and it's
actually not built on those bots.
Can you help on this?

[wang16, 2014-07-21 00:59:17]

The CQ bit was checked by shiliu.wang@intel.com

[wang16, 2014-07-21 00:59:17]

The CQ bit was unchecked by shiliu.wang@intel.com

[wang16, 2014-07-21 01:25:07]

The CQ bit was checked by shiliu.wang@intel.com

[commit-bot: I haz the power, 2014-07-21 01:25:48]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/shiliu.wang@intel.com/399553002/40001

[wang16, 2014-07-21 01:44:40]

update AUTHOR

[wang16, 2014-07-21 01:49:28]

On 2014/07/21 01:44:40, wang16 wrote:
> update AUTHOR

Intel already signed corporate CLA for me. Just update the AUTHORS to include my
email.

[wang16, 2014-07-21 01:49:40]

The CQ bit was checked by shiliu.wang@intel.com

[commit-bot: I haz the power, 2014-07-21 01:50:46]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/shiliu.wang@intel.com/399553002/60001

[commit-bot: I haz the power, 2014-07-21 07:11:11]

Change committed as 284396