Bug 103219 – Initialize new web processes with list of auto-start origins for plug-ins

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Jon Lee]

Initialize new web processes with list of auto-start origins.

[Jon Lee]

<rdar://problem/12747219>

[Brady Eidson]

How large will this list be?

[Jon Lee]

Created attachment 176368 [details]
WIP

[Jon Lee]

(In reply to comment #2)
> How large will this list be?

Unknown, to be honest. But it should basically monotonically increase over time.

[Brady Eidson]

(In reply to comment #4)
> (In reply to comment #2)
> > How large will this list be?
> 
> Unknown, to be honest. But it should basically monotonically increase over time.

With an approach that keeps a copy of the list in each WebProcess, this is a red flag for me.

[Maciej Stachowiak]

(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #2)
> > > How large will this list be?
> > 
> > Unknown, to be honest. But it should basically monotonically increase over time.
> 
> With an approach that keeps a copy of the list in each WebProcess, this is a red flag for me.

Perhaps the eventual direction should probably be to keep the master copy in the UIProcess and query/update asynchronously. But we can do an estimate of memory cost. Let's say a user intentionally enables some plugin content on 200 sites (probably a pretty high number!). Let's say 20 unicode characters for a domain name, 2 domains per entry. Then you are talking about a ~16kb cost per WebProcess, which is trivial. So I don't think memory use is a strong reason to hold off for the initial version.

[Brady Eidson]

Okay sounds reasonable

[Alexey Proskuryakov]

Duplicating the data still seems undesirable even if the direct cost is not so big. Why can't WebProcess just send a message to ask UI process when needed?

[Maciej Stachowiak]

(In reply to comment #8)
> Duplicating the data still seems undesirable even if the direct cost is not so big. Why can't WebProcess just send a message to ask UI process when needed?

I'm guessing this way was initially simpler to code. I share your instincts about the right eventual approach though.

[Sam Weinig]

(In reply to comment #8)
> Duplicating the data still seems undesirable even if the direct cost is not so big. Why can't WebProcess just send a message to ask UI process when needed?

We don't want to increase the number the of sync calls.  If we want to make this more efficient, I think putting the table of strings into shared memory would be the best option.

[Maciej Stachowiak]

(In reply to comment #10)
> (In reply to comment #8)
> > Duplicating the data still seems undesirable even if the direct cost is not so big. Why can't WebProcess just send a message to ask UI process when needed?
> 
> We don't want to increase the number the of sync calls.  If we want to make this more efficient, I think putting the table of strings into shared memory would be the best option.

I think it's very likely this can be an asynchronous call without really affecting performance, it would just make the code a little bit more complicated.

[Jon Lee]

(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #8)
> > > Duplicating the data still seems undesirable even if the direct cost is not so big. Why can't WebProcess just send a message to ask UI process when needed?
> > 
> > We don't want to increase the number the of sync calls.  If we want to make this more efficient, I think putting the table of strings into shared memory would be the best option.
> 
> I think it's very likely this can be an asynchronous call without really affecting performance, it would just make the code a little bit more complicated.

I believe we can safely have this be async for the one kind of plug-in that can be async'd initialized at this point, but we can't do it for other plug-in types until we know they can be async'd init'd also. So in those cases we need to keep a web process side cache of the table anyway.

For this reason I am going to keep the current approach. But I will go through a client instead of platform strategies, and switch to an unsigned 32-bit hash that includes the mime type.

[Jon Lee]

Created attachment 178677 [details]
Patch

[Anders Carlsson]

Comment on attachment 178677 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=178677&action=review

> Source/WebKit2/UIProcess/Plugins/PlugInAutoStartProvider.cpp:58
> +void PlugInAutoStartProvider::populateCopyOfTable(Vector<unsigned>& copyVector)
> +{
> +    copyToVector(m_autoStartHashes, copyVector);
> +}

I think you should make this return a Vector<unsigned> instead and give it a more appropriate name.

[Jon Lee]

Committed r137342: <http://trac.webkit.org/changeset/137342>