DevTools: DOMStorage action history must be reset once page/script modifies the DOM storage

DevTools: DOMStorage action history must be reset once page/script modifies the DOM storage

CL https://codereview.chromium.org/21163003/ introduced undo/redo feature on DOM storage panel.
In this, if the page or the scripts running inside the page itself adds/deletes/modifies/clears the DOM storage, the
undo/redo history object must be reset in order to keep the storage entries consistent otherwise
it can mess up with the page script.

R=pfeldman, apavlov@chromium.org, vsevik

[vivekg__, 2013-08-27 11:44:26]

The DOMStorage object needs to keep the track of the calls made to the
InspectorDOMStorageAgent for setting/removal of the entries. If there is any
update received and if the DOMStorage object is not the source for modification,
the storage history must be reset.

This requires making the test case work in asynchronous manner hence most of the
changes were done in the test case along with some of the cleanup. 

Also adding the test case resetDOMStorageHistoryUponPageModification() to demo
the expected behavior of history reset.

Please take a look and let me know your feedback.

Thank you,
Vivek.

[pfeldman, 2013-08-27 14:27:34]

https://codereview.chromium.org/23480006/diff/1/LayoutTests/inspector/storage...
File LayoutTests/inspector/storage-panel-dom-storage-undo-redo.html (right):

https://codereview.chromium.org/23480006/diff/1/LayoutTests/inspector/storage...
LayoutTests/inspector/storage-panel-dom-storage-undo-redo.html:70:
setTimeout(perform, 0);
Please don't use setTimeout in tests. It most likely does not do what you think
and only increases flakiness.

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

https://codereview.chromium.org/23480006/diff/1/Source/devtools/front_end/DOM...
Source/devtools/front_end/DOMStorage.js:432: domStorage.resetUndoHistory();
Due to async nature of things, it is not necessarily initiated by inspector.
Consider the following case:

1) User clicks "remove item" in the inspector on key "a"
2) While request is going over the wire, page removes "b" from localStorage 
3) this method is invoked and it thinks that "b" removal was initiated by the
inspector

and vice versa. The only way to figure out who initiated the mutation is to
process it on backend synchronously.

[vivekg__, 2013-08-28 06:06:48]

On 2013/08/27 14:27:34, pfeldman wrote:

Thanks Pavel.

>
https://codereview.chromium.org/23480006/diff/1/LayoutTests/inspector/storage...
> File LayoutTests/inspector/storage-panel-dom-storage-undo-redo.html (right):
> 
>
https://codereview.chromium.org/23480006/diff/1/LayoutTests/inspector/storage...
> LayoutTests/inspector/storage-panel-dom-storage-undo-redo.html:70:
> setTimeout(perform, 0);
> Please don't use setTimeout in tests. It most likely does not do what you
think
> and only increases flakiness.
> 

Yeah I tried not to use setTimeout but the test is not working without that and
if I use runAfterPendingDispatches also it wont work.
Any alternative suggestion you have?

>
https://codereview.chromium.org/23480006/diff/1/Source/devtools/front_end/DOM...
> File Source/devtools/front_end/DOMStorage.js (right):
> 
>
https://codereview.chromium.org/23480006/diff/1/Source/devtools/front_end/DOM...
> Source/devtools/front_end/DOMStorage.js:432: domStorage.resetUndoHistory();
> Due to async nature of things, it is not necessarily initiated by inspector.
> Consider the following case:
> 
> 1) User clicks "remove item" in the inspector on key "a"
> 2) While request is going over the wire, page removes "b" from localStorage 
> 3) this method is invoked and it thinks that "b" removal was initiated by the
> inspector
> 
> and vice versa. The only way to figure out who initiated the mutation is to
> process it on backend synchronously.

How do we make the call to storageArea->setItem/removeItem to be sync as it goes
back to the browser process n returns async way?

[pfeldman, 2013-08-28 07:32:49]

> How do we make the call to storageArea->setItem/removeItem to be sync as it
goes
> back to the browser process n returns async way?

Oh, so we can't implement it reliably then. Given the amount of investment
involved, I'd suggest to revert the undo/redo feature for localStorage and focus
on something else. It does not look like it is feasible + no one has asked for
it.

[vivekg__, 2013-08-28 07:49:34]

On 2013/08/28 07:32:49, pfeldman wrote:
> > How do we make the call to storageArea->setItem/removeItem to be sync as it
> goes
> > back to the browser process n returns async way?
> 
> Oh, so we can't implement it reliably then. Given the amount of investment
> involved, I'd suggest to revert the undo/redo feature for localStorage and
focus
> on something else. It does not look like it is feasible + no one has asked for
> it.

yeah that's true as it would involve lot of re-factoring to support this one odd
case. It makes sense to revert this change. Thanks! 

Will submit a patch to revert this change.