109572 – Web Inspector: Local/session storage tree items in the Resources panel after page refresh are not shown

Bug 109572 - Web Inspector: Local/session storage tree items in the Resources panel after page refresh are not shown

Summary: Web Inspector: Local/session storage tree items in the Resources panel after ...

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	Web Inspector (Deprecated) (show other bugs)
Version:	528+ (Nightly build)
Hardware:	All All

Importance:	P2 Normal
Assignee:	Alexander Pavlov (apavlov)

URL:
Keywords:

Depends on:	109801 110105 110232 110457
Blocks:
	Show dependency tree / graph

Reported:	2013-02-12 05:31 PST by Alexander Pavlov (apavlov)
Modified:	2013-02-21 06:10 PST (History)
CC List:	22 users (show)

See Also:

Attachments
Patch (73.54 KB, patch) 2013-02-20 05:59 PST, Alexander Pavlov (apavlov)	no flags	Details \| Formatted Diff \| Diff
Patch (73.57 KB, patch) 2013-02-20 06:20 PST, Alexander Pavlov (apavlov)	no flags	Details \| Formatted Diff \| Diff
For EWS (73.89 KB, patch) 2013-02-20 08:51 PST, Alexander Pavlov (apavlov)	no flags	Details \| Formatted Diff \| Diff
Patch (76.68 KB, patch) 2013-02-20 23:52 PST, Alexander Pavlov (apavlov)	no flags	Details \| Formatted Diff \| Diff
Patch (77.34 KB, patch) 2013-02-21 01:34 PST, Alexander Pavlov (apavlov)	no flags	Details \| Formatted Diff \| Diff
Comments addressed (75.65 KB, patch) 2013-02-21 01:59 PST, Alexander Pavlov (apavlov)	no flags	Details \| Formatted Diff \| Diff
Show Obsolete (5) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

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>

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