RESOLVED FIXED144082
[Mac] Disable QTKit by default on future OS X.
https://bugs.webkit.org/show_bug.cgi?id=144082
Summary [Mac] Disable QTKit by default on future OS X.
Jer Noble
Reported 2015-04-22 17:26:10 PDT
[Mac] Disable QTKit by default on future OS X.
Attachments
Patch (7.47 KB, patch)
2015-04-22 17:31 PDT, Jer Noble
darin: review+
Patch for landing (6.21 KB, patch)
2015-04-23 10:49 PDT, Jer Noble
no flags
Patch for landing (6.75 KB, patch)
2015-04-23 10:57 PDT, Jer Noble
no flags
Jer Noble
Comment 1 2015-04-22 17:31:08 PDT
Darin Adler
Comment 2 2015-04-22 18:06:29 PDT
Comment on attachment 251389 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=251389&action=review Can we find a way to not have this in three different places!? > Source/WTF/wtf/Platform.h:1007 > +#define WTF_USE_QTKIT 1 This is a strange use of USE(QTKIT). If it’s false, then you default to not using QTKit, but can turn it on at runtime. That’s not normally what our USE macros mean. > Source/WebCore/page/Settings.cpp:80 > -#if PLATFORM(COCOA) > +#if USE(QTKIT) > bool Settings::gQTKitEnabled = true; > +#else > +bool Settings::gQTKitEnabled = false; > #endif Still need to wrap this in #if PLATFORM(COCOA)
Jer Noble
Comment 3 2015-04-22 20:43:39 PDT
(In reply to comment #2) > Comment on attachment 251389 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=251389&action=review > > Can we find a way to not have this in three different places!? I'd love to, but WebCore, WebKit, and WebKit2 each have their own definitions for their respective default settings. > > Source/WTF/wtf/Platform.h:1007 > > +#define WTF_USE_QTKIT 1 > > This is a strange use of USE(QTKIT). If it’s false, then you default to not > using QTKit, but can turn it on at runtime. That’s not normally what our USE > macros mean. I realize; this is just the first step. The next step is to actually disable the implementation. Meanwhile, having this USE macro means not having to repeat the same platform macro everywhere in each of the settings defaults. I'm open to other ideas though. > > Source/WebCore/page/Settings.cpp:80 > > -#if PLATFORM(COCOA) > > +#if USE(QTKIT) > > bool Settings::gQTKitEnabled = true; > > +#else > > +bool Settings::gQTKitEnabled = false; > > #endif > > Still need to wrap this in #if PLATFORM(COCOA) Oh, good point.
Jer Noble
Comment 4 2015-04-23 10:49:49 PDT
Created attachment 251454 [details] Patch for landing Got rid of WTF_USE_WEBKIT and managed to have the WebKit and WebKit2 preference defaults derived from the WebCore ones.
Jer Noble
Comment 5 2015-04-23 10:57:09 PDT
Created attachment 251455 [details] Patch for landing
WebKit Commit Bot
Comment 6 2015-04-23 15:44:48 PDT
Comment on attachment 251455 [details] Patch for landing Clearing flags on attachment: 251455 Committed r183221: <http://trac.webkit.org/changeset/183221>
Jon Lee
Comment 7 2015-04-27 00:28:54 PDT
Alexey Proskuryakov
Comment 8 2015-05-01 10:33:20 PDT
As discussed in person, we probably need more tests updated, see <rdar://problem/20527672>.
Note You need to log in before you can comment on or make changes to this bug.