WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
50898
Change the WebKit2 public API so there is no explicit WKPageNamespace object
https://bugs.webkit.org/show_bug.cgi?id=50898
Summary
Change the WebKit2 public API so there is no explicit WKPageNamespace object
Sam Weinig
Reported
2010-12-12 15:27:23 PST
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.
Attachments
WIP (Do not review)
(64.15 KB, patch)
2010-12-12 16:24 PST
,
Sam Weinig
no flags
Details
Formatted Diff
Diff
Patch
(70.23 KB, patch)
2010-12-12 17:03 PST
,
Sam Weinig
no flags
Details
Formatted Diff
Diff
Updated Patch
(70.89 KB, patch)
2010-12-12 17:31 PST
,
Sam Weinig
ossy
: review-
Details
Formatted Diff
Diff
Update Patch
(70.85 KB, patch)
2010-12-13 14:08 PST
,
Sam Weinig
ossy
: review+
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Sam Weinig
Comment 1
2010-12-12 16:24:42 PST
Created
attachment 76339
[details]
WIP (Do not review)
Sam Weinig
Comment 2
2010-12-12 16:33:25 PST
<
rdar://problem/8253496
>
Early Warning System Bot
Comment 3
2010-12-12 16:41:24 PST
Attachment 76339
[details]
did not build on qt: Build output:
http://queues.webkit.org/results/6984081
Sam Weinig
Comment 4
2010-12-12 17:03:45 PST
Created
attachment 76341
[details]
Patch
Early Warning System Bot
Comment 5
2010-12-12 17:16:54 PST
Attachment 76341
[details]
did not build on qt: Build output:
http://queues.webkit.org/results/7005066
mitz
Comment 6
2010-12-12 17:20:25 PST
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.
Sam Weinig
Comment 7
2010-12-12 17:31:15 PST
Created
attachment 76342
[details]
Updated Patch
Sam Weinig
Comment 8
2010-12-12 17:38:57 PST
> > 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?
Early Warning System Bot
Comment 9
2010-12-12 17:44:03 PST
Attachment 76342
[details]
did not build on qt: Build output:
http://queues.webkit.org/results/6895068
mitz
Comment 10
2010-12-12 17:46:17 PST
(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.
Csaba Osztrogonác
Comment 11
2010-12-13 14:02:14 PST
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.
Sam Weinig
Comment 12
2010-12-13 14:08:01 PST
Created
attachment 76436
[details]
Update Patch Does the Qt bot stop after the first error?
Csaba Osztrogonác
Comment 13
2010-12-13 14:19:10 PST
(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.
Sam Weinig
Comment 14
2010-12-13 14:29:33 PST
(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.
Csaba Osztrogonác
Comment 15
2010-12-13 14:32:31 PST
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)
Sam Weinig
Comment 16
2010-12-13 14:38:47 PST
(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
.
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