Bug 144404 – Web Inspector: Split Storage from Resources tab

• Component: Web Inspector
• Status: RESOLVED FIXED

[Timothy Hatcher]

Add a Storage tab.

[Timothy Hatcher]

Created attachment 251976 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 251976 [details]
Patch

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

I think this patch is actually missing WebInspector.StorageTabContentView, so StorageTabContentView.js.

> Source/WebInspectorUI/UserInterface/Controllers/ApplicationCacheManager.js:59
> +    get applicationCacheObjects()
> +    {
> +        var applicationCacheObjects = [];
> +        for (var id in this._applicationCacheObjects)
> +            applicationCacheObjects.push(this._applicationCacheObjects[id]);
> +        return applicationCacheObjects;
> +    }

This code looks weird because this._applicationCacheObjects is initialized to an array up above. Should it be an object? Or better yet a Map (no `delete`, and this would be [...map.values()]).

> Source/WebInspectorUI/UserInterface/Views/DatabaseTableContentView.js:93
> +            this._dataGrid = undefined;

What is this for? (Unfortunately the patch review tool is not giving me context).

[Timothy Hatcher]

Comment on attachment 251976 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Controllers/ApplicationCacheManager.js:59
>> +    }
> 
> This code looks weird because this._applicationCacheObjects is initialized to an array up above. Should it be an object? Or better yet a Map (no `delete`, and this would be [...map.values()]).

_applicationCacheObjects is treated as a map object below! I'll fix the _applicationCacheObjects = [] to be _applicationCacheObjects = {}. I save converting it to Map until later.

>> Source/WebInspectorUI/UserInterface/Views/DatabaseTableContentView.js:93
>> +            this._dataGrid = undefined;
> 
> What is this for? (Unfortunately the patch review tool is not giving me context).

Sorry. It fixes an exception. Seems this._dataGrid can be allocated but this._dataGrid.element is null/undefined.

[Timothy Hatcher]

Created attachment 251977 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 251977 [details]
Patch

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

r=me, commit-queue won't work at the moment, patch is not applying.

> Source/WebInspectorUI/UserInterface/Views/StorageSidebarPanel.js:290
> +        this._localStorageRootTreeElement = null;
> +        this._sessionStorageRootTreeElement = null;
> +        this._databaseRootTreeElement = null;
> +        this._databaseHostTreeElementMap = {};
> +        this._indexedDatabaseRootTreeElement = null;
> +        this._indexedDatabaseHostTreeElementMap = {};
> +        this._cookieStorageRootTreeElement = null;
> +        this._applicationCacheRootTreeElement = null;
> +        this._applicationCacheURLTreeElementMap = {};

This is repeated in the constructor. I wonder if it should be a helper, like _reset.

> Source/WebInspectorUI/UserInterface/Views/StorageTabContentView.js:50
> +        return representedObject instanceof WebInspector.DOMStorageObject || representedObject instanceof WebInspector.CookieStorageObject ||
> +            representedObject instanceof WebInspector.DatabaseTableObject || representedObject instanceof WebInspector.DatabaseObject ||
> +            representedObject instanceof WebInspector.ApplicationCacheFrame || representedObject instanceof WebInspector.IndexedDatabaseObjectStore ||
> +            representedObject instanceof WebInspector.IndexedDatabaseObjectStoreIndex;

Style: I think we normally put the "||" on the start of the next line instead of at the end of the previous.

[Timothy Hatcher]

Comment on attachment 251977 [details]
Patch

https://trac.webkit.org/r183580