[webkit-reviews] review denied: [Bug 83012] Web Inspector: localStorage items are not updated when the storage changes : [Attachment 141993] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Vivek Galatage
<vivekgalatage at gmail.com>'s request for review:
Bug 83012: Web Inspector: localStorage items are not updated when the storage
changes
https://bugs.webkit.org/show_bug.cgi?id=83012

Attachment 141993: Patch
https://bugs.webkit.org/attachment.cgi?id=141993&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=141993&action=review


> Source/WebCore/inspector/InspectorDOMStorageAgent.cpp:199
> +	   id = storageId(sourceFrame->domWindow()->sessionStorage(ec));

See my comment below first. You would need to change
InspectorDOMStorageResource::isSameHostAndType to accept SecurityOrigin to fix
it.

> Source/WebCore/inspector/InspectorInstrumentation.h:1163
> +	   didDispatchDOMStorageEventImpl(instrumentingAgents, key, oldValue,
newValue, storageType, securityOrigin, page->mainFrame());

I probably made your life a bit harder with the additional Chromium code and
migration to the Page* in instrumentation. Sorry about that. Imagine that there
is an iframe from domain "foo" that is dispatching this event. Main frame is
from "bar". We should update local storage for "foo", but as written in this
code, we will update the "bar". Thing is that SecurityOrigin defines the local
storage you'd like to update.