Bug 26494 - RenderTheme::themeForPage reads from Settings before it has been initialized by WebKit
Summary: RenderTheme::themeForPage reads from Settings before it has been initialized ...
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Nobody
Depends on:
Reported: 2009-06-17 15:12 PDT by Adam Roben (:aroben)
Modified: 2009-06-19 23:22 PDT (History)
2 users (show)

See Also:

Suggested fix (2.34 KB, patch)
2009-06-18 09:02 PDT, Kenneth Rohde Christiansen
sfalken: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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)))

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.