DevTools: DOMStorage should notify the view about storage modifications instead of DOMStorageModel

DevTools: DOMStorage should notify the view about storage modifications instead of DOMStorageModel

R=apavlov, pfeldman, vsevik

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=156635

[vivekg_samsung, 2013-08-07 15:52:39]

With respect to the current event notification model, the DOMStorageModel is
relaying the event to the listeners i.e. the DOMStorageItemsView. This has a
potential issue about cross storage event notifications being sent to the views.

This should be avoided and only the relevant views should get the notifications.

[pfeldman, 2013-08-23 12:17:50]

https://codereview.chromium.org/22346003/diff/1/Source/devtools/front_end/DOM...
File Source/devtools/front_end/DOMStorage.js (left):

https://codereview.chromium.org/22346003/diff/1/Source/devtools/front_end/DOM...
Source/devtools/front_end/DOMStorage.js:394:
this.dispatchEventToListeners(WebInspector.DOMStorageModel.Events.DOMStorageItemsCleared,
storageData);
It looks like changing this with domStorage.dispatchEventToListeners will
produce smaller diff and achieve similar result.

https://codereview.chromium.org/22346003/diff/1/Source/devtools/front_end/DOM...
File Source/devtools/front_end/DOMStorage.js (right):

https://codereview.chromium.org/22346003/diff/1/Source/devtools/front_end/DOM...
Source/devtools/front_end/DOMStorage.js:347: domStorageModified:
function(storageId, eventType, eventData)
This can be private (private things are private to file, not class). Call it
_fireDOMStorageEvent

https://codereview.chromium.org/22346003/diff/1/Source/devtools/front_end/DOM...
File Source/devtools/front_end/DOMStorageItemsView.js (right):

https://codereview.chromium.org/22346003/diff/1/Source/devtools/front_end/DOM...
Source/devtools/front_end/DOMStorageItemsView.js:173: if (childNode.data.value
=== storageData.oldValue) {
This changes semantics. Why?

[vivekg__, 2013-08-23 13:19:36]

Thanks for the review Pavel. I have addressed all the comments and added new
patch. PTAL.

https://codereview.chromium.org/22346003/diff/1/Source/devtools/front_end/DOM...
File Source/devtools/front_end/DOMStorage.js (left):

https://codereview.chromium.org/22346003/diff/1/Source/devtools/front_end/DOM...
Source/devtools/front_end/DOMStorage.js:394:
this.dispatchEventToListeners(WebInspector.DOMStorageModel.Events.DOMStorageItemsCleared,
storageData);
On 2013/08/23 12:17:51, pfeldman wrote:
> It looks like changing this with domStorage.dispatchEventToListeners will
> produce smaller diff and achieve similar result.

Sounds great. Will revert this. Thanks!

https://codereview.chromium.org/22346003/diff/1/Source/devtools/front_end/DOM...
File Source/devtools/front_end/DOMStorageItemsView.js (right):

https://codereview.chromium.org/22346003/diff/1/Source/devtools/front_end/DOM...
Source/devtools/front_end/DOMStorageItemsView.js:173: if (childNode.data.value
=== storageData.oldValue) {
On 2013/08/23 12:17:51, pfeldman wrote:
> This changes semantics. Why?

Oops, this was a stray local change slipped into the patch. Will revert it.
Thanks!

[pfeldman, 2013-08-23 13:24:52]

lgtm

[commit-bot: I haz the power, 2013-08-23 14:08:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vivek.vg@samsung.com/22346003/5001

[commit-bot: I haz the power, 2013-08-23 15:15:57]

Change committed as 156635