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+

Adam Roben (:aroben)
Reported 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.
Attachments
Suggested fix (2.34 KB, patch)
2009-06-18 09:02 PDT, Kenneth Rohde Christiansen 		sfalken: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Kenneth Rohde Christiansen
Comment 1 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.
Kenneth Rohde Christiansen
Comment 2 2009-06-18 09:02:11 PDT
Created attachment 31497 [details] Suggested fix
Steve Falkenburg
Comment 3 2009-06-19 22:30:52 PDT
Looking at this again, I'm not sure the fix is correct. Investigating...
Steve Falkenburg
Comment 4 2009-06-19 23:14:53 PDT
OK - tested the patch. It works.
Steve Falkenburg
Comment 5 2009-06-19 23:22:02 PDT
Landed fix in r44890.
Note You need to log in before you can comment on or make changes to this bug.