check the permission for wake lock before calling setWakeMode (issue 11362018)

[qin...@chromium.org]

Reviewers: joth, Yaron,

Message:
PTAL

Description:
check the permission for wake lock before calling setWakeMode


Please review this at https://codereview.chromium.org/11362018/

SVN Base: svn://svn.chromium.org/chrome/trunk/src

Affected files:
M media/base/android/java/src/org/chromium/media/MediaPlayerListener.java


Index:
media/base/android/java/src/org/chromium/media/MediaPlayerListener.java
diff --git
a/media/base/android/java/src/org/chromium/media/MediaPlayerListener.java
b/media/base/android/java/src/org/chromium/media/MediaPlayerListener.java
index
7ab0fe8ba64332361190c95eeebd351c2e65f77a..7381ee1ec022ec059f2610279a07ce6cc985818f
100644
---
a/media/base/android/java/src/org/chromium/media/MediaPlayerListener.java
+++
b/media/base/android/java/src/org/chromium/media/MediaPlayerListener.java
@@ -5,6 +5,8 @@
package org.chromium.media;

import android.content.Context;
+import android.content.pm.PackageManager;
+import android.Manifest.permission;
import android.media.MediaPlayer;

import org.chromium.base.CalledByNative;
@@ -93,7 +95,10 @@ class MediaPlayerListener implements
MediaPlayer.OnPreparedListener,
mediaPlayer.setOnPreparedListener(listener);
mediaPlayer.setOnSeekCompleteListener(listener);
mediaPlayer.setOnVideoSizeChangedListener(listener);
- mediaPlayer.setWakeMode(context,
android.os.PowerManager.FULL_WAKE_LOCK);
+ if (PackageManager.PERMISSION_GRANTED ==
+ context.checkCallingPermission(permission.WAKE_LOCK)) {
+ mediaPlayer.setWakeMode(context,
android.os.PowerManager.FULL_WAKE_LOCK);
+ }
}

/**

[yfri...@chromium.org]

https://codereview.chromium.org/11362018/diff/1/media/base/android/java/src/org/chromium/media/MediaPlayerListener.java
File
media/base/android/java/src/org/chromium/media/MediaPlayerListener.java
(right):

https://codereview.chromium.org/11362018/diff/1/media/base/android/java/src/org/chromium/media/MediaPlayerListener.java#newcode98
media/base/android/java/src/org/chromium/media/MediaPlayerListener.java:98:
if (PackageManager.PERMISSION_GRANTED ==
Shouldn't we just add the WAKE_LOCK permission? Is there a way that the
permission can be rejected? Is there a specific bug associated with
this?

https://codereview.chromium.org/11362018/

[qin...@chromium.org]

This is for the webview, which does not have permissions.

[yfri...@chromium.org]

lgtm



https://codereview.chromium.org/11362018/

[commi...@chromium.org]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/qin...@chromium.org/11362018/1


https://chromiumcodereview.appspot.com/11362018/

[commi...@chromium.org]

Step "update" is always a major failure.
Look at the try server FAQ for more details.

https://chromiumcodereview.appspot.com/11362018/

[commi...@chromium.org]

[commi...@chromium.org]

Change committed as 165311

https://chromiumcodereview.appspot.com/11362018/

[Jonathan Dixon]

Thanks qinmin!

Did you have any thoughts on using LayoutParams FLAG_KEEP_SCREEN_ON for apps without the permission, or for all cases? (I could raise a lower prior crbug for to look into that if needed)

[Min Qin]

For MediaPlayer, i think we have to use PowerManager constants as the API does not create a new wakelock directly. But I think we can use that constant else if we are creating a wake lock

Min