diff options
author | erikchen@chromium.org <erikchen@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-07-31 20:00:17 +0000 |
---|---|---|
committer | erikchen@chromium.org <erikchen@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-07-31 20:00:17 +0000 |
commit | 72dfa077d95c8889cbb008fc14021d283bfeef6c (patch) | |
tree | ff519fdb0ee56707936be28402c289d31a4f0d07 | |
parent | 8fed76e91b6c8406b30af0c0dbc01314cf2bd1db (diff) | |
download | chromium_src-72dfa077d95c8889cbb008fc14021d283bfeef6c.zip chromium_src-72dfa077d95c8889cbb008fc14021d283bfeef6c.tar.gz chromium_src-72dfa077d95c8889cbb008fc14021d283bfeef6c.tar.bz2 |
Revert of Cache the current WebPreferences on RenderViewHostImpl. (https://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
22 files changed, 33 insertions, 72 deletions
diff --git a/android_webview/native/aw_settings.cc b/android_webview/native/aw_settings.cc index 2a0a50b..e9ba10e 100644 --- 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 index 52f0c36..c8f10db 100644 --- 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 index 8a174a6..87b4810 100644 --- 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 index 4b64946..67a77a4 100644 --- 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 index 44e6c6d..bf08701 100644 --- 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 index bc9b08a..0079bf9 100644 --- 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 index f97410d..055a2f1 100644 --- 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 index 2d32143..7ab181b 100644 --- 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 index 1da2fdd..ef729e4 100644 --- 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 index f2cac9c..1a1ed43 100644 --- 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 index 1305eea..7f2d4d7 100644 --- 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 index 86875b4..7218e14 100644 --- 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 index 41a110e..9c0f17f 100644 --- 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 index 8c453a3..4a44123 100644 --- 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 index 06b993c..0be391e 100644 --- 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 index 250204e..e2a9764 100644 --- 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 index a0bf9b3..9c40100 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc @@ -3846,7 +3846,7 @@ 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, @@ -3854,7 +3854,7 @@ WebPreferences WebContentsImpl::ComputeWebkitPrefs() { 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 index fd69e98..aee4309 100644 --- 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 index 2c4c25f..840be8c 100644 --- 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 index c3b52fc..73ccb7e 100644 --- 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 index 19d5418..c337e2b 100644 --- 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 index 8181b1c..5061bfb 100644 --- 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; |