[Webkit-unassigned] [Bug 89666] [Qt] Separate WebKit2 instances use the same local storage

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jun 21 08:37:51 PDT 2012


https://bugs.webkit.org/show_bug.cgi?id=89666


Balazs Kelemen <kbalazs at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #148807|                            |review-
               Flag|                            |




--- Comment #3 from Balazs Kelemen <kbalazs at webkit.org>  2012-06-21 08:37:49 PST ---
(From update of attachment 148807)
View in context: https://bugs.webkit.org/attachment.cgi?id=148807&action=review

The idea is good but you need to polish the patch. Please listen to the style bot, and use check-webkit-style next time. (I did not mentioned style issues in the review.)

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

Normally we summarize the change in a few sentences here.

> Source/WebKit2/Shared/qt/QtDefaultDataLocation.cpp:49
> +    QString& s_dataLocation = globalDataLocation();

It's no more a static, so please remove the misleading s_ prefix.

> Source/WebKit2/Shared/qt/QtDefaultDataLocation.cpp:65
> +    QString& s_dataLocation = globalDataLocation();

ditto

> Source/WebKit2/Shared/qt/QtDefaultDataLocation.h:36
> +QString& s_dataLocation();

What is this??? You don't use it anywhere.

> Source/WebKit2/Shared/qt/QtDefaultDataLocation.h:38
> +QWEBKIT_EXPORT void setDataLocation(QString dataLocation);

I would put this into global namespace. Maybe we could find a more expressing name, I'm not sure what could it be. setPersistentDataLocation maybe. (And than rename defaultDataLocation to persistentDataLocation and the files as well, in a following cleanup patch.)

> Tools/WebKitTestRunner/qt/TestControllerQt.cpp:57
> +    if (getenv("DUMPRENDERTREE_TEMP") != NULL)
> +        WebKit::setDataLocation(getenv("DUMPRENDERTREE_TEMP"));

remove != NULL, and avoid calling qgetenv twice, please.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list