[Settings] Split non-macro generated parts of Settings into SettingsBase base class
Created attachment 323847 [details] Patch
Attachment 323847 [details] did not pass style-queue: ERROR: Source/WebCore/page/SettingsBase.h:140: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] ERROR: Source/WebCore/page/SettingsBase.h:190: The parameter name "mode" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/page/SettingsBase.cpp:314: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] ERROR: Source/WebCore/page/SettingsBase.cpp:417: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/WebCore/page/SettingsBase.cpp:455: Multi line control clauses should use braces. [whitespace/braces] [4] Total errors found: 5 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 323847 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=323847&action=review > Source/WebCore/page/SettingsBase.cpp:144 > +SettingsBase::SettingsBase(Page* page) Take a reference to a page? > Source/WebCore/page/SettingsBase.cpp:145 > : m_page(nullptr) Initialize in class definition instead of here? > Source/WebCore/page/SettingsBase.cpp:146 > , m_mediaTypeOverride("screen") Initialize in class definition instead of here? > Source/WebCore/page/SettingsBase.cpp:148 > , m_storageBlockingPolicy(SecurityOrigin::AllowAllStorage) Initialize in class definition instead of here? > Source/WebCore/page/SettingsBase.cpp:168 > , m_timeWithoutMouseMovementBeforeHidingControls(3_s) Initialize in class definition instead of here? > Source/WebCore/page/SettingsBase.h:102 > + ~SettingsBase(); I’d think this would need to be virtual, or the reference counting would need to be in the derived class, not the base. Or some other trick to make sure we call ~Settings when we do the last deref.
(In reply to Darin Adler from comment #3) > Comment on attachment 323847 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=323847&action=review > > > Source/WebCore/page/SettingsBase.cpp:144 > > +SettingsBase::SettingsBase(Page* page) > > Take a reference to a page? Alas, we can't, there is caller who passes nullptr in Document for frameless Documents. > > > Source/WebCore/page/SettingsBase.cpp:145 > > : m_page(nullptr) > > Initialize in class definition instead of here? Yup, will fix all of these. > > Source/WebCore/page/SettingsBase.h:102 > > + ~SettingsBase(); > > I’d think this would need to be virtual, or the reference counting would > need to be in the derived class, not the base. Or some other trick to make > sure we call ~Settings when we do the last deref. I'm going to move the RefCounted back to Settings.
Created attachment 323855 [details] Patch
Attachment 323855 [details] did not pass style-queue: ERROR: Source/WebCore/page/SettingsBase.h:140: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] ERROR: Source/WebCore/page/SettingsBase.h:190: The parameter name "mode" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/page/SettingsBase.cpp:311: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] ERROR: Source/WebCore/page/SettingsBase.cpp:414: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/WebCore/page/SettingsBase.cpp:452: Multi line control clauses should use braces. [whitespace/braces] [4] Total errors found: 5 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 323855 [details] Patch Clearing flags on attachment: 323855 Committed r223332: <https://trac.webkit.org/changeset/223332>
All reviewed patches have been landed. Closing bug.
<rdar://problem/35000638>