[webkit-reviews] review denied: [Bug 92919] Out-of-process plug-ins should support asynchronous initialization. : [Attachment 156781] Part 3 - v4 - Add the feature (with some tests) (and Qt build fixes) (and possibly GTK build fixes)

[bugzilla-daemon at webkit.org]

Anders Carlsson <andersca at apple.com> has denied Brady Eidson
<beidson at apple.com>'s request for review:
Bug 92919: Out-of-process plug-ins should support asynchronous initialization.
https://bugs.webkit.org/show_bug.cgi?id=92919

Attachment 156781: Part 3 - v4 - Add the feature (with some tests) (and Qt
build fixes) (and possibly GTK build fixes)
https://bugs.webkit.org/attachment.cgi?id=156781&action=review

------- Additional Comments from Anders Carlsson <andersca at apple.com>
> Source/WebKit2/PluginProcess/WebProcessConnection.cpp:93
> +static HashSet<uint64_t>& asynchronousInstanceIDsToIgnore()
> +{
> +    DEFINE_STATIC_LOCAL(HashSet<uint64_t>, instances, ());
> +    return instances;
> +}

This should be a member of WebProcessConnection. pluginInstanceIDs are not
unique across multiple WebProcesses.

> Source/WebKit2/PluginProcess/WebProcessConnection.cpp:311
> +    if (creationParameters.artificialPluginInitializationDelayEnabled) {
> +	   static unsigned int artificialPluginInitializationDelay = 5;
> +	   sleep(artificialPluginInitializationDelay);
> +    }

I don't like that this test code is here, but I guess it's OK. The constant
should not be static, and you can just use unsigned instead of unsigned int.

> Source/WebKit2/WebProcess/Plugins/Plugin.h:88
> +    virtual bool isInitializingAsynchronously() const = 0;

How about isBeingAsynchronouslyInitialized() instead? Also, add a comment
indicating what this function does.

> Source/WebKit2/WebProcess/Plugins/PluginController.h:164
> +    virtual void didInitializePlugin() = 0;
> +    virtual void didFailToInitializePlugin() = 0;

Please add comments indicating what these functions are supposed to do.