Revert of Cache the current WebPreferences on RenderViewHostImpl. (ht…

…tps://codereview.chromium.org/373323003/) Reason for revert: Causes DOA on 38-dev. https://code.google.com/p/chromium/issues/detail?id=398751 Original issue's description: > Cache the current WebPreferences on RenderViewHostImpl. > > 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. > > BUG=390799 > > Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=284592 TBR=jam, michaelbai, chrishtr, avi NOTREECHECKS=true NOTRY=true BUG=390799 Review URL: https://codereview.chromium.org/429413003 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@286876 0039d316-1c4b-4281-b951-d872f2087c98
marcosholgado · Jul 31, 2014 · 72dfa07 · 72dfa07
1 parent 8fed76e
commit 72dfa07
Show file tree

Hide file tree

Showing 22 changed files with 33 additions and 72 deletions.
diff --git a/android_webview/native/aw_settings.cc b/android_webview/native/aw_settings.cc
@@ -158,7 +158,8 @@ void AwSettings::UpdateWebkitPreferencesLocked(JNIEnv* env, jobject obj) {
   content::RenderViewHost* render_view_host =
       web_contents()->GetRenderViewHost();
   if (!render_view_host) return;
-  render_view_host->OnWebkitPreferencesChanged();
+  render_view_host->UpdateWebkitPreferences(
+      render_view_host->GetWebkitPreferences());
 }
 
 void AwSettings::UpdateInitialPageScaleLocked(JNIEnv* env, jobject obj) {

diff --git a/chrome/browser/android/voice_search_tab_helper.cc b/chrome/browser/android/voice_search_tab_helper.cc
@@ -32,8 +32,6 @@ static void UpdateAutoplayStatus(JNIEnv* env,
   if (!prefs.user_gesture_required_for_media_playback)
     return;
 
-  // 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);

diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
@@ -2524,8 +2524,6 @@ void ChromeContentBrowserClient::OverrideWebkitPrefs(
   }
   DCHECK(!web_prefs->default_encoding.empty());
 
-  // Note: it's not posible for kExtensionsScheme to change during the lifetime
-  // of the process.
   WebContents* web_contents = WebContents::FromRenderViewHost(rvh);
   extensions::ViewType view_type = extensions::GetViewType(web_contents);
   ExtensionService* service =

diff --git a/chrome/browser/prefs/chrome_pref_service_unittest.cc b/chrome/browser/prefs/chrome_pref_service_unittest.cc
@@ -117,7 +117,7 @@ class ChromePrefServiceWebKitPrefs : public ChromeRenderViewHostTestHarness {
 // to a WebPreferences object.
 TEST_F(ChromePrefServiceWebKitPrefs, PrefsCopied) {
   WebPreferences webkit_prefs =
-      WebContentsTester::For(web_contents())->TestComputeWebkitPrefs();
+      WebContentsTester::For(web_contents())->TestGetWebkitPrefs();
 
   // These values have been overridden by the profile preferences.
   EXPECT_EQ("UTF-8", webkit_prefs.default_encoding);

diff --git a/chrome/browser/printing/print_dialog_cloud.cc b/chrome/browser/printing/print_dialog_cloud.cc
@@ -361,8 +361,6 @@ void CloudPrintFlowHandler::Observe(
         // script permissions required for the web UI.
         RenderViewHost* rvh = web_ui()->GetWebContents()->GetRenderViewHost();
         if (rvh) {
-          // TODO(chrishtr): this is wrong. allow_scripts_to_close_windows will
-          // be reset the next time a preference changes.
           WebPreferences webkit_prefs = rvh->GetWebkitPreferences();
           webkit_prefs.allow_scripts_to_close_windows = true;
           rvh->UpdateWebkitPreferences(webkit_prefs);

diff --git a/chrome/browser/ui/prefs/prefs_tab_helper.cc b/chrome/browser/ui/prefs/prefs_tab_helper.cc
@@ -49,31 +49,23 @@ namespace {
 // The list of prefs we want to observe.
 const char* kPrefsToObserve[] = {
   prefs::kDefaultCharset,
-  prefs::kDisable3DAPIs,
-  prefs::kEnableHyperlinkAuditing,
   prefs::kWebKitAllowDisplayingInsecureContent,
   prefs::kWebKitAllowRunningInsecureContent,
   prefs::kWebKitDefaultFixedFontSize,
   prefs::kWebKitDefaultFontSize,
-  prefs::kWebKitDomPasteEnabled,
 #if defined(OS_ANDROID)
   prefs::kWebKitFontScaleFactor,
   prefs::kWebKitForceEnableZoom,
   prefs::kWebKitPasswordEchoEnabled,
 #endif
-  prefs::kWebKitInspectorSettings,
-  prefs::kWebKitJavascriptCanOpenWindowsAutomatically,
   prefs::kWebKitJavascriptEnabled,
   prefs::kWebKitJavaEnabled,
   prefs::kWebKitLoadsImagesAutomatically,
   prefs::kWebKitMinimumFontSize,
   prefs::kWebKitMinimumLogicalFontSize,
   prefs::kWebKitPluginsEnabled,
-  prefs::kWebKitShrinksStandaloneImagesToFit,
   prefs::kWebkitTabsToLinks,
-  prefs::kWebKitTextAreasAreResizable,
-  prefs::kWebKitUsesUniversalDetector,
-  prefs::kWebKitWebSecurityEnabled,
+  prefs::kWebKitUsesUniversalDetector
 };
 
 const int kPrefsToObserveLength = arraysize(kPrefsToObserve);
@@ -120,7 +112,6 @@ ALL_FONT_SCRIPTS(WEBKIT_WEBPREFS_FONTS_STANDARD)
     }
   }
 }
-#endif  // !defined(OS_ANDROID)
 
 // Registers |obs| to observe per-script font prefs under the path |map_name|.
 // On android, there's no exposed way to change these prefs, so we can save
@@ -137,6 +128,7 @@ void RegisterFontFamilyMapObserver(
     registrar->Add(pref_name.c_str(), obs);
   }
 }
+#endif  // !defined(OS_ANDROID)
 
 #if defined(OS_WIN)
 // On Windows with antialising we want to use an alternate fixed font like
@@ -334,9 +326,12 @@ PrefsTabHelper::PrefsTabHelper(WebContents* contents)
         &PrefsTabHelper::OnWebPrefChanged, base::Unretained(this));
     for (int i = 0; i < kPrefsToObserveLength; ++i) {
       const char* pref_name = kPrefsToObserve[i];
+      DCHECK(std::string(pref_name) == prefs::kDefaultCharset ||
+             StartsWithASCII(pref_name, "webkit.webprefs.", true));
       pref_change_registrar_.Add(pref_name, webkit_callback);
     }
 
+#if !defined(OS_ANDROID)
     RegisterFontFamilyMapObserver(&pref_change_registrar_,
                                   prefs::kWebKitStandardFontFamilyMap,
                                   webkit_callback);
@@ -358,6 +353,7 @@ PrefsTabHelper::PrefsTabHelper(WebContents* contents)
     RegisterFontFamilyMapObserver(&pref_change_registrar_,
                                   prefs::kWebKitPictographFontFamilyMap,
                                   webkit_callback);
+#endif  // !defined(OS_ANDROID)
   }
 
   renderer_preferences_util::UpdateFromSystemSettings(
@@ -580,7 +576,7 @@ Profile* PrefsTabHelper::GetProfile() {
   return Profile::FromBrowserContext(web_contents_->GetBrowserContext());
 }
 
-void PrefsTabHelper::OnFontFamilyPrefChanged(const std::string& pref_name) {
+void PrefsTabHelper::OnWebPrefChanged(const std::string& pref_name) {
   // When a font family pref's value goes from non-empty to the empty string, we
   // must add it to the usual WebPreferences struct passed to the renderer.
   //
@@ -608,12 +604,6 @@ void PrefsTabHelper::OnFontFamilyPrefChanged(const std::string& pref_name) {
       return;
     }
   }
-}
-
-void PrefsTabHelper::OnWebPrefChanged(const std::string& pref_name) {
-#if !defined(OS_ANDROID)
-  OnFontFamilyPrefChanged(pref_name);
-#endif
 
-  web_contents_->GetRenderViewHost()->OnWebkitPreferencesChanged();
+  UpdateWebPreferences();
 }
diff --git a/chrome/browser/ui/prefs/prefs_tab_helper.h b/chrome/browser/ui/prefs/prefs_tab_helper.h
@@ -51,8 +51,6 @@ class PrefsTabHelper : public content::NotificationObserver,
   void UpdateRendererPreferences();
 
   Profile* GetProfile();
-
-  void OnFontFamilyPrefChanged(const std::string& pref_name);
   void OnWebPrefChanged(const std::string& pref_name);
 
   content::WebContents* web_contents_;

diff --git a/content/browser/android/content_settings.cc b/content/browser/android/content_settings.cc
@@ -39,7 +39,7 @@ bool ContentSettings::GetJavaScriptEnabled(JNIEnv* env, jobject obj) {
   RenderViewHost* render_view_host = web_contents()->GetRenderViewHost();
   if (!render_view_host)
     return false;
-  return render_view_host->GetWebkitPreferences().javascript_enabled;
+  return render_view_host->GetDelegate()->GetWebkitPrefs().javascript_enabled;
 }
 
 void ContentSettings::WebContentsDestroyed() {

diff --git a/content/browser/browser_plugin/browser_plugin_guest.cc b/content/browser/browser_plugin/browser_plugin_guest.cc
@@ -223,10 +223,7 @@ void BrowserPluginGuest::Initialize(
       new BrowserPluginMsg_GuestContentWindowReady(instance_id_,
                                                    guest_routing_id));
 
-  // TODO(chrishtr): this code is wrong. The navigate_on_drag_drop field will
-  // be reset again the next time preferences are updated.
-  WebPreferences prefs =
-      GetWebContents()->GetRenderViewHost()->GetWebkitPreferences();
+  WebPreferences prefs = GetWebContents()->GetWebkitPrefs();
   prefs.navigate_on_drag_drop = false;
   GetWebContents()->GetRenderViewHost()->UpdateWebkitPreferences(prefs);
 

diff --git a/content/browser/frame_host/interstitial_page_impl.cc b/content/browser/frame_host/interstitial_page_impl.cc
@@ -509,11 +509,11 @@ RendererPreferences InterstitialPageImpl::GetRendererPrefs(
   return renderer_preferences_;
 }
 
-WebPreferences InterstitialPageImpl::ComputeWebkitPrefs() {
+WebPreferences InterstitialPageImpl::GetWebkitPrefs() {
   if (!enabled())
     return WebPreferences();
 
-  return render_view_host_->ComputeWebkitPrefs(url_);
+  return render_view_host_->GetWebkitPrefs(url_);
 }
 
 void InterstitialPageImpl::RenderWidgetDeleted(

diff --git a/content/browser/frame_host/interstitial_page_impl.h b/content/browser/frame_host/interstitial_page_impl.h
@@ -131,7 +131,7 @@ class CONTENT_EXPORT InterstitialPageImpl
                                     int error_code) OVERRIDE;
   virtual RendererPreferences GetRendererPrefs(
       BrowserContext* browser_context) const OVERRIDE;
-  virtual WebPreferences ComputeWebkitPrefs() OVERRIDE;
+  virtual WebPreferences GetWebkitPrefs() OVERRIDE;
   virtual gfx::Rect GetRootWindowResizerRect() const OVERRIDE;
   virtual void CreateNewWindow(
       int render_process_id,

diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc
@@ -2123,7 +2123,7 @@ void RenderProcessHostImpl::OnGpuSwitching() {
       continue;
 
     RenderViewHost* rvh = RenderViewHost::From(widget);
-    rvh->OnWebkitPreferencesChanged();
+    rvh->UpdateWebkitPreferences(rvh->GetWebkitPreferences());
   }
 }
 

diff --git a/content/browser/renderer_host/render_view_host_delegate.cc b/content/browser/renderer_host/render_view_host_delegate.cc
@@ -22,7 +22,7 @@ WebContents* RenderViewHostDelegate::GetAsWebContents() {
   return NULL;
 }
 
-WebPreferences RenderViewHostDelegate::ComputeWebkitPrefs() {
+WebPreferences RenderViewHostDelegate::GetWebkitPrefs() {
   return WebPreferences();
 }
 

diff --git a/content/browser/renderer_host/render_view_host_delegate.h b/content/browser/renderer_host/render_view_host_delegate.h
@@ -139,9 +139,9 @@ class CONTENT_EXPORT RenderViewHostDelegate {
   virtual RendererPreferences GetRendererPrefs(
       BrowserContext* browser_context) const = 0;
 
-  // Computes a WebPreferences object that will be used by the renderer
+  // Returns a WebPreferences object that will be used by the renderer
   // associated with the owning render view host.
-  virtual WebPreferences ComputeWebkitPrefs();
+  virtual WebPreferences GetWebkitPrefs();
 
   // Notification the user has made a gesture while focus was on the
   // page. This is used to avoid uninitiated user downloads (aka carpet

diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc
@@ -289,7 +289,7 @@ bool RenderViewHostImpl::CreateRenderView(
   ViewMsg_New_Params params;
   params.renderer_preferences =
       delegate_->GetRendererPrefs(GetProcess()->GetBrowserContext());
-  params.web_preferences = GetWebkitPreferences();
+  params.web_preferences = delegate_->GetWebkitPrefs();
   params.view_id = GetRoutingID();
   params.main_frame_routing_id = main_frame_routing_id_;
   params.surface_id = surface_id();
@@ -329,7 +329,7 @@ void RenderViewHostImpl::SyncRendererPrefs() {
                                         GetProcess()->GetBrowserContext())));
 }
 
-WebPreferences RenderViewHostImpl::ComputeWebkitPrefs(const GURL& url) {
+WebPreferences RenderViewHostImpl::GetWebkitPrefs(const GURL& url) {
   TRACE_EVENT0("browser", "RenderViewHostImpl::GetWebkitPrefs");
   WebPreferences prefs;
 
@@ -1418,10 +1418,7 @@ void RenderViewHostImpl::ExitFullscreen() {
 }
 
 WebPreferences RenderViewHostImpl::GetWebkitPreferences() {
-  if (!web_preferences_.get()) {
-    OnWebkitPreferencesChanged();
-  }
-  return *web_preferences_;
+  return delegate_->GetWebkitPrefs();
 }
 
 void RenderViewHostImpl::DisownOpener() {
@@ -1432,14 +1429,9 @@ void RenderViewHostImpl::DisownOpener() {
 }
 
 void RenderViewHostImpl::UpdateWebkitPreferences(const WebPreferences& prefs) {
-  web_preferences_.reset(new WebPreferences(prefs));
   Send(new ViewMsg_UpdateWebPreferences(GetRoutingID(), prefs));
 }
 
-void RenderViewHostImpl::OnWebkitPreferencesChanged() {
-  UpdateWebkitPreferences(delegate_->ComputeWebkitPrefs());
-}
-
 void RenderViewHostImpl::GetAudioOutputControllers(
     const GetAudioOutputControllersCallback& callback) const {
   AudioRendererHost* audio_host =

diff --git a/content/browser/renderer_host/render_view_host_impl.h b/content/browser/renderer_host/render_view_host_impl.h
@@ -204,7 +204,6 @@ class CONTENT_EXPORT RenderViewHostImpl
   virtual WebPreferences GetWebkitPreferences() OVERRIDE;
   virtual void UpdateWebkitPreferences(
       const WebPreferences& prefs) OVERRIDE;
-  virtual void OnWebkitPreferencesChanged() OVERRIDE;
   virtual void GetAudioOutputControllers(
       const GetAudioOutputControllersCallback& callback) const OVERRIDE;
   virtual void SelectWordAroundCaret() OVERRIDE;
@@ -242,7 +241,7 @@ class CONTENT_EXPORT RenderViewHostImpl
   }
 
   // Returns the content specific prefs for this RenderViewHost.
-  WebPreferences ComputeWebkitPrefs(const GURL& url);
+  WebPreferences GetWebkitPrefs(const GURL& url);
 
   // Sends the given navigation message. Use this rather than sending it
   // yourself since this does the internal bookkeeping described below. This
@@ -608,11 +607,6 @@ class CONTENT_EXPORT RenderViewHostImpl
   // True if the current focused element is editable.
   bool is_focused_element_editable_;
 
-  // This is updated every time UpdateWebkitPreferences is called. That method
-  // is in turn called when any of the settings change that the WebPreferences
-  // values depend on.
-  scoped_ptr<WebPreferences> web_preferences_;
-
   DISALLOW_COPY_AND_ASSIGN(RenderViewHostImpl);
 };
 

diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
@@ -3846,15 +3846,15 @@ bool WebContentsImpl::AddMessageToConsole(int32 level,
                                         source_id);
 }
 
-WebPreferences WebContentsImpl::ComputeWebkitPrefs() {
+WebPreferences WebContentsImpl::GetWebkitPrefs() {
   // We want to base the page config off of the actual URL, rather than the
   // virtual URL.
   // TODO(nasko): Investigate how to remove the GetActiveEntry usage here,
   // as it is deprecated and can be out of sync with GetRenderViewHost().
   GURL url = controller_.GetActiveEntry()
       ? controller_.GetActiveEntry()->GetURL() : GURL::EmptyGURL();
 
-  return GetRenderManager()->current_host()->ComputeWebkitPrefs(url);
+  return GetRenderManager()->current_host()->GetWebkitPrefs(url);
 }
 
 int WebContentsImpl::CreateSwappedOutRenderView(

diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h
@@ -417,7 +417,7 @@ class CONTENT_EXPORT WebContentsImpl
                                    const base::string16& source_id) OVERRIDE;
   virtual RendererPreferences GetRendererPrefs(
       BrowserContext* browser_context) const OVERRIDE;
-  virtual WebPreferences ComputeWebkitPrefs() OVERRIDE;
+  virtual WebPreferences GetWebkitPrefs() OVERRIDE;
   virtual void OnUserGesture() OVERRIDE;
   virtual void OnIgnoredUIEvent() OVERRIDE;
   virtual void RendererUnresponsive(RenderViewHost* render_view_host,

diff --git a/content/public/browser/render_view_host.h b/content/public/browser/render_view_host.h
@@ -194,14 +194,9 @@ class CONTENT_EXPORT RenderViewHost : virtual public RenderWidgetHost {
   // RenderViewHostDelegate.
   virtual void SyncRendererPrefs() = 0;
 
-  // Returns the current WebKit preferences. Note: WebPreferences is cached, so
-  // this lookup will be fast
+  // Returns the current WebKit preferences.
   virtual WebPreferences GetWebkitPreferences() = 0;
 
-  // If any state that affects the webkit preferences changed, this method must
-  // be called. This triggers recomputing preferences.
-  virtual void OnWebkitPreferencesChanged() = 0;
-
   // Passes a list of Webkit preferences to the renderer.
   virtual void UpdateWebkitPreferences(const WebPreferences& prefs) = 0;
 

diff --git a/content/public/test/web_contents_tester.h b/content/public/test/web_contents_tester.h
@@ -86,8 +86,8 @@ class WebContentsTester {
       const Referrer& referrer,
       PageTransition transition) = 0;
 
-  // Promote ComputeWebkitPrefs to public.
-  virtual WebPreferences TestComputeWebkitPrefs() = 0;
+  // Promote GetWebkitPrefs to public.
+  virtual WebPreferences TestGetWebkitPrefs() = 0;
 };
 
 }  // namespace content

diff --git a/content/test/test_web_contents.cc b/content/test/test_web_contents.cc
@@ -91,8 +91,8 @@ void TestWebContents::TestDidNavigateWithReferrer(
   frame_tree_.root()->navigator()->DidNavigate(rfh, params);
 }
 
-WebPreferences TestWebContents::TestComputeWebkitPrefs() {
-  return ComputeWebkitPrefs();
+WebPreferences TestWebContents::TestGetWebkitPrefs() {
+  return GetWebkitPrefs();
 }
 
 bool TestWebContents::CreateRenderViewForRenderManager(

diff --git a/content/test/test_web_contents.h b/content/test/test_web_contents.h
@@ -42,7 +42,7 @@ class TestWebContents : public WebContentsImpl, public WebContentsTester {
                                            const GURL& url,
                                            const Referrer& referrer,
                                            PageTransition transition) OVERRIDE;
-  virtual WebPreferences TestComputeWebkitPrefs() OVERRIDE;
+  virtual WebPreferences TestGetWebkitPrefs() OVERRIDE;
 
   TestRenderViewHost* pending_test_rvh() const;