Changes to enable HTML5 autoplay for voice search.

content/browser/android/content_view_core_impl.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/android/content_view_core_impl.h"
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_array.h"
 #include "base/android/jni_string.h"
 #include "base/android/scoped_java_ref.h"
@@ skipping 311 matching lines @@
 }
 
 void ContentViewCoreImpl::OnShow(JNIEnv* env, jobject obj) {
   Show();
 }
 
 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();
 }
 
 void ContentViewCoreImpl::PauseVideo() {
   RenderViewHost* host = web_contents_->GetRenderViewHost();
   if (host)
     host->Send(new ViewMsg_PauseVideo(host->GetRoutingID()));
 }
@@ skipping 1294 matching lines @@
       reinterpret_cast<ui::ViewAndroid*>(view_android),
       reinterpret_cast<ui::WindowAndroid*>(window_android));
   return reinterpret_cast<intptr_t>(view);
 }
 
 bool RegisterContentViewCore(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
 }  // namespace content