[webkit-reviews] review denied: [Bug 61940] [Qt] [WK2] Overwrite default plugin directories for Qt WebKit2 tests : [Attachment 95774] fix patch

[bugzilla-daemon at webkit.org]

Kenneth Rohde Christiansen <kenneth at webkit.org> has denied Chang Shu
<cshu at webkit.org>'s request for review:
Bug 61940: [Qt] [WK2] Overwrite default plugin directories for Qt WebKit2 tests
https://bugs.webkit.org/show_bug.cgi?id=61940

Attachment 95774: fix patch
https://bugs.webkit.org/attachment.cgi?id=95774&action=review

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=95774&action=review

> Source/WebKit2/UIProcess/WebContext.h:106
> +    void setAdditionalPluginsDirectory(const String&, bool);

the bool value is not obvious here, add the override arguemnt

> Source/WebKit2/UIProcess/API/C/WKContext.cpp:131
> +void _WKContextSetAdditionalPluginsDirectory(WKContextRef contextRef,
WKStringRef pluginsDirectory, bool overwrite)

isnt it override and not overwrite ?

> Source/WebKit2/UIProcess/Plugins/PluginInfoStore.cpp:46
> -void PluginInfoStore::setAdditionalPluginsDirectories(const Vector<String>&
directories)
> +void PluginInfoStore::setAdditionalPluginsDirectories(const Vector<String>&
directories, bool overwrite)

This seems like a misuse of the method.

When you are overriding you are not adding anything additional. I would add
another method and maybe make an internal method if code can be shared