[RemotePlayback] Keep track of source compatibility and reject prompt() correspondingly

[RemotePlayback] Keep track of source compatibility and reject prompt() correspondingly

BUG=659675, 659677, 659679
TEST=manual on avayvod.github.io/remote-playback/test.html
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/b485de7de811fb75f1160c23dd5b2b5a85c8a6b4
Cr-Commit-Position: refs/heads/master@{#431516}

[whywhat, 2016-11-05 04:10:05]

avayvod@chromium.org changed reviewers:
+ dcheng@chromium.org, hubbe@chromium.org, mlamouri@chromium.org

[whywhat, 2016-11-05 04:10:06]

PTaL

hubbe: *media*
mlamouri: *remote*
dcheng: *messages*

Thanks!

[whywhat, 2016-11-05 04:10:14]

The CQ bit was checked by avayvod@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-05 04:10:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-05 05:07:08]

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

[commit-bot: I haz the power, 2016-11-05 05:07:09]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[whywhat, 2016-11-08 03:45:50]

Fixed failing tests

[whywhat, 2016-11-08 03:45:50]

Description was changed from

==========
[RemotePlayback] Keep track of source compatibility and reject prompt()
correspondingly

BUG=659675,659677,659679
TEST=manual on avayvod.github.io/remote-playback/test.html
==========

to

==========
[RemotePlayback] Keep track of source compatibility and reject prompt()
correspondingly

BUG=659675,659677,659679
TEST=manual on avayvod.github.io/remote-playback/test.html
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[whywhat, 2016-11-08 03:49:39]

PTAL

[whywhat, 2016-11-08 03:49:44]

The CQ bit was checked by avayvod@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-08 03:49:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hubbe, 2016-11-08 03:58:06]

media/* LGTM

[haraken, 2016-11-08 05:13:10]

haraken@chromium.org changed reviewers:
+ haraken@chromium.org

[haraken, 2016-11-08 05:13:11]

https://codereview.chromium.org/2480003002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLMediaElement.cpp (right):

https://codereview.chromium.org/2480003002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:481: *this,
ScriptState::forMainWorld(frame));

Is it not possible that this code runs on an isolated world?

https://codereview.chromium.org/2480003002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/loader/FrameLoaderClient.h (right):

https://codereview.chromium.org/2480003002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/FrameLoaderClient.h:206: ScriptState*) =
0;

Nit: Put ScriptState* as the first parameter.

[commit-bot: I haz the power, 2016-11-08 05:47:46]

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

[commit-bot: I haz the power, 2016-11-08 05:47:46]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dcheng, 2016-11-08 07:40:00]

https://codereview.chromium.org/2480003002/diff/20001/third_party/WebKit/publ...
File
third_party/WebKit/public/platform/modules/remoteplayback/WebRemotePlaybackClient.h
(right):

https://codereview.chromium.org/2480003002/diff/20001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/remoteplayback/WebRemotePlaybackClient.h:23:
bool isSourceCompatible) = 0;
Let's pass enums instead of bools; it makes the callsites easier to read.

(at the very least, let's make the new parameter an enum)

[whywhat, 2016-11-08 17:55:17]

https://codereview.chromium.org/2480003002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLMediaElement.cpp (right):

https://codereview.chromium.org/2480003002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:481: *this,
ScriptState::forMainWorld(frame));
On 2016/11/08 at 05:13:11, haraken wrote:
> Is it not possible that this code runs on an isolated world?

I don't think we plan to enable this for extensions (or workers) today. Is there
a better way to find the right ScriptState?
I could also look into changing RemotePlayback to save ScriptState on the first
call to the relevant method vs passing it to the ctor.

[whywhat, 2016-11-08 22:28:24]

FYI, I've split off the change to WebRemotePlaybackClient creation into a
separate CL: https://codereview.chromium.org/2484973005

[whywhat, 2016-11-09 01:37:43]

Changed bool to an enum

[whywhat, 2016-11-09 01:38:11]

https://codereview.chromium.org/2480003002/diff/20001/third_party/WebKit/publ...
File
third_party/WebKit/public/platform/modules/remoteplayback/WebRemotePlaybackClient.h
(right):

https://codereview.chromium.org/2480003002/diff/20001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/remoteplayback/WebRemotePlaybackClient.h:23:
bool isSourceCompatible) = 0;
On 2016/11/08 at 07:40:00, dcheng wrote:
> Let's pass enums instead of bools; it makes the callsites easier to read.
> 
> (at the very least, let's make the new parameter an enum)

Done.

[whywhat, 2016-11-09 01:39:30]

Added the missing file

[whywhat, 2016-11-09 02:37:27]

Rebased

[whywhat, 2016-11-09 02:37:51]

PTAL

[whywhat, 2016-11-09 02:37:56]

The CQ bit was checked by avayvod@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-09 02:38:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[haraken, 2016-11-09 02:47:57]

WebKit LGTM

[commit-bot: I haz the power, 2016-11-09 05:19:13]

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

[commit-bot: I haz the power, 2016-11-09 05:19:14]

Dry run: This issue passed the CQ dry run.

[dcheng, 2016-11-09 19:03:41]

https://codereview.chromium.org/2480003002/diff/80001/content/common/media/me...
File content/common/media/media_player_messages_android.h (right):

https://codereview.chromium.org/2480003002/diff/80001/content/common/media/me...
content/common/media/media_player_messages_android.h:135: bool /*
is_source_compatible */)
Sorry, I should have been clearer in my original comments, but can we use the
enum here as well?

[whywhat, 2016-11-09 22:09:28]

https://codereview.chromium.org/2480003002/diff/80001/content/common/media/me...
File content/common/media/media_player_messages_android.h (right):

https://codereview.chromium.org/2480003002/diff/80001/content/common/media/me...
content/common/media/media_player_messages_android.h:135: bool /*
is_source_compatible */)
On 2016/11/09 at 19:03:41, dcheng wrote:
> Sorry, I should have been clearer in my original comments, but can we use the
enum here as well?

Any advice on reusing the enum from public/platform/?

third_party/WebKit is not allowed in content/common or chrome/browser.
I bet content/common is not allowed in blink either.

[dcheng, 2016-11-09 22:16:40]

On 2016/11/09 22:09:28, whywhat wrote:
>
https://codereview.chromium.org/2480003002/diff/80001/content/common/media/me...
> File content/common/media/media_player_messages_android.h (right):
> 
>
https://codereview.chromium.org/2480003002/diff/80001/content/common/media/me...
> content/common/media/media_player_messages_android.h:135: bool /*
> is_source_compatible */)
> On 2016/11/09 at 19:03:41, dcheng wrote:
> > Sorry, I should have been clearer in my original comments, but can we use
the
> enum here as well?
> 
> Any advice on reusing the enum from public/platform/?
> 
> third_party/WebKit is not allowed in content/common or chrome/browser.
> I bet content/common is not allowed in blink either.

We have a lot of DEPS exceptions for enum definitions, and (to me) it seems
reasonable to make an exception for this enum type too.

[whywhat, 2016-11-09 22:24:08]

Use enum instead of bool everywhere

[whywhat, 2016-11-09 22:26:03]

On 2016/11/09 at 22:16:40, dcheng wrote:
> On 2016/11/09 22:09:28, whywhat wrote:
> >
https://codereview.chromium.org/2480003002/diff/80001/content/common/media/me...
> > File content/common/media/media_player_messages_android.h (right):
> > 
> >
https://codereview.chromium.org/2480003002/diff/80001/content/common/media/me...
> > content/common/media/media_player_messages_android.h:135: bool /*
> > is_source_compatible */)
> > On 2016/11/09 at 19:03:41, dcheng wrote:
> > > Sorry, I should have been clearer in my original comments, but can we use
the
> > enum here as well?
> > 
> > Any advice on reusing the enum from public/platform/?
> > 
> > third_party/WebKit is not allowed in content/common or chrome/browser.
> > I bet content/common is not allowed in blink either.
> 
> We have a lot of DEPS exceptions for enum definitions, and (to me) it seems
reasonable to make an exception for this enum type too.

Ah, good point. I didn't know we had exceptions for enums. Done.

[whywhat, 2016-11-09 22:26:14]

The CQ bit was checked by avayvod@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-09 22:26:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[whywhat, 2016-11-09 22:28:18]

Removed unnecessary forward declaration.

[dcheng, 2016-11-09 22:52:41]

ipc lgtm

[whywhat, 2016-11-09 22:53:48]

avayvod@chromium.org changed reviewers:
+ jochen@chromium.org

[whywhat, 2016-11-09 22:53:48]

+jochen for content/common/DEPS and chrome/browser/DEPS

[whywhat, 2016-11-10 15:41:00]

avayvod@chromium.org changed reviewers:
+ avi@chromium.org, thakis@chromium.org
- jochen@chromium.org, mlamouri@chromium.org

[whywhat, 2016-11-10 15:41:01]

+thakis: chrome/browser/DEPS
+avi: content/common/DEPS

[Nico, 2016-11-10 17:32:59]

chrome/deps lgtm

https://codereview.chromium.org/2480003002/diff/120001/chrome/browser/DEPS
File chrome/browser/DEPS (right):

https://codereview.chromium.org/2480003002/diff/120001/chrome/browser/DEPS#ne...
chrome/browser/DEPS:92: # header-only types, and some selected common code.
wow, gross. maybe we should have webkit/public/enums and allow only that here?

[Avi (use Gerrit), 2016-11-10 19:46:12]

content/common/DEPS LGTM

[whywhat, 2016-11-10 21:02:28]

Added WebRemotePlaybackAvailability.h to the BUILD.gn

[whywhat, 2016-11-10 21:15:43]

The CQ bit was checked by avayvod@chromium.org

[whywhat, 2016-11-10 21:15:43]

The patchset sent to the CQ was uploaded after l-g-t-m from hubbe@chromium.org,
haraken@chromium.org, avi@chromium.org, thakis@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2480003002/#ps140001
(title: "Added WebRemotePlaybackAvailability.h to the BUILD.gn")

[commit-bot: I haz the power, 2016-11-10 21:16:07]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-10 21:40:08]

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

[commit-bot: I haz the power, 2016-11-10 21:40:10]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[whywhat, 2016-11-11 00:15:58]

Fixed the java enum comment

[whywhat, 2016-11-11 00:16:48]

The CQ bit was checked by avayvod@chromium.org

[whywhat, 2016-11-11 00:16:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, avi@chromium.org, thakis@chromium.org, hubbe@chromium.org,
dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2480003002/#ps160001
(title: "Fixed the java enum comment")

[commit-bot: I haz the power, 2016-11-11 00:17:39]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-11 01:04:07]

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

[commit-bot: I haz the power, 2016-11-11 01:04:09]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[whywhat, 2016-11-11 04:54:52]

The CQ bit was checked by avayvod@chromium.org

[commit-bot: I haz the power, 2016-11-11 04:55:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-11 07:33:15]

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

[commit-bot: I haz the power, 2016-11-11 07:33:19]

Failed to apply patch for
chrome/android/java/src/org/chromium/chrome/browser/media/remote/RemoteMediaPlayerBridge.java:
While running git apply --index -p1;
  error: patch failed:
chrome/android/java/src/org/chromium/chrome/browser/media/remote/RemoteMediaPlayerBridge.java:9
  error:
chrome/android/java/src/org/chromium/chrome/browser/media/remote/RemoteMediaPlayerBridge.java:
patch does not apply

Patch:      
chrome/android/java/src/org/chromium/chrome/browser/media/remote/RemoteMediaPlayerBridge.java
Index:
chrome/android/java/src/org/chromium/chrome/browser/media/remote/RemoteMediaPlayerBridge.java
diff --git
a/chrome/android/java/src/org/chromium/chrome/browser/media/remote/RemoteMediaPlayerBridge.java
b/chrome/android/java/src/org/chromium/chrome/browser/media/remote/RemoteMediaPlayerBridge.java
index
33dbff5861d5a360981a0252e50ee8f5f304395f..829330db0f50a6a73ae84bad1ea7748a80ccc773
100644
---
a/chrome/android/java/src/org/chromium/chrome/browser/media/remote/RemoteMediaPlayerBridge.java
+++
b/chrome/android/java/src/org/chromium/chrome/browser/media/remote/RemoteMediaPlayerBridge.java
@@ -9,6 +9,7 @@ import android.graphics.Bitmap;
 import org.chromium.base.Log;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
+import
org.chromium.blink_public.platform.modules.remoteplayback.WebRemotePlaybackAvailability;
 import org.chromium.chrome.browser.media.remote.RemoteVideoInfo.PlayerState;
 
 /**
@@ -318,9 +319,19 @@ public class RemoteMediaPlayerBridge {
     }
 
     private void onRouteAvailabilityChange() {
+        Log.d(TAG, "onRouteAvailabilityChange: " + mRouteIsAvailable + ", " +
mIsPlayable);
         if (mNativeRemoteMediaPlayerBridge == 0) return;
-        boolean usable = mRouteIsAvailable && mIsPlayable;
-        nativeOnRouteAvailabilityChanged(mNativeRemoteMediaPlayerBridge,
usable);
+
+        int availability = WebRemotePlaybackAvailability.DeviceNotAvailable;
+        if (!mRouteIsAvailable && !mIsPlayable) {
+            availability = WebRemotePlaybackAvailability.SourceNotSupported;
+        } else if (mRouteIsAvailable && mIsPlayable) {
+            availability = WebRemotePlaybackAvailability.DeviceAvailable;
+        } else if (mRouteIsAvailable) {
+            // mIsPlayable is false here.
+            availability = WebRemotePlaybackAvailability.SourceNotCompatible;
+        }
+        nativeOnRouteAvailabilityChanged(mNativeRemoteMediaPlayerBridge,
availability);
     }
 
     @CalledByNative
@@ -354,8 +365,8 @@ public class RemoteMediaPlayerBridge {
     private native void nativeOnPaused(long nativeRemoteMediaPlayerBridge);
     private native void nativeOnRouteUnselected(long
nativeRemoteMediaPlayerBridge);
     private native void nativeOnPlaybackFinished(long
nativeRemoteMediaPlayerBridge);
-    private native void nativeOnRouteAvailabilityChanged(long
nativeRemoteMediaPlayerBridge,
-            boolean available);
+    private native void nativeOnRouteAvailabilityChanged(
+            long nativeRemoteMediaPlayerBridge, int availability);
     private native void nativeOnCancelledRemotePlaybackRequest(long
nativeRemoteMediaPlayerBridge);
     private native String nativeGetTitle(long nativeRemoteMediaPlayerBridge);
     private native void nativePauseLocal(long nativeRemoteMediaPlayerBridge);

[commit-bot: I haz the power, 2016-11-11 07:41:07]

Description was changed from

==========
[RemotePlayback] Keep track of source compatibility and reject prompt()
correspondingly

BUG=659675,659677,659679
TEST=manual on avayvod.github.io/remote-playback/test.html
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
[RemotePlayback] Keep track of source compatibility and reject prompt()
correspondingly

BUG=659675,659677,659679
TEST=manual on avayvod.github.io/remote-playback/test.html
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/b485de7de811fb75f1160c23dd5b2b5a85c8a6b4
Cr-Commit-Position: refs/heads/master@{#431516}
==========

[commit-bot: I haz the power, 2016-11-11 07:41:10]

Patchset 9 (id:??) landed as
https://crrev.com/b485de7de811fb75f1160c23dd5b2b5a85c8a6b4
Cr-Commit-Position: refs/heads/master@{#431516}