Summary: | Write storage changes to disk | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Anders Carlsson <andersca> | ||||
Component: | New Bugs | Assignee: | Anders Carlsson <andersca> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | ||||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Anders Carlsson
2013-05-06 10:45:50 PDT
Created attachment 200721 [details]
Patch
Comment on attachment 200721 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=200721&action=review r=me > Source/WebKit2/UIProcess/Storage/LocalStorageDatabase.cpp:40 > +static const double databaseUpdateInterval = 1.0; I'd call this 'databaseUpdateIntervalInSeconds' > Source/WebKit2/UIProcess/Storage/LocalStorageDatabase.cpp:217 > + for (size_t i = 0; i < maximumItemsToUpdate; ++i) { 'i' should be 'int' here to match maximumItemsToUpdate and HashMap::size(). > Source/WebKit2/UIProcess/Storage/LocalStorageDatabase.cpp:248 > + for (HashMap<String, String>::const_iterator it = changedItems.begin(), end = changedItems.end(); it != end; ++it) { auto? > Source/WebKit2/UIProcess/Storage/LocalStorageDatabase.cpp:252 > + statement.bindText(1, it->key); Missing error handling. Is this intentional? > Source/WebKit2/UIProcess/Storage/LocalStorageDatabase.cpp:256 > + statement.bindBlob(2, it->value); Missing error handling. Is this intentional? > Source/WebKit2/UIProcess/Storage/LocalStorageDatabase.cpp:264 > + statement.reset(); Missing error handling. Is this intentional? > Source/WebKit2/UIProcess/Storage/LocalStorageDatabase.h:78 > + HashMap<String, String> m_changedItems; > + bool m_didScheduleDatabaseUpdate; The order of these members makes me twitch. > Source/WebKit2/UIProcess/Storage/StorageManager.cpp:160 > + if (m_localStorageDatabase) > + m_localStorageDatabase->setItem(key, value); Should we still be dispatching events if !m_localStorageDatabase? (In reply to comment #2) > > > Source/WebKit2/UIProcess/Storage/LocalStorageDatabase.cpp:248 > > + for (HashMap<String, String>::const_iterator it = changedItems.begin(), end = changedItems.end(); it != end; ++it) { > > auto? Yes. > > > Source/WebKit2/UIProcess/Storage/LocalStorageDatabase.cpp:252 > > + statement.bindText(1, it->key); > > Missing error handling. Is this intentional? > > > Source/WebKit2/UIProcess/Storage/LocalStorageDatabase.cpp:256 > > + statement.bindBlob(2, it->value); > > Missing error handling. Is this intentional? The various bind functions will only fail if the column index is out of range or if malloc fails - I don’t think we need to care about those conditions. > > > Source/WebKit2/UIProcess/Storage/LocalStorageDatabase.cpp:264 > > + statement.reset(); > > Missing error handling. Is this intentional? Reset will only return an error if the previous step returned an error. > > > Source/WebKit2/UIProcess/Storage/StorageManager.cpp:160 > > + if (m_localStorageDatabase) > > + m_localStorageDatabase->setItem(key, value); > > Should we still be dispatching events if !m_localStorageDatabase? Yup. (For session storage). Committed r149615: <http://trac.webkit.org/changeset/149615> |