Bug 54227 – Web Inspector: make InspectorAgent own sub-agents, align agent creation/deletion routines.

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

[Pavel Feldman]

This patch will align some of our agents' lifetime, creation and deletion code, state interaction and such.

[Pavel Feldman]

Created attachment 82010 [details]
[PATCH] wip patch.

[Pavel Feldman]

Created attachment 82106 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 82106 [details]
Patch

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

> Source/WebCore/inspector/InspectorAgent.cpp:-337
> -    releaseFrontendLifetimeAgents();

Please put an ASSERT(!m_frontend) here.

> Source/WebCore/inspector/InspectorDOMAgent.cpp:943
> +void InspectorDOMAgent::mainDOMContentLoaded()

mainDOMContentLoaded -> mainFrameDOMContentLoaded

> Source/WebCore/inspector/InspectorDatabaseResource.cpp:57
>  void InspectorDatabaseResource::bind(InspectorFrontend* frontend)

Let's rename bind into pushToFrontend

[Ilya Tikhonovsky]

Comment on attachment 82106 [details]
Patch

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

> Source/WebCore/inspector/InspectorDOMAgent.cpp:279
>  void InspectorDOMAgent::startListening(Document* doc)
>  {
> -    if (m_documents.contains(doc))
> -        return;
> -
> -    doc->addEventListener(eventNames().DOMContentLoadedEvent, this, false);
> -    doc->addEventListener(eventNames().loadEvent, this, true);
>      m_documents.add(doc);
>  }
>  
>  void InspectorDOMAgent::stopListening(Document* doc)
>  {
> -    if (!m_documents.contains(doc))
> -        return;
> -
> -    doc->removeEventListener(eventNames().DOMContentLoadedEvent, this, false);
> -    doc->removeEventListener(eventNames().loadEvent, this, true);
>      m_documents.remove(doc);
>  }

There are no accessors to the m_documents map but mainFrameDocument.
Looks like we can drop map and handle only MainFrame document pointer.

[Pavel Feldman]

Created attachment 82128 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 82128 [details]
Patch

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

> LayoutTests/inspector/command-line-api.html:8
> +	var expressions = [

This fix should go in a separate change, r- for this.

> LayoutTests/inspector/command-line-api.html:19
> +            InspectorTest.completeTest();

wrong alignment

[Pavel Feldman]

Created attachment 82129 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 82129 [details]
Patch

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

> Source/WebCore/inspector/InspectorDatabaseAgent.h:45
> +    class FrontendProvider : public RefCounted<FrontendProvider> {

No need to define this class in the public part of the agent. You can just put forward declaration here and define the class in .cpp.

[Pavel Feldman]

Committed r78336: <http://trac.webkit.org/changeset/78336>