WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
119903
Consider making WebCore::Settings ref-counted.
https://bugs.webkit.org/show_bug.cgi?id=119903
Summary
Consider making WebCore::Settings ref-counted.
Andreas Kling
Reported
2013-08-16 11:31:36 PDT
If we were to make WebCore::Settings a ref-counted object, it could allow detached Frames (and perhaps also Documents) to access Settings despite not currently having a way to grab at the Page.
Attachments
Snack for EWS
(66.10 KB, patch)
2013-08-16 11:33 PDT
,
Andreas Kling
no flags
Details
Formatted Diff
Diff
Snack for EWS
(73.75 KB, patch)
2013-08-16 11:53 PDT
,
Andreas Kling
webkit-ews
: commit-queue-
Details
Formatted Diff
Diff
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2
(837.48 KB, application/zip)
2013-08-16 13:05 PDT
,
Build Bot
no flags
Details
Coconut for EWS
(78.37 KB, patch)
2013-08-16 14:23 PDT
,
Andreas Kling
gtk-ews
: commit-queue-
Details
Formatted Diff
Diff
Proposed patch
(78.74 KB, patch)
2013-08-16 16:49 PDT
,
Andreas Kling
ggaren
: review+
eflews.bot
: commit-queue-
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
Andreas Kling
Comment 1
2013-08-16 11:33:16 PDT
Created
attachment 208935
[details]
Snack for EWS This is the basic idea. By making Settings ref-counted, Frame can cache a pointer to the Page::settings() instead of always going through the Page. This means that Settings continue to apply to detached Frames, and we can avoid a ton of null checking.
Andreas Kling
Comment 2
2013-08-16 11:53:49 PDT
Created
attachment 208941
[details]
Snack for EWS
WebKit Commit Bot
Comment 3
2013-08-16 11:55:38 PDT
Attachment 208941
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/WebCore.exp.in', u'Source/WebCore/bindings/ScriptControllerBase.cpp', u'Source/WebCore/bindings/js/JSDOMWindowBase.cpp', u'Source/WebCore/bindings/js/JSDOMWindowCustom.cpp', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp', u'Source/WebCore/css/CSSFontSelector.cpp', u'Source/WebCore/css/MediaQueryEvaluator.cpp', u'Source/WebCore/dom/DOMImplementation.cpp', u'Source/WebCore/dom/Document.cpp', u'Source/WebCore/editing/Editor.cpp', u'Source/WebCore/editing/EditorCommand.cpp', u'Source/WebCore/editing/FrameSelection.cpp', u'Source/WebCore/editing/SpellChecker.cpp', u'Source/WebCore/editing/TextCheckingHelper.cpp', u'Source/WebCore/html/HTMLEmbedElement.cpp', u'Source/WebCore/html/HTMLSelectElement.cpp', u'Source/WebCore/html/ImageDocument.cpp', u'Source/WebCore/html/PluginDocument.cpp', u'Source/WebCore/html/canvas/WebGLRenderingContext.cpp', u'Source/WebCore/html/parser/XSSAuditor.cpp', u'Source/WebCore/inspector/InspectorPageAgent.cpp', u'Source/WebCore/loader/DocumentLoader.cpp', u'Source/WebCore/loader/DocumentWriter.cpp', u'Source/WebCore/loader/FrameLoader.cpp', u'Source/WebCore/loader/HistoryController.cpp', u'Source/WebCore/loader/MixedContentChecker.cpp', u'Source/WebCore/loader/SubframeLoader.cpp', u'Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp', u'Source/WebCore/loader/appcache/ApplicationCacheHost.cpp', u'Source/WebCore/loader/cache/CachedImage.cpp', u'Source/WebCore/loader/cache/CachedResourceLoader.cpp', u'Source/WebCore/loader/icon/IconController.cpp', u'Source/WebCore/page/ContextMenuController.cpp', u'Source/WebCore/page/DOMWindow.cpp', u'Source/WebCore/page/DragController.cpp', u'Source/WebCore/page/EventHandler.cpp', u'Source/WebCore/page/FocusController.cpp', u'Source/WebCore/page/Frame.cpp', u'Source/WebCore/page/Frame.h', u'Source/WebCore/page/FrameView.cpp', u'Source/WebCore/page/Navigator.cpp', u'Source/WebCore/page/Page.h', u'Source/WebCore/page/Settings.cpp', u'Source/WebCore/page/Settings.h', u'Source/WebCore/page/SpatialNavigation.cpp', u'Source/WebCore/page/mac/EventHandlerMac.mm', u'Source/WebCore/rendering/RenderBlock.cpp', u'Source/WebCore/rendering/RenderFrameSet.cpp', u'Source/WebCore/rendering/RenderIFrame.cpp', u'Source/WebCore/rendering/RenderLayerBacking.cpp', u'Source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm', u'Source/WebKit/mac/WebCoreSupport/WebFrameNetworkingContext.mm', u'Source/WebKit2/WebProcess/Network/WebResourceLoadScheduler.cpp', u'Source/WebKit2/WebProcess/Plugins/PluginView.cpp', u'Source/WebKit2/WebProcess/WebCoreSupport/mac/WebFrameNetworkingContext.mm']" exit_code: 1 Source/WebCore/css/CSSFontSelector.cpp:379: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/css/CSSFontSelector.cpp:381: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/css/CSSFontSelector.cpp:383: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/css/CSSFontSelector.cpp:385: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/css/CSSFontSelector.cpp:387: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/css/CSSFontSelector.cpp:389: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/css/CSSFontSelector.cpp:391: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 7 in 54 files If any of these errors are false positives, please file a bug against check-webkit-style.
Early Warning System Bot
Comment 4
2013-08-16 12:06:52 PDT
Comment on
attachment 208941
[details]
Snack for EWS
Attachment 208941
[details]
did not pass qt-ews (qt): Output:
http://webkit-queues.appspot.com/results/1464854
Early Warning System Bot
Comment 5
2013-08-16 12:10:05 PDT
Comment on
attachment 208941
[details]
Snack for EWS
Attachment 208941
[details]
did not pass qt-wk2-ews (qt-wk2): Output:
http://webkit-queues.appspot.com/results/1466693
EFL EWS Bot
Comment 6
2013-08-16 12:45:33 PDT
Comment on
attachment 208941
[details]
Snack for EWS
Attachment 208941
[details]
did not pass efl-wk2-ews (efl-wk2): Output:
http://webkit-queues.appspot.com/results/1469607
kov's GTK+ EWS bot
Comment 7
2013-08-16 12:51:04 PDT
Comment on
attachment 208941
[details]
Snack for EWS
Attachment 208941
[details]
did not pass gtk-ews (gtk): Output:
http://webkit-queues.appspot.com/results/1477446
Build Bot
Comment 8
2013-08-16 13:05:45 PDT
Comment on
attachment 208941
[details]
Snack for EWS
Attachment 208941
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/1465777
New failing tests: editing/selection/verify-editing-behavior-for-line-granularity.html editing/pasteboard/emacs-ctrl-a-k-y.html fast/forms/textarea-arrow-navigation.html editing/selection/caret-at-end-of-text-line-followed-by-empty-block-in-vertical-mode.html editing/pasteboard/emacs-ctrl-k-with-move.html editing/selection/4947387.html editing/pasteboard/paste-sanitize-crash-1.html editing/pasteboard/paste-sanitize-crash-2.html editing/deleting/delete-to-end-of-paragraph.html fast/events/selectstart-by-arrow-keys-prevent-default.html editing/input/editable-container-with-word-wrap-normal.html http/tests/inspector/inspect-element.html editing/selection/move-begin-end.html
Build Bot
Comment 9
2013-08-16 13:05:48 PDT
Created
attachment 208948
[details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-13 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.4
Andreas Kling
Comment 10
2013-08-16 14:23:05 PDT
Created
attachment 208958
[details]
Coconut for EWS
kov's GTK+ EWS bot
Comment 11
2013-08-16 16:26:40 PDT
Comment on
attachment 208958
[details]
Coconut for EWS
Attachment 208958
[details]
did not pass gtk-ews (gtk): Output:
http://webkit-queues.appspot.com/results/1467698
Andreas Kling
Comment 12
2013-08-16 16:49:08 PDT
Created
attachment 208965
[details]
Proposed patch
WebKit Commit Bot
Comment 13
2013-08-16 16:51:38 PDT
Attachment 208965
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/WebCore.exp.in', u'Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp', u'Source/WebCore/bindings/ScriptControllerBase.cpp', u'Source/WebCore/bindings/js/JSDOMWindowBase.cpp', u'Source/WebCore/bindings/js/JSDOMWindowCustom.cpp', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp', u'Source/WebCore/css/CSSFontSelector.cpp', u'Source/WebCore/css/MediaQueryEvaluator.cpp', u'Source/WebCore/dom/DOMImplementation.cpp', u'Source/WebCore/dom/Document.cpp', u'Source/WebCore/editing/Editor.cpp', u'Source/WebCore/editing/EditorCommand.cpp', u'Source/WebCore/editing/FrameSelection.cpp', u'Source/WebCore/editing/SpellChecker.cpp', u'Source/WebCore/editing/TextCheckingHelper.cpp', u'Source/WebCore/html/HTMLEmbedElement.cpp', u'Source/WebCore/html/HTMLSelectElement.cpp', u'Source/WebCore/html/ImageDocument.cpp', u'Source/WebCore/html/PluginDocument.cpp', u'Source/WebCore/html/canvas/WebGLRenderingContext.cpp', u'Source/WebCore/html/parser/XSSAuditor.cpp', u'Source/WebCore/inspector/InspectorPageAgent.cpp', u'Source/WebCore/loader/DocumentLoader.cpp', u'Source/WebCore/loader/DocumentWriter.cpp', u'Source/WebCore/loader/FrameLoader.cpp', u'Source/WebCore/loader/HistoryController.cpp', u'Source/WebCore/loader/MixedContentChecker.cpp', u'Source/WebCore/loader/SubframeLoader.cpp', u'Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp', u'Source/WebCore/loader/appcache/ApplicationCacheHost.cpp', u'Source/WebCore/loader/cache/CachedImage.cpp', u'Source/WebCore/loader/cache/CachedResourceLoader.cpp', u'Source/WebCore/loader/icon/IconController.cpp', u'Source/WebCore/page/ContextMenuController.cpp', u'Source/WebCore/page/DOMWindow.cpp', u'Source/WebCore/page/DragController.cpp', u'Source/WebCore/page/EventHandler.cpp', u'Source/WebCore/page/FocusController.cpp', u'Source/WebCore/page/Frame.cpp', u'Source/WebCore/page/Frame.h', u'Source/WebCore/page/FrameView.cpp', u'Source/WebCore/page/Navigator.cpp', u'Source/WebCore/page/Page.h', u'Source/WebCore/page/Settings.cpp', u'Source/WebCore/page/Settings.h', u'Source/WebCore/page/SpatialNavigation.cpp', u'Source/WebCore/page/mac/EventHandlerMac.mm', u'Source/WebCore/rendering/RenderBlock.cpp', u'Source/WebCore/rendering/RenderFrameSet.cpp', u'Source/WebCore/rendering/RenderIFrame.cpp', u'Source/WebCore/rendering/RenderLayerBacking.cpp', u'Source/WebKit/efl/WebCoreSupport/EditorClientEfl.cpp', u'Source/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp', u'Source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm', u'Source/WebKit/mac/WebCoreSupport/WebFrameNetworkingContext.mm', u'Source/WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp', u'Source/WebKit/win/WebCoreSupport/WebFrameNetworkingContext.cpp', u'Source/WebKit/wince/WebCoreSupport/EditorClientWinCE.cpp', u'Source/WebKit2/WebProcess/Network/WebResourceLoadScheduler.cpp', u'Source/WebKit2/WebProcess/Plugins/PluginView.cpp', u'Source/WebKit2/WebProcess/WebCoreSupport/mac/WebFrameNetworkingContext.mm', u'Source/WebKit2/WebProcess/WebCoreSupport/soup/WebFrameNetworkingContext.cpp']" exit_code: 1 Source/WebCore/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Total errors found: 1 in 62 files If any of these errors are false positives, please file a bug against check-webkit-style.
Geoffrey Garen
Comment 14
2013-08-16 17:59:50 PDT
Comment on
attachment 208965
[details]
Proposed patch View in context:
https://bugs.webkit.org/attachment.cgi?id=208965&action=review
r=me
> Source/WebCore/html/HTMLEmbedElement.cpp:196 > + // Workaround for <
rdar://problem/6642221
>. > + if (frame->settings().usesDashboardBackwardCompatibilityMode()) > + return true;
My favorite of all the settings.
> Source/WebCore/page/Frame.h:144 > + Settings& settings() const { return *m_settings; }
Should this be const Settings&? I don't think any clients want the ability to change the settings inside WebCore.
EFL EWS Bot
Comment 15
2013-08-16 22:40:33 PDT
Comment on
attachment 208965
[details]
Proposed patch
Attachment 208965
[details]
did not pass efl-ews (efl): Output:
http://webkit-queues.appspot.com/results/1393035
Andreas Kling
Comment 16
2013-08-17 03:53:58 PDT
(In reply to
comment #14
)
> (From update of
attachment 208965
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=208965&action=review
> > r=me
Thanks!
> > Source/WebCore/page/Frame.h:144 > > + Settings& settings() const { return *m_settings; } > > Should this be const Settings&? I don't think any clients want the ability to change the settings inside WebCore.
Inspector changes some stuff. :|
Andreas Kling
Comment 17
2013-09-06 18:45:34 PDT
This was
http://trac.webkit.org/r154219
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug