[Webkit-unassigned] [Bug 62669] [Qt] Add storage tracker WK1 API for QtWebKit port

[bugzilla-daemon at webkit.org]

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





--- Comment #13 from kasthuri <kasthuri.n-s at nokia.com>  2011-06-29 07:25:48 PST ---
(In reply to comment #11)
> (From update of attachment 98428 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=98428&action=review
> 
> > Source/WebKit/qt/Api/qwebpage.cpp:402
> > +    delete m_localStorageTracker;
> 
> Like previous comment make sure it is 0 or wrap the definition variable in #if ENABLE(DOM_STORAGE) because in the case !ENABLE(DOM_STORAGE) the pointer is not initialized.

Changed.
> 
> > Source/WebKit/qt/Api/qwebpage.cpp:3955
> >  
> 
> Where is doc?
Added the doc.  Didn't add doc previously as I was not sure whether to expose this as a public API or do it internally in qwebpage and let the use just look for signal if interested. 
> 
> > Source/WebKit/qt/Api/qwebpage.cpp:3956
> > +void QWebPage::observeStorageTrackerNotifications()
> 
> And if I don't want to observe the notification anymore? Isn't it better to have an API with a bool on/off?

My thinking is if the user no longer is interested, then he can disconnect from the signal.  Again as quoted above, if the observeStorageTrackerNotifications() call is handled internally and not exposed as a API, then all the user has to do is just connect to localStorageModified() signal.
> 
> > Source/WebKit/qt/Api/qwebpage.cpp:3959
> > +    QObject::connect(d->m_localStorageTracker, SIGNAL(originModified(const QString&)), this, SIGNAL(localStorageModified(const QString&)), Qt::UniqueConnection);
> 
> Perhaps it is justified but could you explain me why UniqueConnection?
I used this to prevent duplicate connections from happening, if in case the user calls the api twice.
> 
> > Source/WebKit/qt/Api/qwebsecurityorigin.cpp:304
> > +    Removes a particular security origin which has local storage.
> 
> Per doc policy you need to make a comment on the parameter.
> 
Done.
> > Source/WebKit/qt/Api/qwebsecurityorigin.cpp:315
> > +    Returns the disk usage for a particular security origin.
> 
> Ditto.
>
Changed. 
> > Source/WebKit/qt/Api/qwebsecurityorigin.cpp:327
> > +    Syncs local storage dbs.
> 
> You have to use full english words in the public documentation.
> 
Changed.
> > Source/WebKit/qt/WebCoreSupport/StorageTrackerClientQt.h:32
> > +#include <QtCore/qstring.h>
> 
> Why QObject and the next line qstring.h. Make it consistent.
> 
Changed.
> > LayoutTests/platform/qt/Skipped:148
> > +storage/domstorage/localstorage/storagetracker/storage-tracker-7-usage.html
> 
> it doesn't pass? Perhaps you should open a bug report about that one.
There is a hardcoded size in the testcase which I'm checking further.  Will update the skipped list or create a new bug accordingly.

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