Changes to enable HTML5 autoplay for voice search.

content/browser/android/content_view_core_impl.cc

Index: content/browser/android/content_view_core_impl.cc
diff --git a/content/browser/android/content_view_core_impl.cc b/content/browser/android/content_view_core_impl.cc
index 2a53563c4f309e698c4ece2b021b230e0d2537cd..8b1ff2c0d9c2e0dff40601929bf96ef01e26ef2c 100644
--- a/content/browser/android/content_view_core_impl.cc
+++ b/content/browser/android/content_view_core_impl.cc
@@ -329,6 +329,20 @@ void ContentViewCoreImpl::Show() {
   GetWebContents()->WasShown();
 }
 
+void ContentViewCoreImpl::SetMediaAutoplayEnabled(JNIEnv* env, jobject obj,
+    jboolean is_enabled) {

[Ted C, 2013/12/10 22:58:20]

if they don't all fit on the first line in C++, each one goes on it's own line.

In this case, I think you can do:

SetMediaAutoplayEnabled(JNIEnv* env,
                        jobject obj,
                        jboolean is_enabled) {
  ...
}

If the names are long, then you do something like:

SetMediaAutoplayEnabled(
    JNIEnv* env_super_long_names_are_totally_neato,
    jobject obj_and_would_be_longer_than_80_chars,
    jboolean is_enabled) {
  ...
}

[apiccion, 2013/12/13 06:09:12]

Done. Thank you!

+  SetMediaAutoplayEnabled(is_enabled);
+}
+
+void ContentViewCoreImpl::SetMediaAutoplayEnabled(bool is_enabled) {

[Ted C, 2013/12/10 22:58:20]

do you need this to be callable from native?

If not, I would just inline all this in the JNI method above.

[apiccion, 2013/12/13 06:09:12]

Done.

+  if (is_enabled == is_media_autoplay_enabled_) return;

[Ted C, 2013/12/10 22:58:20]

The "return;" needs to go on the following line w/o braces in c++ land.

[apiccion, 2013/12/13 06:09:12]

Done.

+  is_media_autoplay_enabled_ = is_enabled;
+  RenderViewHost* host = web_contents_->GetRenderViewHost();
+  WebPreferences prefs = host->GetWebkitPreferences();
+  prefs.user_gesture_required_for_media_playback = !is_enabled;
+  host->UpdateWebkitPreferences(prefs);

[Ted C, 2013/12/10 22:58:20]

Actually, I just checked out RenderViewHostImpl::GetWebkitPrefs(const GURL& url)
that you linked, and I'm wondering if we could put this in:

ChromeContentBrowserClient::OverrideWebkitPrefs(...)

Maybe that would get rid of the need for you to turn it on and off if that
happens to get called each navigation.  Might be worth investigating.

If not, I mentioned that it might be good to extract all of this into a
VoiceSearchTabHelper that manages the web contents observer as well as this code
instead of having to put it into ContentViewCore.

[apiccion, 2013/12/13 06:09:12]

It only triggers on browser context navigation. So we still need a mechanism to
disable it. Additionally, I'm not sure if it's 'safe' to enable it from an
observer since there will be a lag between IPC calls (e.g. it may not enable on
time).

On 2013/12/10 22:58:20, Ted C wrote:

+}
+
 void ContentViewCoreImpl::Hide() {
   GetWebContents()->WasHidden();
   PauseVideo();