Bug 128620 – Web Inspector: DOMStorageView should listen for events from DOMStorageObject

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Blaze Burg]

There's a layering violation now; the StorageManager explicitly maintains a map of available views and forwards events to them directly.

[Radar WebKit Bug Importer]

<rdar://problem/16040514>

[Blaze Burg]

Created attachment 223894 [details]
patch

[Timothy Hatcher]

Comment on attachment 223894 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=223894&action=review

> Source/WebInspectorUI/ChangeLog:8
> +        The storage manager used to have a map of DOMStorageViews and delivered events

Yeah that was gross!

> Source/WebInspectorUI/UserInterface/DOMStorageObserver.js:70
> -        WebInspector.storageManager.domStorageItemsCleared(storageId);
> +        var object = WebInspector.storageManager.domStorageForId(storageId);
> +        object.itemsCleared(storageId);
>      },
>  
>      domStorageItemRemoved: function(storageId, key)
>      {
> -        WebInspector.storageManager.domStorageItemRemoved(storageId, key);
> +        var object = WebInspector.storageManager.domStorageForId(storageId);
> +        object.itemRemoved(storageId, key);
>      },
>  
>      domStorageItemAdded: function(storageId, key, value)
>      {
> -        WebInspector.storageManager.domStorageItemAdded(storageId, key, value);
> +        var object = WebInspector.storageManager.domStorageForId(storageId);
> +        object.itemAdded(storageId, key, value);
>      },
>  
>      domStorageItemUpdated: function(storageId, key, oldValue, value)
>      {
> -        WebInspector.storageManager.domStorageItemUpdated(storageId, key, oldValue, value);
> +        var object = WebInspector.storageManager.domStorageForId(storageId);
> +        object.itemUpdated(storageId, key, oldValue, value);

This differs from other observers. Usually the protocol observers are very thin and do no work other than calling the right manager(s). That way any compatibility work happens in the manager. Not that this way is entirely bad. It would just push down compatibility massaging into the observer.

> Source/WebInspectorUI/UserInterface/StorageManager.js:108
> +    domStorageForId: function(id)

domStorageForIdentifier

[Blaze Burg]

Comment on attachment 223894 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=223894&action=review

>> Source/WebInspectorUI/UserInterface/DOMStorageObserver.js:70
>> +        object.itemUpdated(storageId, key, oldValue, value);
> 
> This differs from other observers. Usually the protocol observers are very thin and do no work other than calling the right manager(s). That way any compatibility work happens in the manager. Not that this way is entirely bad. It would just push down compatibility massaging into the observer.

I don't have strong opinions here, except that as written there are 4 fewer dummy functions. But the compatibility argument makes sense to me.

>> Source/WebInspectorUI/UserInterface/StorageManager.js:108
>> +    domStorageForId: function(id)
> 
> domStorageForIdentifier

Oh, I was going to get that.

[Blaze Burg]

Committed r163927: <http://trac.webkit.org/changeset/163927>