Bug 105438 – Web Inspector: Provide an isolated InspectorState instance rather than the shared one for each Agent

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: Web Inspector (Deprecated)

[Alexander Pavlov (apavlov)]

Patch to follow.

[Alexander Pavlov (apavlov)]

Created attachment 180336 [details]
Patch

[Andrey Kosyakov]

Comment on attachment 180336 [details]
Patch

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

> Source/WebCore/inspector/InspectorPageAgent.cpp:-417
> -    if (!scripts) {

Why is this gone?

[Alexander Pavlov (apavlov)]

Comment on attachment 180336 [details]
Patch

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

>> Source/WebCore/inspector/InspectorPageAgent.cpp:-417
>> -    if (!scripts) {
> 
> Why is this gone?

Please see the changelog entry for this method and getObject() implementation :)

[Andrey Kosyakov]

Comment on attachment 180336 [details]
Patch

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

>>> Source/WebCore/inspector/InspectorPageAgent.cpp:-417
>>> -    if (!scripts) {
>> 
>> Why is this gone?
> 
> Please see the changelog entry for this method and getObject() implementation :)

Ok, I see. I did not know the fact that getObject() creates object if field is missing. The sad fact is that it sort of counters the effect of remove upon the first check of this property. Looks like we might be better off with replacing a call to remove() with setting the field to an empty object.

[Pavel Feldman]

Comment on attachment 180336 [details]
Patch

I'd suggest the following refactoring that would make things even better: structure state so that instead of holding all agents' states in a single flat m_properties map, it would have an object property per agent. The resetting the state of a particular agent would be as simple as resetting its state object (or replacing it with new blank object). Such refactoring would require InspectorController to shard state object and pass individual InspectorState into each of the agents.

[Alexander Pavlov (apavlov)]

Created attachment 180516 [details]
Patch

[Build Bot]

Comment on attachment 180516 [details]
Patch

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

[Alexander Pavlov (apavlov)]

Created attachment 180522 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 180522 [details]
Patch

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

> Source/WebCore/inspector/InspectorState.cpp:118
> +    InspectorState* result = new InspectorState(this, stateProperties);

This should be OwnPtr<InspectorState> or you can inline this.

> Source/WebCore/inspector/InspectorState.cpp:123
> +void InspectorCompositeState::loadFromCookie(const String& InspectorCompositeStateCookie)

InspectorCompositeStateCookie -> inspectorCompositeStateCookie

[Alexander Pavlov (apavlov)]

Created attachment 180649 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 180649 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        Web Inspector: Make use of the new InspectorState::remove() in inspector agents

The main part of this change is about introduction of a composite state that allows to avoid name collisions, we should probably rename the bug.

[Alexander Pavlov (apavlov)]

The change in its current variation allows to get rid of agent-specific prefix hacks for state property names.

[Alexander Pavlov (apavlov)]

Committed r138436: <http://trac.webkit.org/changeset/138436>

[WebKit Review Bot]

Re-opened since this is blocked by bug 105723

[Alexander Pavlov (apavlov)]

Re-implemented in bug 105722.

*** This bug has been marked as a duplicate of bug 105722 ***