[webkit-reviews] review denied: [Bug 29536] WebInspector: Migrate DOM storage inspection to serialized interaction : [Attachment 39842] patch

[bugzilla-daemon at webkit.org]

Timothy Hatcher <timothy at hatcher.name> has denied Yury Semikhatsky
<yurys at chromium.org>'s request for review:
Bug 29536: WebInspector: Migrate DOM storage inspection to serialized
interaction
https://bugs.webkit.org/show_bug.cgi?id=29536

Attachment 39842: patch
https://bugs.webkit.org/attachment.cgi?id=39842&action=review

------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>
> +    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+.