RESOLVED FIXED 29536
WebInspector: Migrate DOM storage inspection to serialized interaction
https://bugs.webkit.org/show_bug.cgi?id=29536
Summary WebInspector: Migrate DOM storage inspection to serialized interaction
Yury Semikhatsky
Reported 2009-09-21 00:52:01 PDT
This is next step in migrating Storage Panel to serialized interaction.
Attachments
patch (36.37 KB, patch)
2009-09-21 00:53 PDT, Yury Semikhatsky
timothy: review-
patch (36.24 KB, patch)
2009-09-22 12:17 PDT, Yury Semikhatsky
no flags
Yury Semikhatsky
Comment 1 2009-09-21 00:53:46 PDT
Pavel Feldman
Comment 2 2009-09-22 01:34:03 PDT
This looks good to me!
Timothy Hatcher
Comment 3 2009-09-22 09:51:00 PDT
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+.
Yury Semikhatsky
Comment 4 2009-09-22 12:17:16 PDT
Yury Semikhatsky
Comment 5 2009-09-22 12:19:08 PDT
(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.
WebKit Commit Bot
Comment 6 2009-09-22 16:03:00 PDT
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
Eric Seidel (no email)
Comment 7 2009-09-22 16:14:36 PDT
Comment on attachment 39936 [details] patch Flakey test. bug 29505. Adding back to the commit-queue.
WebKit Commit Bot
Comment 8 2009-09-22 17:09:18 PDT
Comment on attachment 39936 [details] patch Clearing flags on attachment: 39936 Committed r48659: <http://trac.webkit.org/changeset/48659>
WebKit Commit Bot
Comment 9 2009-09-22 17:09:21 PDT
All reviewed patches have been landed. Closing bug.
Dimitri Glazkov (Google)
Comment 10 2009-09-22 17:32:32 PDT
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.
Dimitri Glazkov (Google)
Comment 11 2009-09-22 19:57:00 PDT
Chromium build fixes landed as http://trac.webkit.org/changeset/48665 and http://trac.webkit.org/changeset/48663. Come on guys.
Pavel Feldman
Comment 12 2009-09-22 23:04:56 PDT
(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.
Note You need to log in before you can comment on or make changes to this bug.