Summary: | [chromium] Plumb through flag for enabling partial swap | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jonathan Backer <backer> | ||||||||
Component: | New Bugs | Assignee: | Jonathan Backer <backer> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | cc-bugs, fishd, jamesr, piman, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Jonathan Backer
2011-12-14 10:15:02 PST
Created attachment 119241 [details]
Patch
Comment on attachment 119241 [details]
Patch
LGTM
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API. Created attachment 119251 [details]
Patch
Comment on attachment 119251 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=119251&action=review > Source/WebCore/page/Settings.h:646 > + bool m_partialSwapEnabled : 1; you need to initialize this in the Settings constructor or its value will be undefined and propagate out to the rest of the code. Created attachment 119255 [details]
Patch
Comment on attachment 119255 [details]
Patch
R=me
Comment on attachment 119255 [details] Patch Clearing flags on attachment: 119255 Committed r102817: <http://trac.webkit.org/changeset/102817> All reviewed patches have been landed. Closing bug. Comment on attachment 119255 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=119255&action=review > Source/WebCore/page/Settings.h:510 > void setPerTileDrawingEnabled(bool enabled) { m_perTileDrawingEnabled = enabled; } > bool perTileDrawingEnabled() const { return m_perTileDrawingEnabled; } > > + void setPartialSwapEnabled(bool enabled) { m_partialSwapEnabled = enabled; } > + bool partialSwapEnabled() const { return m_partialSwapEnabled; } Why is this Chromium-specific stuff not #ifdeffed? You're bloating Settings on all platforms. Also I have no idea what "partial swap" is and whether it applies to my platform. We don't have any #if PLATFORM() guards on things in Settings today, even though many of the bits only make sense for a certain platform or set of platforms. We could start, although 1 bit per Page is a pretty darn small cost. Actually, I think for this flag we could keep everything out of Settings (by using the chromium WebKit API's WebSettings interface), but if we want to allow layout tests to set this flag via window.internals I think the bit needs to go on WebCore::Settings. (In reply to comment #12) > Actually, I think for this flag we could keep everything out of Settings (by using the chromium WebKit API's WebSettings interface), but if we want to allow layout tests to set this flag via window.internals I think the bit needs to go on WebCore::Settings. Can't layoutTestController.overridePreference be used for such things? That allows tests to manipulate settings through whatever WebKit API each port provides. |