WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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-
Details
Formatted Diff
Diff
patch
(36.24 KB, patch)
2009-09-22 12:17 PDT
,
Yury Semikhatsky
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Yury Semikhatsky
Comment 1
2009-09-21 00:53:46 PDT
Created
attachment 39842
[details]
patch
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
Created
attachment 39936
[details]
patch
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.
Top of Page
Format For Printing
XML
Clone This Bug