Created attachment 191494 [details]
Patch

This follows the input given in bug 109455.

Comment on attachment 191494 [details]
Patch

LGTM

Comment on attachment 191494 [details]
Patch

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

No problem with this per se, but I am unhappy there are both C APIs and platformDefault. It would be good if we could get rid of platformDefault for everyone.

Okay for this patch on WebKit2.

> Source/WebKit2/UIProcess/Plugins/qt/PluginProcessProxyQt.cpp:66
> +    QString cachePath = QStandardPaths::writableLocation(QStandardPaths::CacheLocation);
> +    ASSERT(!cachePath.isEmpty());

I don't know if the user has any control over QStandardPaths::writableLocation.

If there is any way for cachePath.isEmpty() to be empty due to a user mistake, it would be good to have a fall back in addition to the assertion.

> Source/WebKit2/UIProcess/qt/QtWebContext.cpp:161
> +    Q_ASSERT(!path.isEmpty());

ditto

> Source/WebKit2/UIProcess/qt/QtWebIconDatabaseClient.cpp:57
> +    WKContextSetIconDatabasePath(toAPI(context), adoptWK(WKStringCreateWithQString(QtWebContext::preparedStoragePath(QtWebContext::IconDatabaseStorage))).get());

I would split this line to have
    [...]
    WKContextSetIconDatabasePath((toAPI(context), path.get());

Created attachment 192457 [details]
Patch

Added empty cache file path handling in PluginProcessProxyQt.cpp.
I'd prefer to keep the assert instead of handling an empty data location in QtWebContext since it complexifies the code a lot more, and there is no way to test this code, so I'd prefer to see if it is really needed before. From my research in the QStandardPaths implementation, there should always be a non-empty path returned for DataLocation and CacheLocation.

Comment on attachment 192457 [details]
Patch

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

> Source/WebKit2/ChangeLog:7
> +        Reviewed by NOBODY (OOPS!).
> +

Missing sign-off line :). Please fix before landing.

Committed r145517: <http://trac.webkit.org/changeset/145517>