Cache the current WebPreferences on RenderViewHostImpl (try #2).

Cache the current WebPreferences on RenderViewHostImpl (try #2).

This makes lookups of a WebPreferences field fast.
In order do this, add code to listen to all preferences updates in order to recompute the cache.

The first version was rolled back because some code in Blink that listened for inspector prefs updates
incorrectly sent the preference update back to Chromium, causing an infinite loop.

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

[chrishtr, 2014-08-06 16:14:59]

https://codereview.chromium.org/444843002/diff/40001/content/browser/renderer...
File content/browser/renderer_host/render_view_host_impl.cc (right):

https://codereview.chromium.org/444843002/diff/40001/content/browser/renderer...
content/browser/renderer_host/render_view_host_impl.cc:1441: // This is
defensive code to avoid infinite loops due to code run inside
See additional defensive code here to avoid an infinite loop. It should never
happen,
but the severity of the problem if it happens justifies this code.

[boliu, 2014-08-06 16:52:35]

android_webview lgtm

https://codereview.chromium.org/444843002/diff/40001/content/browser/renderer...
File content/browser/renderer_host/render_view_host_impl.h (right):

https://codereview.chromium.org/444843002/diff/40001/content/browser/renderer...
content/browser/renderer_host/render_view_host_impl.h:205: virtual void
UpdateWebkitPreferences(
I think this should be removed from the public content api now? (Didn't actually
check if all usage is removed)

[chrishtr, 2014-08-06 17:00:52]

https://codereview.chromium.org/444843002/diff/40001/content/browser/renderer...
File content/browser/renderer_host/render_view_host_impl.h (right):

https://codereview.chromium.org/444843002/diff/40001/content/browser/renderer...
content/browser/renderer_host/render_view_host_impl.h:205: virtual void
UpdateWebkitPreferences(
On 2014/08/06 16:52:35, boliu wrote:
> I think this should be removed from the public content api now? (Didn't
actually
> check if all usage is removed)

There are still call sites (some in this CL). This is the way to explicitly
update the preferences.
See for example voice_search_tab_helper.cc.

[jam, 2014-08-07 05:51:04]

Is it possible to upload the rebased original change, and then upload the fixes?
that way I can just review the fixes and not the whole cl again

[chrishtr, 2014-08-07 17:26:03]

On 2014/08/07 05:51:04, jam wrote:
> Is it possible to upload the rebased original change, and then upload the
fixes?
> that way I can just review the fixes and not the whole cl again

Good idea. I've done that. Patch set 5 is CL 373323003, rebased to ToT. Patch
set 6 is the current CL. You'll see only two files changed from patch set 5.

[chrishtr, 2014-08-07 17:27:13]

https://codereview.chromium.org/444843002/diff/140001/chrome/browser/android/...
File chrome/browser/android/voice_search_tab_helper.cc (right):

https://codereview.chromium.org/444843002/diff/140001/chrome/browser/android/...
chrome/browser/android/voice_search_tab_helper.cc:27: const CommandLine&
command_line = *CommandLine::ForCurrentProcess();
This integrates CL 405253004 also.

[jam, 2014-08-07 18:20:47]

lgtm

[chrishtr, 2014-08-07 18:21:33]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2014-08-07 18:23:39]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/chrishtr@chromium.org/444843002/140001

[commit-bot: I haz the power, 2014-08-08 00:48:23]

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

[commit-bot: I haz the power, 2014-08-08 00:48:33]

Failed to request the patch to try. Please note that binary files are still
unsupported at the moment, this is being worked on.

Thanks for your patience.

Transient error: <urlopen error [Errno 104] Connection reset by peer>

[chrishtr, 2014-08-08 16:14:14]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2014-08-08 16:14:57]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/chrishtr@chromium.org/444843002/140001

[commit-bot: I haz the power, 2014-08-08 16:16:12]

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

[commit-bot: I haz the power, 2014-08-08 16:16:13]

Failed to apply patch for chrome/browser/android/voice_search_tab_helper.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  can't find file to patch at input line 6
  Perhaps you used the wrong -p or --strip option?
  The text leading up to this was:
  --------------------------
  |Index: chrome/browser/android/voice_search_tab_helper.cc
  |diff --git a/chrome/browser/android/voice_search_tab_helper.cc
b/chrome/browser/android/voice_search_tab_helper.cc
  |index
c8f10db3662fae873a0708f60ee2a395dd6b40b3..0782f7578ece106454e03072177956f114b5073f
100644
  |--- a/chrome/browser/android/voice_search_tab_helper.cc
  |+++ b/chrome/browser/android/voice_search_tab_helper.cc
  --------------------------
  No file to patch.  Skipping patch.
  2 out of 2 hunks ignored

Patch:       chrome/browser/android/voice_search_tab_helper.cc
Index: chrome/browser/android/voice_search_tab_helper.cc
diff --git a/chrome/browser/android/voice_search_tab_helper.cc
b/chrome/browser/android/voice_search_tab_helper.cc
index
c8f10db3662fae873a0708f60ee2a395dd6b40b3..0782f7578ece106454e03072177956f114b5073f
100644
--- a/chrome/browser/android/voice_search_tab_helper.cc
+++ b/chrome/browser/android/voice_search_tab_helper.cc
@@ -4,9 +4,11 @@
 
 #include "chrome/browser/android/voice_search_tab_helper.h"
 
+#include "base/command_line.h"
 #include "components/google/core/browser/google_util.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/content_switches.h"
 #include "content/public/common/web_preferences.h"
 #include "jni/VoiceSearchTabHelper_jni.h"
 
@@ -20,19 +22,25 @@ bool RegisterVoiceSearchTabHelper(JNIEnv* env) {
 static void UpdateAutoplayStatus(JNIEnv* env,
                                  jobject obj,
                                  jobject j_web_contents) {
+  // In the case where media autoplay has been disabled by default (e.g. in
+  // performance media tests) do not update it based on navigation changes.
+  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
+  if (command_line.HasSwitch(
+      switches::kDisableGestureRequirementForMediaPlayback))
+    return;
+
   WebContents* web_contents = WebContents::FromJavaWebContents(j_web_contents);
   content::RenderViewHost* host = web_contents->GetRenderViewHost();
   content::WebPreferences prefs = host->GetWebkitPreferences();
 
-  // In the case where media autoplay has been enabled by default (e.g. in
-  // performance media tests) do not update it based on navigation changes.
-  //
-  // Note that GetWekitPreferences() is 'stateless'. It returns the default
-  // webkit preferences configuration from command line switches.
-  if (!prefs.user_gesture_required_for_media_playback)
-    return;
-
-  prefs.user_gesture_required_for_media_playback =
+  bool gesture_required =
       !google_util::IsGoogleSearchUrl(web_contents->GetLastCommittedURL());
-  host->UpdateWebkitPreferences(prefs);
+
+  if (gesture_required != prefs.user_gesture_required_for_media_playback) {
+    // TODO(chrishtr): this is wrong. user_gesture_required_for_media_playback
+    // will be reset the next time a preference changes.
+    prefs.user_gesture_required_for_media_playback =
+        !google_util::IsGoogleSearchUrl(web_contents->GetLastCommittedURL());
+    host->UpdateWebkitPreferences(prefs);
+  }
 }

[chrishtr, 2014-08-08 16:34:18]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2014-08-08 16:34:43]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/chrishtr@chromium.org/444843002/160001

[chrishtr, 2014-08-08 18:04:14]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2014-08-08 18:06:36]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/chrishtr@chromium.org/444843002/180001

[commit-bot: I haz the power, 2014-08-08 21:31:36]

Change committed as 288438