Bug 26494

Summary: RenderTheme::themeForPage reads from Settings before it has been initialized by WebKit
Product: WebKit Reporter: Adam Roben (:aroben) <aroben>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: kenneth, sfalken
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows XP   
Attachments:
Description Flags
Suggested fix sfalken: review+

Description Adam Roben (:aroben) 2009-06-17 15:12:57 PDT
r44758 added RenderTheme::themeForPage and a call to it from the Page constructor. In RenderThemeSafari.cpp, the themeForPage implementation using Settings to decide which instance of a RenderTheme subclass to return.

But the Page constructor (and thus themeForPage) is called before Settings has been initialized by WebKit. This caused bug 26493, though we've worked around the issue for now. It would be better to fix the design problem here.
Comment 1 Kenneth Rohde Christiansen 2009-06-18 08:35:40 PDT
Wouldn't it make sense to initialize the settings before constructing a page? or is there any reason for not doing so?

WebView (windows) already calls "WebPreferences* sharedPreferences = WebPreferences::sharedStandardPreferences();" before constructing the Page().

Couldn't we add the below before creating the Page?

bool shouldPaintNativeControls;
if (SUCCEEDED(m_preferences->shouldPaintNativeControls(&shouldPaintNativeControls)))
    Settings::setShouldPaintNativeControls(shouldPaintNativeControls)

both functions are static.
Comment 2 Kenneth Rohde Christiansen 2009-06-18 09:02:11 PDT
Created attachment 31497 [details]
Suggested fix
Comment 3 Steve Falkenburg 2009-06-19 22:30:52 PDT
Looking at this again, I'm not sure the fix is correct. Investigating...
Comment 4 Steve Falkenburg 2009-06-19 23:14:53 PDT
OK - tested the patch. It works.
Comment 5 Steve Falkenburg 2009-06-19 23:22:02 PDT
Landed fix in r44890.