Bug 251426

Summary:	[GTK][WPE] Remove webkit_web_view_new_with_related_view from new API too
Product:	WebKit	Reporter:	Carlos Garcia Campos <cgarcia>
Component:	WebKitGTK	Assignee:	Carlos Garcia Campos <cgarcia>
Status:	RESOLVED FIXED
Severity:	Normal	CC:	bugs-noreply, mcatanzaro
Priority:	P2	Keywords:	Gtk
Version:	WebKit Nightly Build
Hardware:	Unspecified
OS:	Unspecified
Bug Depends on:
Bug Blocks:	210100

Description Carlos Garcia Campos 2023-01-31 01:34:21 PST

The related web view was initially thought as a way to ensure a web view is created in the same web process of another web view. That's what the WebKitWebView:related-view documentation says. That's implies that both web views should use the same web context. In webkitWebViewConstructed() we ensure that in case of related web view we always use the same web context, ignoring the web-context passed as construction if any. But we do the same for network-session/is-ephemeral and is-controlled-by-automation. Then we have the constructor webkit_web_view_new_with_related_view() that in addition also passes the same settings, user-content-manager and website-policies. This is confusing, I think webkit_web_view_new_with_related_view() should only pass related-view to g_object_new(). Or maybe we should just remove webkit_web_view_new_with_related_view(), since it seems it's not actually used. Then clarify in the documentation that a related web view always shares the web-context, network-session/is-ephemeral and is-controlled-by-automation, and passing any of these together with related-view has no effect.

Comment 1 Michael Catanzaro 2023-01-31 06:11:11 PST

(In reply to Carlos Garcia Campos from comment #0)
> This is confusing, I think
> webkit_web_view_new_with_related_view() should only pass related-view to
> g_object_new(). Or maybe we should just remove
> webkit_web_view_new_with_related_view(), since it seems it's not actually
> used.

It only makes sense to keep webkit_web_view_new_with_related_view() if all the other construct properties get taken from the related-view.

> Then clarify in the documentation that a related web view always
> shares the web-context, network-session/is-ephemeral and
> is-controlled-by-automation, and passing any of these together with
> related-view has no effect.

As long as there's a g_critical() if you do it wrong, sure.

Comment 2 Carlos Garcia Campos 2023-01-31 07:45:11 PST

(In reply to Michael Catanzaro from comment #1)
> (In reply to Carlos Garcia Campos from comment #0)
> > This is confusing, I think
> > webkit_web_view_new_with_related_view() should only pass related-view to
> > g_object_new(). Or maybe we should just remove
> > webkit_web_view_new_with_related_view(), since it seems it's not actually
> > used.
> 
> It only makes sense to keep webkit_web_view_new_with_related_view() if all
> the other construct properties get taken from the related-view.
> 
> > Then clarify in the documentation that a related web view always
> > shares the web-context, network-session/is-ephemeral and
> > is-controlled-by-automation, and passing any of these together with
> > related-view has no effect.
> 
> As long as there's a g_critical() if you do it wrong, sure.

I'm not sure, it's convenient to just pass something even if it's ignored as long as you know it.

Comment 3 Michael Catanzaro 2023-02-01 06:11:30 PST

(In reply to Carlos Garcia Campos from comment #2)
> I'm not sure, it's convenient to just pass something even if it's ignored as
> long as you know it.

Even if it might be occasionally convenient, it's confusing and developers won't expect it. Not a good idea!

Comment 4 Carlos Garcia Campos 2023-02-03 02:54:20 PST

I think we can simply remove it.

Comment 5 Carlos Garcia Campos 2023-02-03 02:58:01 PST

Pull request: https://github.com/WebKit/WebKit/pull/9593

Comment 6 EWS 2023-02-06 02:32:34 PST

Committed 259885@main (57a26786ef09): <https://commits.webkit.org/259885@main>

Reviewed commits have been landed. Closing PR #9593 and removing active labels.