WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
143743
WebKitTestRunner should use modern API on OS X and iOS
https://bugs.webkit.org/show_bug.cgi?id=143743
Summary
WebKitTestRunner should use modern API on OS X and iOS
Enrica Casucci
Reported
2015-04-14 17:14:29 PDT
We should adopt the modern API on Apple platforms.
Attachments
Patch
(36.59 KB, patch)
2015-04-14 17:19 PDT
,
Enrica Casucci
no flags
Details
Formatted Diff
Diff
Patch2
(36.98 KB, patch)
2015-04-15 13:09 PDT
,
Enrica Casucci
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews105 for mac-mavericks-wk2
(503.36 KB, application/zip)
2015-04-15 13:24 PDT
,
Build Bot
no flags
Details
Patch
(43.30 KB, patch)
2015-05-01 02:38 PDT
,
Tim Horton
no flags
Details
Formatted Diff
Diff
Patch
(43.40 KB, patch)
2015-05-01 03:41 PDT
,
Tim Horton
no flags
Details
Formatted Diff
Diff
Patch
(46.31 KB, patch)
2015-05-01 05:54 PDT
,
Tim Horton
no flags
Details
Formatted Diff
Diff
Patch
(56.86 KB, patch)
2015-05-01 23:06 PDT
,
Tim Horton
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews105 for mac-mavericks-wk2
(301.02 KB, application/zip)
2015-05-01 23:40 PDT
,
Build Bot
no flags
Details
Patch
(56.88 KB, patch)
2015-05-01 23:45 PDT
,
Tim Horton
buildbot
: commit-queue-
Details
Formatted Diff
Diff
Archive of layout-test-results from ews105 for mac-mavericks-wk2
(1.02 MB, application/zip)
2015-05-02 00:51 PDT
,
Build Bot
no flags
Details
patch
(76.13 KB, patch)
2015-08-15 04:31 PDT
,
Tim Horton
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews104 for mac-mavericks-wk2
(1.10 MB, application/zip)
2015-08-15 05:34 PDT
,
Build Bot
no flags
Details
Patch
(68.99 KB, patch)
2015-08-18 16:36 PDT
,
Tim Horton
no flags
Details
Formatted Diff
Diff
Patch
(69.09 KB, patch)
2015-08-18 17:34 PDT
,
Tim Horton
no flags
Details
Formatted Diff
Diff
Patch
(84.24 KB, patch)
2015-08-18 23:46 PDT
,
Tim Horton
no flags
Details
Formatted Diff
Diff
Patch
(84.28 KB, patch)
2015-08-19 00:00 PDT
,
Tim Horton
no flags
Details
Formatted Diff
Diff
Patch
(83.58 KB, patch)
2015-08-20 18:49 PDT
,
Tim Horton
no flags
Details
Formatted Diff
Diff
Patch
(83.51 KB, patch)
2015-08-20 19:20 PDT
,
Tim Horton
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews107 for mac-mavericks-wk2
(715.72 KB, application/zip)
2015-08-20 20:15 PDT
,
Build Bot
no flags
Details
Patch
(56.90 KB, patch)
2015-08-21 15:54 PDT
,
Tim Horton
andersca
: review+
Details
Formatted Diff
Diff
Show Obsolete
(18)
View All
Add attachment
proposed patch, testcase, etc.
Enrica Casucci
Comment 1
2015-04-14 17:19:51 PDT
Created
attachment 250763
[details]
Patch
Enrica Casucci
Comment 2
2015-04-15 13:09:05 PDT
Created
attachment 250841
[details]
Patch2 Fixes build failures.
Build Bot
Comment 3
2015-04-15 13:24:28 PDT
Comment on
attachment 250841
[details]
Patch2
Attachment 250841
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/4685786897186816
Number of test failures exceeded the failure limit.
Build Bot
Comment 4
2015-04-15 13:24:32 PDT
Created
attachment 250844
[details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Darin Adler
Comment 5
2015-04-16 17:26:57 PDT
Regressions: Unexpected timeouts (7) animations/3d/change-transform-in-end-event.html [ Timeout ] compositing/animation/animation-compositing.html [ Timeout ] compositing/backing/backface-visibility-flip.html [ Timeout ] compositing/checkerboard.html [ Timeout ] compositing/contents-scale/animating.html [ Timeout ] compositing/layer-creation/animation-overlap-with-children.html [ Timeout ] compositing/reflections/animation-inside-reflection.html [ Timeout ]
Darin Adler
Comment 6
2015-04-17 09:14:50 PDT
Comment on
attachment 250841
[details]
Patch2 Looks like there are 30 tests failing. I suggest fixing those before we get this reviewed.
Tim Horton
Comment 7
2015-05-01 01:59:26 PDT
Timers are getting throttled to 1 second intervals. Going to look into this now.
Tim Horton
Comment 8
2015-05-01 02:09:49 PDT
That's probably because we're not disabling occlusion detection when we're using WKWebView. There's a second mistake here; the patch as it stands uses WKWebView by default, even though it purports to have it off by default with a command line argument to enable it. That's why this broke tests without actually enabling it. This happened because Options::useModernAPI was not initialized!
Tim Horton
Comment 9
2015-05-01 02:22:06 PDT
Next big problem seems to be that plugins don't work. I remembered that WKWebView has plugins off by default, but flipping the preference on doesn't seem to be sufficient. Anyway, I think we should be able to land this as it is now (with WKWebView off by default) like Enrica originally planned, and continue to improve the missing things and then eventually turn it on. I'll post a fixed up patch shortly for EWS to play with.
Tim Horton
Comment 10
2015-05-01 02:38:38 PDT
Created
attachment 252144
[details]
Patch
Tim Horton
Comment 11
2015-05-01 03:26:49 PDT
Plugins are broken because the patch is quite wrong, and ends up making two different WKContext/WKProcessPools and using them for different things. No good! Hacky fix seems to work... maybe this will get us far enough that we can avoid landing the switchable-API version of this patch?
Tim Horton
Comment 12
2015-05-01 03:38:25 PDT
Only four failures in fast/ now, and none in compositing/! That helped a lot.
Tim Horton
Comment 13
2015-05-01 03:41:39 PDT
Created
attachment 252146
[details]
Patch
Tim Horton
Comment 14
2015-05-01 04:09:05 PDT
Next failure is because BackgroundShouldExtendBeyondPage defaults to NO in the antique API and YES in the modern API. Easy fix, WKTR can maintain the old default.
Tim Horton
Comment 15
2015-05-01 05:47:05 PDT
Next up: teaching EventSenderProxy::mouseUp to hit the inner WKView instead of the WKWebView when the mouse moves outside of the window, to make fast/events/mouseup-outside-document.html pass.
Tim Horton
Comment 16
2015-05-01 05:54:20 PDT
Created
attachment 252148
[details]
Patch
Tim Horton
Comment 17
2015-05-01 06:07:40 PDT
37 failures remain, all but one in http/ or userscripts/. The http/tests/contentextensions tests seem pretty completely hosed, and it looks like the userscripts tests don't work at all either. Might need some help on those tomorrow.
Tim Horton
Comment 18
2015-05-01 06:44:53 PDT
It seems like content extensions and user scripts are broken for the same reason: WKTR is using WKPageGroup SPI that doesn't play nicely with the Modern API (I guess?). We end up with two UserContentControllers (one owned by WebPage, one owned by WebPageGroupProxy), and install extensions/userscripts/etc. on one of them (the WebPageGroupProxy one, I think) while evaluating loads against the other one (the WebPage one, I think). Maybe Sam has some idea of how we should resolve this.
Tim Horton
Comment 19
2015-05-01 06:48:00 PDT
(In reply to
comment #18
)
> Maybe Sam has some idea of how we should resolve this.
Maybe WKTR just needs to use the ObjC WKUserContentController API instead of WKPageGroup if we're using WKWebView? I wonder if we can make this API misuse a harder failure. As it is, it's pretty mysterious.
Simon Fraser (smfr)
Comment 20
2015-05-01 10:05:49 PDT
Comment on
attachment 252148
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=252148&action=review
> Tools/ChangeLog:9 > + This patch add support for the modern API on OS X and iOS.
adds
> Tools/ChangeLog:14 > + run with the --use-modern-api option on the command line. > + The code is slightly more complex than it should, because > + I wanted to keep supporting the C API for Apple platforms, > + since some of the tests are still a bit flakey.
Using the term 'modern' though out this patch will quickly become an anachronism. Maybe "native API" or "objective-C API"? Or rename the old one to "legacy".
> Tools/WebKitTestRunner/Options.cpp:139 > + optionList.append(Option("--use-modern-api", "Uses WKWebView on Mac.", handleOptionUseModernAPI, false));
Not iOS?
> Tools/WebKitTestRunner/TestController.cpp:371 > + // FIXME: This seems backwards. Is there any reason we can't always make a WKContextRef here and put it in the > + // WKWebViewConfiguration in platformInitializeConfiguration by casting to WKProcessPool?
Seems confusing. Should we fix this now?
> Tools/WebKitTestRunner/TestController.h:138 > + static PlatformWebView* platformCreateOtherPage(PlatformWebView* parentView, WKContextRef, WKPageGroupRef, WKPageRef otherPage, WKDictionaryRef options);
'other' is confusing here.
> Tools/WebKitTestRunner/TestController.h:281 > + bool m_useModernAPI;
C++11 initializer?
> Tools/WebKitTestRunner/ios/PlatformWebViewIOS.mm:90 > + _useTiledDrawing = useTiledDrawing; > + return [super initWithFrame:frame configuration:configuration];
Weird to assign to a member var before you initialize super.
> Tools/WebKitTestRunner/ios/PlatformWebViewIOS.mm:163 > + WKPreferencesSetCompositingBordersVisible(preferences, YES); > + WKPreferencesSetCompositingRepaintCountersVisible(preferences, YES);
I don't think we should hardcode those to be on. The fact that iOS did this was leftover debugging.
> Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm:39 > +- (WKPageRef)_pageForTesting;
Gross.
> Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm:95 > + _useThreadedScrolling = useThreadedScrolling;
Ditto.
Tim Horton
Comment 21
2015-05-01 10:29:13 PDT
Comment on
attachment 252148
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=252148&action=review
>> Tools/ChangeLog:14 >> + since some of the tests are still a bit flakey. > > Using the term 'modern' though out this patch will quickly become an anachronism. Maybe "native API" or "objective-C API"? > > Or rename the old one to "legacy".
If all goes well later today we won't need the switch and this comment won't be necessary. The patch is verrrrry close.
>> Tools/WebKitTestRunner/TestController.cpp:371 >> + // WKWebViewConfiguration in platformInitializeConfiguration by casting to WKProcessPool? > > Seems confusing. Should we fix this now?
Want to talk to Sam. But maybe!
>> Tools/WebKitTestRunner/TestController.h:138 >> + static PlatformWebView* platformCreateOtherPage(PlatformWebView* parentView, WKContextRef, WKPageGroupRef, WKPageRef otherPage, WKDictionaryRef options); > > 'other' is confusing here.
Additional? Secondary? Isn't it called other in the API?
>> Tools/WebKitTestRunner/ios/PlatformWebViewIOS.mm:90 >> + return [super initWithFrame:frame configuration:configuration]; > > Weird to assign to a member var before you initialize super.
That is definitely weird.
>> Tools/WebKitTestRunner/ios/PlatformWebViewIOS.mm:163 >> + WKPreferencesSetCompositingRepaintCountersVisible(preferences, YES); > > I don't think we should hardcode those to be on. The fact that iOS did this was leftover debugging.
Oh god no. Did not notice that. Also did you notice that iOS doesn't have snapshotting implemented? :|
>> Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm:39 >> +- (WKPageRef)_pageForTesting; > > Gross.
This is the leadership-approved way of doing this. Didn't want it to be in any headers, not even Private.
Tim Horton
Comment 22
2015-05-01 12:10:29 PDT
(In reply to
comment #19
)
> (In reply to
comment #18
) > > Maybe Sam has some idea of how we should resolve this. > > Maybe WKTR just needs to use the ObjC WKUserContentController API instead of > WKPageGroup if we're using WKWebView?
This ^ seems to work. Content extension tests pass now.
Alexey Proskuryakov
Comment 23
2015-05-01 12:22:50 PDT
Is it feasible to make an overview of what old code becomes unused when using the modern API path? I'd like to look over the dying code to verify that there aren't any non-obvious important hacks that could be forgotten.
Tim Horton
Comment 24
2015-05-01 12:48:54 PDT
(In reply to
comment #23
)
> Is it feasible to make an overview of what old code becomes unused when > using the modern API path? I'd like to look over the dying code to verify > that there aren't any non-obvious important hacks that could be forgotten.
I'm going to get rid of the switch and delete the old code, so it will show up as red and we can look through it then.
Tim Horton
Comment 25
2015-05-01 16:02:18 PDT
User scripts are harder. The user scripts are injected by JS in the page, and the asynchronicity of pushing them to the UI process and back (to go through WKUserContentController, which seems to be the only modern API way to add user scripts) causes the user script to be installed *after* at-load user scripts are evaluated. So, probably we need to add some bundle-side SPI for WKTR to synchronously inject user scripts. Or something.
Tim Horton
Comment 26
2015-05-01 19:15:49 PDT
(In reply to
comment #25
)
> User scripts are harder. The user scripts are injected by JS in the page, > and the asynchronicity of pushing them to the UI process and back (to go > through WKUserContentController, which seems to be the only modern API way > to add user scripts) causes the user script to be installed *after* at-load > user scripts are evaluated. > > So, probably we need to add some bundle-side SPI for WKTR to synchronously > inject user scripts. Or something.
I added SPI to WKBundlePage. I don't know that it was a good idea, but it works, and now user scripts and user stylesheets work. Hopefully Anders or Sam can advise if this is OK. Going to do a full test run and see what's left.
Tim Horton
Comment 27
2015-05-01 23:06:19 PDT
Created
attachment 252218
[details]
Patch
Build Bot
Comment 28
2015-05-01 23:40:01 PDT
Comment on
attachment 252218
[details]
Patch
Attachment 252218
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/5480560394764288
Number of test failures exceeded the failure limit.
Build Bot
Comment 29
2015-05-01 23:40:06 PDT
Created
attachment 252219
[details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Tim Horton
Comment 30
2015-05-01 23:45:15 PDT
Created
attachment 252220
[details]
Patch
Build Bot
Comment 31
2015-05-02 00:51:27 PDT
Comment on
attachment 252220
[details]
Patch
Attachment 252220
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/5327298781773824
New failing tests: userscripts/user-script-audio-document.html userscripts/user-script-plugin-document.html userscripts/script-not-run-for-fragments.html userscripts/document-element-available-at-start.html userscripts/simple-stylesheet.html userscripts/user-script-top-frame-only.html userscripts/user-script-all-frames.html userscripts/script-run-at-start.html userscripts/script-run-at-end.html userscripts/user-style-top-frame-only.html userscripts/mixed-case-stylesheet.html userscripts/user-script-image-document.html userscripts/user-script-video-document.html userscripts/user-stylesheet-invalidate.html userscripts/user-style-all-frames.html fast/css/user-stylesheet-crash.html
Build Bot
Comment 32
2015-05-02 00:51:33 PDT
Created
attachment 252221
[details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Tim Horton
Comment 33
2015-08-14 18:32:22 PDT
Made some more progress today, adding some code to clear content extensions the right way (maybe) with the modern API, and setting a setting (storageBlockingPolicy) whose default is different between the antique and modern APIs. Next up is getting us to use one StorageManager or set the backing directory on the right one, or something (but currently LocalStorage leaks between runs).
Tim Horton
Comment 34
2015-08-14 23:31:06 PDT
Now I've added WKWebsiteDataStoreConfiguration and a way to make a WKWebsiteDataStore given a configuration, adopted it in WKTR, and added a way to disable the legacy data store in the process pool, and we're ending up with just one data store, correctly configured. Yay.
Tim Horton
Comment 35
2015-08-15 03:58:00 PDT
Eight tests remain (on Mac): compositing/video/poster.html - weird 1px image diff fast/css-generated-content/initial-letter-clearance.html fast/css-generated-content/initial-letter-descender.html fast/text/international/bold-bengali.html fast/text/international/danda-space.html fast/text/midword-break-before-surrogate-pair.html - these all have slight text metrics/margin differences (why?) http/tests/media/video-buffered-range-contains-currentTime.html - this has an extra EVENT(seeked) in the output http/tests/security/storage-blocking-strengthened-plugin.html - this says 'false' instead of 'true' in frame0 (haven't looked at what that means yet)
Tim Horton
Comment 36
2015-08-15 04:31:10 PDT
Created
attachment 259088
[details]
patch
WebKit Commit Bot
Comment 37
2015-08-15 04:32:47 PDT
Attachment 259088
[details]
did not pass style-queue: ERROR: Tools/WebKitTestRunner/mac/TestControllerMac.mm:40: Alphabetical sorting problem. [build/include_order] [4] ERROR: Tools/WebKitTestRunner/TestController.h:139: The parameter name "options" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Tools/WebKitTestRunner/TestController.h:140: The parameter name "options" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 3 in 34 files If any of these errors are false positives, please file a bug against check-webkit-style.
Build Bot
Comment 38
2015-08-15 05:34:51 PDT
Comment on
attachment 259088
[details]
patch
Attachment 259088
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.webkit.org/results/61348
New failing tests: userscripts/user-script-audio-document.html userscripts/user-script-plugin-document.html userscripts/script-not-run-for-fragments.html http/tests/security/contentSecurityPolicy/user-style-sheet-font-crasher.html userscripts/document-element-available-at-start.html userscripts/simple-stylesheet.html userscripts/user-script-all-frames.html userscripts/user-script-top-frame-only.html userscripts/script-run-at-start.html userscripts/script-run-at-end.html userscripts/user-style-top-frame-only.html userscripts/mixed-case-stylesheet.html userscripts/user-script-image-document.html userscripts/user-script-video-document.html userscripts/user-stylesheet-invalidate.html userscripts/user-style-all-frames.html fast/css/user-stylesheet-crash.html
Build Bot
Comment 39
2015-08-15 05:34:56 PDT
Created
attachment 259089
[details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Alexey Proskuryakov
Comment 40
2015-08-15 12:47:08 PDT
> http/tests/media/video-buffered-range-contains-currentTime.html > - this has an extra EVENT(seeked) in the output
This is unrelated to the patch, see
bug 148042
.
Tim Horton
Comment 41
2015-08-15 12:51:08 PDT
(In reply to
comment #40
)
> > http/tests/media/video-buffered-range-contains-currentTime.html > > - this has an extra EVENT(seeked) in the output > > This is unrelated to the patch, see
bug 148042
.
Nice :D I guess I have to run on Mavericks and see why EWS has so many failures that I don't see.
Tim Horton
Comment 42
2015-08-18 13:24:55 PDT
(In reply to
comment #41
)
> (In reply to
comment #40
) > > > http/tests/media/video-buffered-range-contains-currentTime.html > > > - this has an extra EVENT(seeked) in the output > > > > This is unrelated to the patch, see
bug 148042
. > > Nice :D > > I guess I have to run on Mavericks and see why EWS has so many failures that > I don't see.
I just realized that these are with the flag (--use-modern-api) *off*, so I haven't actually tested that. Will do that now. Some people think we shouldn't bother having a flag, so I might just get rid of it (and a bunch of code!).
Tim Horton
Comment 43
2015-08-18 16:36:13 PDT
Created
attachment 259315
[details]
Patch
Tim Horton
Comment 44
2015-08-18 17:34:19 PDT
Created
attachment 259330
[details]
Patch
Tim Horton
Comment 45
2015-08-18 23:46:04 PDT
Created
attachment 259364
[details]
Patch
Tim Horton
Comment 46
2015-08-19 00:00:30 PDT
Created
attachment 259365
[details]
Patch
Tim Horton
Comment 47
2015-08-19 18:32:40 PDT
Comment on
attachment 259365
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=259365&action=review
> Tools/WebKitTestRunner/TestController.cpp:622 > + WKPreferencesSetStorageBlockingPolicy(preferences, kWKAllowAllStorage);
Add fixmes for these.
> Tools/WebKitTestRunner/TestController.cpp:683 > + WKPageGroupRemoveAllUserContentFilters(WKPageGetPageGroup(m_mainWebView->page()));
Sam thinks we might not need this.
> Tools/WebKitTestRunner/efl/TestControllerEfl.cpp:73 > + return WKPageGroupGetPreferences(m_pageGroup.get());
This should probably use page configuration (and we shouldn't use page group anywhere!)
> Tools/WebKitTestRunner/ios/PlatformWebViewIOS.mm:37 > +- (WKPageRef)_pageForTesting;
I don't think this needs to be here anymore.
> Tools/WebKitTestRunner/mac/EventSenderProxy.mm:273 > + // when using the modern API, so the level of fakery and horror increases.
Figure out a better "why" here.
Tim Horton
Comment 48
2015-08-19 18:41:25 PDT
Comment on
attachment 259365
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=259365&action=review
> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:5062 > +void WebPage::addUserScript(const String& source, WebCore::UserContentInjectedFrames injectedFrames, WebCore::UserScriptInjectionTime injectionTime)
Talk to Anders about what happens here with the other ports, Sam thinks that using this unconditionally from WKTR might break them. Possible plan is to adopt WKPageConfiguration and set a UserContentControllerRef on it, so we don't end up using the PageGroup's.
Tim Horton
Comment 49
2015-08-20 18:49:07 PDT
Created
attachment 259555
[details]
Patch
Tim Horton
Comment 50
2015-08-20 19:20:29 PDT
Created
attachment 259563
[details]
Patch
Build Bot
Comment 51
2015-08-20 20:15:42 PDT
Comment on
attachment 259563
[details]
Patch
Attachment 259563
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.webkit.org/results/83995
Number of test failures exceeded the failure limit.
Build Bot
Comment 52
2015-08-20 20:15:46 PDT
Created
attachment 259574
[details]
Archive of layout-test-results from ews107 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Tim Horton
Comment 53
2015-08-21 13:21:19 PDT
Split one WebKit2 part out into
https://bugs.webkit.org/show_bug.cgi?id=148325
Tim Horton
Comment 54
2015-08-21 15:54:54 PDT
Created
attachment 259678
[details]
Patch
Anders Carlsson
Comment 55
2015-08-21 16:30:13 PDT
Comment on
attachment 259678
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=259678&action=review
> Tools/WebKitTestRunner/PlatformWebView.h:44 > -@class WKView; > +@class NSView; > +@class UIView; > +@class WKWebView; > +@class WKWebViewConfiguration; > @class WebKitTestRunnerWindow; > #else > -class WKView; > +class NSView; > +class UIView; > +class WKWebView; > +class WKWebViewConfiguration; > class WebKitTestRunnerWindow;
Please use OBJC_CLASS here.
> Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:95 > + {
Please remove \n here.
Tim Horton
Comment 56
2015-08-21 17:36:06 PDT
http://trac.webkit.org/changeset/188807
Tim Horton
Comment 57
2015-08-21 18:35:29 PDT
http://trac.webkit.org/changeset/188813
WebKit Commit Bot
Comment 58
2015-08-21 19:29:20 PDT
Re-opened since this is blocked by
bug 148349
Tim Horton
Comment 59
2015-08-22 15:21:31 PDT
Trying again in
http://trac.webkit.org/changeset/188828
Csaba Osztrogonác
Comment 60
2015-08-23 14:30:18 PDT
(In reply to
comment #59
)
> Trying again in
http://trac.webkit.org/changeset/188828
It broke the EFL build. Including the Cocoa only WKFoundation.h header in common code without ifdef guard is incorrect and should have been fixed before/after landing.
Csaba Osztrogonác
Comment 61
2015-08-23 14:38:35 PDT
(In reply to
comment #60
)
> (In reply to
comment #59
) > > Trying again in
http://trac.webkit.org/changeset/188828
> > It broke the EFL build. Including the Cocoa only WKFoundation.h > header in common code without ifdef guard is incorrect and should > have been fixed before/after landing.
Buildfix landed in
https://trac.webkit.org/changeset/188840
.
Csaba Osztrogonác
Comment 62
2015-08-23 14:48:49 PDT
Additional buildfixes landed in
https://trac.webkit.org/changeset/188842
and
https://trac.webkit.org/changeset/188843
Alexey Proskuryakov
Comment 63
2015-08-26 21:42:59 PDT
This caused
bug 148512
.
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