Bug 133015

Summary: Need a way to get a WKPageRef from a WKWebView
Product: WebKit Reporter: David Farler <dfarler>
Component: WebKit2Assignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED WONTFIX    
Severity: Normal CC: andersca, benjamin, commit-queue, ddkilzer, sam, simon.fraser, thorton
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: iPhone / iPad   
OS: All   
Bug Depends on: 133050    
Bug Blocks:    
Attachments:
Description Flags
Patch v1 none

Description David Farler 2014-05-16 15:57:27 PDT
PlatformWebView in WebKitTestRunner has public WKPageRef page() but WKWebView doesn't currently have API to provide this.
Comment 1 David Kilzer (:ddkilzer) 2014-05-17 08:42:23 PDT
Created attachment 231634 [details]
Patch v1

I'm sure there are layer violations (like returning a C API struct from an Objective-C class), but I'm assuming we want this to move off the deprecated WKView class first.
Comment 2 Benjamin Poulain 2014-05-17 17:08:13 PDT
Comment on attachment 231634 [details]
Patch v1

Cool!
Comment 3 WebKit Commit Bot 2014-05-17 17:39:08 PDT
Comment on attachment 231634 [details]
Patch v1

Clearing flags on attachment: 231634

Committed r169001: <http://trac.webkit.org/changeset/169001>
Comment 4 WebKit Commit Bot 2014-05-17 17:39:12 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Sam Weinig 2014-05-17 20:58:00 PDT
I'm worried about why this is being done.  If the idea is to use WKPageRef to set up the needed clients in WKTR, that won't work, and we will really need to add an abstraction so the objective-c API can be used there.
Comment 6 Benjamin Poulain 2014-05-17 22:31:18 PDT
(In reply to comment #5)
> I'm worried about why this is being done.  If the idea is to use WKPageRef to set up the needed clients in WKTR, that won't work, and we will really need to add an abstraction so the objective-c API can be used there.

WKTR will need it to send events and some internal testing stuff.

It looks like it also setup the clients in TestController::createOtherPage(). Moving away from that is gonna suck, the C API was awesome to setup cross-port tests.
Comment 7 Anders Carlsson 2014-05-18 09:24:57 PDT
Yeah, this patch is wrong. I'm going to roll it out.
Comment 8 Anders Carlsson 2014-05-18 09:26:42 PDT
It should not be possible to get at the C SPI from the modern API. We're trying to get rid of the modern API.
Comment 9 WebKit Commit Bot 2014-05-18 09:27:09 PDT
Re-opened since this is blocked by bug 133050
Comment 10 Benjamin Poulain 2014-05-18 14:41:08 PDT
(In reply to comment #7)
> Yeah, this patch is wrong. I'm going to roll it out.

Anders, you created this situation in the first place. Any chance you could fix testing?
Comment 11 Anders Carlsson 2014-05-18 14:55:45 PDT
(In reply to comment #10)
> (In reply to comment #7)
> > Yeah, this patch is wrong. I'm going to roll it out.
> 
> Anders, you created this situation in the first place. Any chance you could fix testing?

Sam is interested in fixing it.

In any case, I'm going to WONTFIX this since we're never going to expose the old SPI from the new API.
Comment 12 Benjamin Poulain 2014-05-18 14:58:33 PDT
(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #7)
> > > Yeah, this patch is wrong. I'm going to roll it out.
> > 
> > Anders, you created this situation in the first place. Any chance you could fix testing?
> 
> Sam is interested in fixing it.
> 
> In any case, I'm going to WONTFIX this since we're never going to expose the old SPI from the new API.

I really don't care how we expose the internals. I just want WKView to get out of the way and be able to run certain tests.