Bug 109572

Summary: Web Inspector: Local/session storage tree items in the Resources panel after page refresh are not shown
Product: WebKit Reporter: Alexander Pavlov (apavlov) <apavlov>
Component: Web Inspector (Deprecated)Assignee: Alexander Pavlov (apavlov) <apavlov>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, buildbot, dglazkov, esprehn+autocc, graouts, gyuyoung.kim, joepeck, keishi, loislo, ojan.autocc, pfeldman, pmuellr, rakuco, rniwa, syoichi, timothy, vivekg, vsevik, web-inspector-bugs, webkit-ews, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 109801, 110105, 110232, 110457    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
For EWS
none
Patch
none
Patch
none
Comments addressed none

Description Alexander Pavlov (apavlov) 2013-02-12 05:31:47 PST
1. open a page on localhost
2. run localStorage.setItem('foo', 'bar')
3. Navigate to the resources tab and select Local Storage > localhost
4. Refresh the page

What is the expected result?
Local Storage > localhost should still be selected

What happens instead?
localhost disappears from Local Storage until you run setItem or getItem.

Upstreaming https://code.google.com/p/chromium/issues/detail?id=104585
Comment 1 Alexander Pavlov (apavlov) 2013-02-20 05:59:48 PST
Created attachment 189299 [details]
Patch
Comment 2 Early Warning System Bot 2013-02-20 06:14:50 PST
Comment on attachment 189299 [details]
Patch

Attachment 189299 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/16647306
Comment 3 Early Warning System Bot 2013-02-20 06:18:30 PST
Comment on attachment 189299 [details]
Patch

Attachment 189299 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/16648303
Comment 4 Alexander Pavlov (apavlov) 2013-02-20 06:20:13 PST
Created attachment 189304 [details]
Patch
Comment 5 WebKit Review Bot 2013-02-20 07:42:02 PST
Comment on attachment 189304 [details]
Patch

Attachment 189304 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16659298

New failing tests:
http/tests/inspector/appcache/appcache-iframe-manifests.html
http/tests/inspector/injected-script-for-origin.html
http/tests/inspector/extensions-network-redirect.html
http/tests/inspector/change-iframe-src.html
http/tests/inspector/modify-cross-domain-rule.html
http/tests/inspector/appcache/appcache-manifest-with-non-existing-file.html
http/tests/inspector/console-resource-errors.html
http/tests/inspector/resource-har-conversion.html
http/tests/inspector/extensions-headers.html
http/tests/inspector-enabled/injected-script-discard.html
http/tests/inspector/console-cross-origin-iframe-logging.html
http/tests/inspector/appcache/appcache-swap.html
http/tests/inspector/extensions-useragent.html
http/tests/inspector-enabled/dynamic-scripts.html
http/tests/inspector/stylesheet-source-mapping.html
http/tests/inspector-enabled/console-exception-while-no-inspector.html
http/tests/inspector/console-cd-completions.html
http/tests/inspector/compiler-script-mapping.html
http/tests/inspector/compiler-source-mapping-debug.html
http/tests/inspector/web-socket-frame-error.html
http/tests/inspector/console-xhr-logging.html
http/tests/inspector/console-xhr-logging-async.html
http/tests/inspector/console-cd.html
http/tests/inspector/resource-har-headers.html
http/tests/inspector/network-preflight-options.html
http/tests/inspector/inspect-iframe-from-different-domain.html
http/tests/inspector-enabled/console-log-before-frame-navigation.html
http/tests/inspector-enabled/dom-storage-open.html
http/tests/inspector/resource-parameters.html
http/tests/inspector-enabled/database-open.html
Comment 6 WebKit Review Bot 2013-02-20 08:28:31 PST
Comment on attachment 189304 [details]
Patch

Attachment 189304 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16660321

New failing tests:
http/tests/inspector/appcache/appcache-iframe-manifests.html
http/tests/inspector/injected-script-for-origin.html
http/tests/inspector/extensions-network-redirect.html
http/tests/inspector/change-iframe-src.html
http/tests/inspector/modify-cross-domain-rule.html
http/tests/inspector/appcache/appcache-manifest-with-non-existing-file.html
http/tests/inspector/console-resource-errors.html
http/tests/inspector/resource-har-conversion.html
http/tests/inspector/extensions-headers.html
http/tests/inspector-enabled/injected-script-discard.html
http/tests/inspector/console-cross-origin-iframe-logging.html
http/tests/inspector/appcache/appcache-swap.html
http/tests/inspector/extensions-useragent.html
http/tests/inspector-enabled/dynamic-scripts.html
http/tests/inspector/stylesheet-source-mapping.html
http/tests/inspector-enabled/console-exception-while-no-inspector.html
http/tests/inspector/console-cd-completions.html
http/tests/inspector/compiler-script-mapping.html
http/tests/inspector/compiler-source-mapping-debug.html
http/tests/inspector/web-socket-frame-error.html
http/tests/inspector/console-xhr-logging.html
http/tests/inspector/console-xhr-logging-async.html
http/tests/inspector/console-cd.html
http/tests/inspector/resource-har-headers.html
http/tests/inspector/network-preflight-options.html
http/tests/inspector/inspect-iframe-from-different-domain.html
http/tests/inspector-enabled/console-log-before-frame-navigation.html
http/tests/inspector-enabled/dom-storage-open.html
http/tests/inspector/resource-parameters.html
http/tests/inspector-enabled/database-open.html
Comment 7 Alexander Pavlov (apavlov) 2013-02-20 08:51:56 PST
Created attachment 189330 [details]
For EWS
Comment 8 WebKit Review Bot 2013-02-20 10:08:14 PST
Comment on attachment 189330 [details]
For EWS

Attachment 189330 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16654400

New failing tests:
http/tests/inspector-enabled/dom-storage-open.html
Comment 9 WebKit Review Bot 2013-02-20 11:22:38 PST
Comment on attachment 189330 [details]
For EWS

Attachment 189330 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16647436

New failing tests:
http/tests/inspector-enabled/dom-storage-open.html
Comment 10 Build Bot 2013-02-20 13:16:36 PST
Comment on attachment 189330 [details]
For EWS

Attachment 189330 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16660458

New failing tests:
http/tests/inspector-enabled/dom-storage-open.html
Comment 11 Alexander Pavlov (apavlov) 2013-02-20 23:52:58 PST
Created attachment 189464 [details]
Patch
Comment 12 Alexander Pavlov (apavlov) 2013-02-21 01:34:05 PST
Created attachment 189474 [details]
Patch
Comment 13 Vsevolod Vlasov 2013-02-21 01:35:40 PST
Comment on attachment 189464 [details]
Patch

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

> Source/WebCore/inspector/InjectedScriptSource.js:188
> +                    hints.domStorageId = JSON.parse(storageId);

We should use InjectedScriptHost.evaluate
 instead.

> Source/WebCore/inspector/InspectorDOMStorageAgent.cpp:118
> +    restore();

You should not call restore from enable

> Source/WebCore/inspector/InspectorDOMStorageAgent.cpp:160
> +    ExceptionCode exception = 0;

Let's get rid of success parameter and use ErrorString for that.

> Source/WebCore/inspector/front-end/DOMStorage.js:34
> + * @param {string=} id

looks like id is not used

> Source/WebCore/inspector/front-end/DOMStorageItemsView.js:62
> +        this.update();

Why do you need this?

> Source/WebCore/inspector/front-end/EmptyView.js:55
> +    _processWillHide: function()

Redundant change?

> Source/WebCore/inspector/front-end/ResourcesPanel.js:286
> +        WebInspector.domStorageModel.storages().forEach(this._addDOMStorage.bind(this));

I would expect that we receive OriginAdded events in this case and hence DOMStorageAdded events, so this line seems redundant.

> Source/WebCore/inspector/front-end/ResourcesPanel.js:344
> +        if (this._domStorageTreeElements.get(domStorage))

I don't think this should ever happen, we should replace this check with a console.assert call.

> Source/WebCore/inspector/front-end/ResourcesPanel.js:506
> +    _dropDOMStorage: function(domStorage)

Not used.
Comment 14 Alexander Pavlov (apavlov) 2013-02-21 01:59:58 PST
Created attachment 189478 [details]
Comments addressed
Comment 15 WebKit Review Bot 2013-02-21 03:58:45 PST
Comment on attachment 189478 [details]
Comments addressed

Clearing flags on attachment: 189478

Committed r143581: <http://trac.webkit.org/changeset/143581>
Comment 16 WebKit Review Bot 2013-02-21 03:58:51 PST
All reviewed patches have been landed.  Closing bug.
Comment 17 WebKit Review Bot 2013-02-21 04:26:11 PST
Re-opened since this is blocked by bug 110457
Comment 18 Alexander Pavlov (apavlov) 2013-02-21 06:10:20 PST
Committed r143593: <http://trac.webkit.org/changeset/143593>