This is next step in migrating Storage Panel to serialized interaction.
Created attachment 39842 [details] patch
This looks good to me!
Comment on attachment 39842 [details] patch > + if (storageResourceId) { > + m_frontend->selectDOMStorage(storageResourceId); > + } No need for braces. > + for (DOMStorageResourcesSet::iterator it = m_domStorageResources.begin(); it != domStorageEnd; ++it) { > + if ((*it)->id() == storageId) { > + return it->get(); > + } > + } Ditto. > + if (isSameHostAndType(storage->frame(), isLocalStorage)) { > + m_frontend->updateDOMStorage(m_id); > + } Ditto. > + return listener->type() == InspectorDOMStorageResourceType > + ? static_cast<const InspectorDOMStorageResource*>(listener) > + : 0; Just put put on one line. > + getEntriesAsync: function(callback) > + setItemAsync: function(key, value, callback) > + removeItemAsync: function(key, callback) I am not sure "Async" is needed for these, since they take a callback, it is implied. All the other async calls Pavel has added don't have that suffix. > + if (columnIdentifier == 0) { > + if (this._keys.indexOf(newText) != -1) { Would be good to use === or !==. > + for (var index = 0; index < entries.length; index++) { Just use "i" instead of index and ++i. > + if (domStorage.id == storageId) { > + return domStorage; > } No need for braces. Otherwise will be r+.
Created attachment 39936 [details] patch
(In reply to comment #3) > (From update of attachment 39842 [details]) > > + if (storageResourceId) { > > + m_frontend->selectDOMStorage(storageResourceId); > > + } > > No need for braces. > Done. > > > + for (DOMStorageResourcesSet::iterator it = m_domStorageResources.begin(); it != domStorageEnd; ++it) { > > + if ((*it)->id() == storageId) { > > + return it->get(); > > + } > > + } > > Ditto. > Done. > > > + if (isSameHostAndType(storage->frame(), isLocalStorage)) { > > + m_frontend->updateDOMStorage(m_id); > > + } > > Ditto. > Done. > > > + return listener->type() == InspectorDOMStorageResourceType > > + ? static_cast<const InspectorDOMStorageResource*>(listener) > > + : 0; > > Just put put on one line. > Done. > > > + getEntriesAsync: function(callback) > > + setItemAsync: function(key, value, callback) > > + removeItemAsync: function(key, callback) > > I am not sure "Async" is needed for these, since they take a callback, it is > implied. All the other async calls Pavel has added don't have that suffix. > Removed Async suffixes. > > > + if (columnIdentifier == 0) { > > + if (this._keys.indexOf(newText) != -1) { > > Would be good to use === or !==. > Done. > > > + for (var index = 0; index < entries.length; index++) { > > Just use "i" instead of index and ++i. > Done. It was called index in the original code. > > > + if (domStorage.id == storageId) { > > + return domStorage; > > } > > No need for braces. > Done.
Comment on attachment 39936 [details] patch Rejecting patch 39936 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1']" exit_code: 1 Running build-dumprendertree Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Testing 11312 test cases. httpd is already running: pid 37053, killing... http/tests/loading/basic-credentials-sent-automatically.html -> failed Exiting early after 1 failures. 8433 tests run. 209.08s total testing time 8432 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 4 test cases (<1%) had stderr output
Comment on attachment 39936 [details] patch Flakey test. bug 29505. Adding back to the commit-queue.
Comment on attachment 39936 [details] patch Clearing flags on attachment: 39936 Committed r48659: <http://trac.webkit.org/changeset/48659>
All reviewed patches have been landed. Closing bug.
This was a two-sided patch for Chromium. Yury, Pavel, please make sure to note this in the future. It's very hard for the webkit gardeners to keep up.
Chromium build fixes landed as http://trac.webkit.org/changeset/48665 and http://trac.webkit.org/changeset/48663. Come on guys.
(In reply to comment #10) > This was a two-sided patch for Chromium. Yury, Pavel, please make sure to note > this in the future. It's very hard for the webkit gardeners to keep up. Sorry about that. It was not supposed to be two-sided, just the import bustage. Won't happen again.