Lets get rid of WKPageNamespace and replace it with the ability to create a view that is forced into the same namespace as another page.
Created attachment 76339 [details] WIP (Do not review)
<rdar://problem/8253496>
Attachment 76339 [details] did not build on qt: Build output: http://queues.webkit.org/results/6984081
Created attachment 76341 [details] Patch
Attachment 76341 [details] did not build on qt: Build output: http://queues.webkit.org/results/7005066
Comment on attachment 76341 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=76341&action=review > WebKit2/ChangeLog:24 > + all callers of it will have their views end up in the same shared process, where > + as with WKViewCreate, a new internal page namespace will be created and could whereas? > WebKit2/ChangeLog:49 > + Add new initializers for the objective-c WKView. It follows the same Objective-C > WebKit2/UIProcess/WebContext.cpp:238 > + if (m_sharedNamespace) > + return m_sharedNamespace.get(); > + > + m_sharedNamespace = createPageNamespace(); > + return m_sharedNamespace.get(); A shorter way to write this is: if (!m_sharedNamespace) m_sharedNamespace = createPageNamespace(); return m_sharedNamespace.get(); > WebKit2/UIProcess/API/C/win/WKView.h:40 > +WK_EXPORT WKViewRef WKViewCreateForAssociatedPage(RECT rect, WKPageRef page, WKPageGroupRef pageGroup, HWND parentWindow); I think a WKPageRef-based create API is an unnecessary convenience. This one is also awkwardly named. Is calling this different from calling WKViewCreate() passing the context of the page? If so, and three different WKViewCreate…() functions are needed, I suggest merging them into one and adding an options parameter. > WebKit2/UIProcess/API/mac/WKView.h:44 > -- (id)initWithFrame:(NSRect)frame pageNamespaceRef:(WKPageNamespaceRef)pageNamespaceRef; > -- (id)initWithFrame:(NSRect)frame pageNamespaceRef:(WKPageNamespaceRef)pageNamespaceRef pageGroupRef:(WKPageGroupRef)pageGroupRef; > +- (id)initWithFrame:(NSRect)frame contextRef:(WKContextRef)contextRef; > +- (id)initWithFrame:(NSRect)frame contextRef:(WKContextRef)contextRef pageGroupRef:(WKPageGroupRef)pageGroupRef; > > -- (WKPageRef)pageRef; > +- (id)initWithFrame:(NSRect)frame contextRef:(WKContextRef)contextRef usingSharedProcess:(BOOL)usingSharedProcess; > +- (id)initWithFrame:(NSRect)frame contextRef:(WKContextRef)contextRef pageGroupRef:(WKPageGroupRef)pageGroupRef usingSharedProcess:(BOOL)usingSharedProcess;; > > -- (void)setDrawsBackground:(BOOL)flag; > -- (BOOL)drawsBackground; > +- (id)initWithFrame:(NSRect)frame forAssociatedPageRef:(WKPageRef)pageRef; > +- (id)initWithFrame:(NSRect)frame forAssociatedPageRef:(WKPageRef)pageRef pageGroupRef:(WKPageGroupRef)pageGroupRef; > Similar comments about the Objective-C API.
Created attachment 76342 [details] Updated Patch
> > WebKit2/UIProcess/API/C/win/WKView.h:40 > > +WK_EXPORT WKViewRef WKViewCreateForAssociatedPage(RECT rect, WKPageRef page, WKPageGroupRef pageGroup, HWND parentWindow); > > I think a WKPageRef-based create API is an unnecessary convenience. This one is also awkwardly named. Is calling this different from calling WKViewCreate() passing the context of the page? If so, and three different WKViewCreate…() functions are needed, I suggest merging them into one and adding an options parameter. The associate page create function is not a convenience, it is the only way ensure that two views have their WebProcess counterparts in the same WebProcess. It should be used in cases like one page calling window.open(). I am not sure how we can merge these into one function, can you explain what its signature would be?
Attachment 76342 [details] did not build on qt: Build output: http://queues.webkit.org/results/6895068
(In reply to comment #8) > > > WebKit2/UIProcess/API/C/win/WKView.h:40 > > > +WK_EXPORT WKViewRef WKViewCreateForAssociatedPage(RECT rect, WKPageRef page, WKPageGroupRef pageGroup, HWND parentWindow); > > > > I think a WKPageRef-based create API is an unnecessary convenience. This one is also awkwardly named. Is calling this different from calling WKViewCreate() passing the context of the page? If so, and three different WKViewCreate…() functions are needed, I suggest merging them into one and adding an options parameter. > > The associate page create function is not a convenience, it is the only way ensure that two views have their WebProcess counterparts in the same WebProcess. It should be used in cases like one page calling window.open(). I am not sure how we can merge these into one function, can you explain what its signature would be? I see now why a separate WKPageRef-based API is needed.
Comment on attachment 76342 [details] Updated Patch r-, because Qt build is still broken. I'm going to check what the problem is, and will help you to fix it.
Created attachment 76436 [details] Update Patch Does the Qt bot stop after the first error?
(In reply to comment #12) > Does the Qt bot stop after the first error? Unfortunately yes. But I started a clean build locally with your patch. It will be finished ~6-8 min, and then I'll ping you.
(In reply to comment #13) > (In reply to comment #12) > > Does the Qt bot stop after the first error? > Unfortunately yes. > > But I started a clean build locally with your patch. > It will be finished ~6-8 min, and then I'll ping you. Thanks.
Comment on attachment 76436 [details] Update Patch View in context: https://bugs.webkit.org/attachment.cgi?id=76436&action=review > WebKitTools/WebKitTestRunner/qt/PlatformWebViewQt.cpp:65 > +PlatformWebView(WKContextRef, WKPageGroupRef); > +PlatformWebView(WKPageRef, WKPageGroupRef); > + Please remove these useless lines, and Qt build will happy. ;) rs+=me to land the previous patch with this Qt file change. (Of course r=Anders)
(In reply to comment #15) > (From update of attachment 76436 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=76436&action=review > > > WebKitTools/WebKitTestRunner/qt/PlatformWebViewQt.cpp:65 > > +PlatformWebView(WKContextRef, WKPageGroupRef); > > +PlatformWebView(WKPageRef, WKPageGroupRef); > > + > > Please remove these useless lines, and Qt build will happy. ;) > rs+=me to land the previous patch with this Qt file change. (Of course r=Anders) Thanks, landed in r73965.