Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(96)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 17030009: Inspector: wrapped inspector agent instances into factory-like wrappers (Closed)

Created:
7 years, 6 months ago by Vladislav Kaznacheev
Modified:
7 years, 5 months ago
Reviewers:
pfeldman
CC:
blink-reviews, caseq+blink_chromium.org, loislo+blink_chromium.org, eustas+blink_chromium.org, alph+blink_chromium.org, eae+blinkwatch, yurys+blink_chromium.org, lushnikov+blink_chromium.org, vsevik+blink_chromium.org, pfeldman+blink_chromium.org, apavlov+blink_chromium.org, aandrey+blink_chromium.org
Visibility:
Public.

Description

Inspector: wrapped inspector agent instances into factory-like wrappers This patch does not change the lifetime of any inspector agent object, but makes it easier to change agents lifetimes on per-domain basis. BUG=248092

Patch Set 1 #

Patch Set 2 : Made InspectorBaseAgent RefCounted #

Total comments: 26

Patch Set 3 : Addressed comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+884 lines, -325 lines) Patch
M Source/core/inspector/CodeGeneratorInspector.py View 1 2 2 chunks +13 lines, -3 lines 0 comments Download
M Source/core/inspector/CodeGeneratorInspectorStrings.py View 1 2 2 chunks +3 lines, -2 lines 0 comments Download
M Source/core/inspector/InspectorAgent.h View 1 2 3 chunks +20 lines, -4 lines 0 comments Download
M Source/core/inspector/InspectorAgent.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/inspector/InspectorApplicationCacheAgent.h View 1 2 3 chunks +18 lines, -8 lines 0 comments Download
M Source/core/inspector/InspectorApplicationCacheAgent.cpp View 1 2 2 chunks +17 lines, -2 lines 0 comments Download
M Source/core/inspector/InspectorBaseAgent.h View 1 2 3 chunks +46 lines, -19 lines 0 comments Download
M Source/core/inspector/InspectorBaseAgent.cpp View 1 2 1 chunk +70 lines, -25 lines 0 comments Download
M Source/core/inspector/InspectorCSSAgent.h View 1 2 5 chunks +18 lines, -7 lines 0 comments Download
M Source/core/inspector/InspectorCSSAgent.cpp View 1 2 4 chunks +10 lines, -4 lines 0 comments Download
M Source/core/inspector/InspectorCanvasAgent.h View 1 2 4 chunks +18 lines, -8 lines 0 comments Download
M Source/core/inspector/InspectorCanvasAgent.cpp View 1 2 2 chunks +13 lines, -2 lines 0 comments Download
M Source/core/inspector/InspectorConsoleAgent.h View 1 2 2 chunks +8 lines, -2 lines 0 comments Download
M Source/core/inspector/InspectorConsoleAgent.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/inspector/InspectorController.h View 1 2 2 chunks +3 lines, -3 lines 0 comments Download
M Source/core/inspector/InspectorController.cpp View 1 2 8 chunks +39 lines, -40 lines 0 comments Download
M Source/core/inspector/InspectorDOMAgent.h View 1 2 6 chunks +18 lines, -11 lines 0 comments Download
M Source/core/inspector/InspectorDOMAgent.cpp View 1 2 2 chunks +13 lines, -2 lines 0 comments Download
M Source/core/inspector/InspectorDOMDebuggerAgent.h View 1 2 4 chunks +18 lines, -5 lines 0 comments Download
M Source/core/inspector/InspectorDOMDebuggerAgent.cpp View 1 2 2 chunks +10 lines, -4 lines 0 comments Download
M Source/core/inspector/InspectorDOMStorageAgent.h View 1 2 3 chunks +18 lines, -8 lines 0 comments Download
M Source/core/inspector/InspectorDOMStorageAgent.cpp View 1 2 2 chunks +14 lines, -3 lines 0 comments Download
M Source/core/inspector/InspectorDatabaseAgent.h View 1 2 3 chunks +21 lines, -5 lines 0 comments Download
M Source/core/inspector/InspectorDatabaseAgent.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/inspector/InspectorDebuggerAgent.h View 1 2 3 chunks +8 lines, -2 lines 0 comments Download
M Source/core/inspector/InspectorDebuggerAgent.cpp View 1 2 2 chunks +3 lines, -3 lines 0 comments Download
M Source/core/inspector/InspectorFileSystemAgent.h View 1 2 2 chunks +17 lines, -4 lines 0 comments Download
M Source/core/inspector/InspectorFileSystemAgent.cpp View 1 2 3 chunks +10 lines, -4 lines 0 comments Download
M Source/core/inspector/InspectorHeapProfilerAgent.h View 1 2 3 chunks +19 lines, -3 lines 0 comments Download
M Source/core/inspector/InspectorHeapProfilerAgent.cpp View 1 2 2 chunks +5 lines, -5 lines 0 comments Download
M Source/core/inspector/InspectorIndexedDBAgent.h View 1 2 2 chunks +18 lines, -8 lines 0 comments Download
M Source/core/inspector/InspectorIndexedDBAgent.cpp View 1 2 2 chunks +13 lines, -2 lines 0 comments Download
M Source/core/inspector/InspectorInputAgent.h View 1 2 1 chunk +21 lines, -5 lines 0 comments Download
M Source/core/inspector/InspectorInputAgent.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/inspector/InspectorLayerTreeAgent.h View 1 2 3 chunks +21 lines, -5 lines 0 comments Download
M Source/core/inspector/InspectorLayerTreeAgent.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/inspector/InspectorMemoryAgent.h View 1 2 3 chunks +21 lines, -5 lines 0 comments Download
M Source/core/inspector/InspectorMemoryAgent.cpp View 2 chunks +3 lines, -3 lines 0 comments Download
M Source/core/inspector/InspectorPageAgent.h View 1 2 4 chunks +19 lines, -3 lines 0 comments Download
M Source/core/inspector/InspectorPageAgent.cpp View 1 2 2 chunks +4 lines, -4 lines 0 comments Download
M Source/core/inspector/InspectorProfilerAgent.h View 1 2 5 chunks +17 lines, -4 lines 0 comments Download
M Source/core/inspector/InspectorProfilerAgent.cpp View 1 2 3 chunks +11 lines, -5 lines 0 comments Download
M Source/core/inspector/InspectorResourceAgent.h View 1 2 4 chunks +18 lines, -10 lines 0 comments Download
M Source/core/inspector/InspectorResourceAgent.cpp View 1 2 3 chunks +15 lines, -4 lines 0 comments Download
M Source/core/inspector/InspectorRuntimeAgent.h View 1 2 3 chunks +8 lines, -2 lines 0 comments Download
M Source/core/inspector/InspectorRuntimeAgent.cpp View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/inspector/InspectorTimelineAgent.h View 1 2 6 chunks +22 lines, -10 lines 0 comments Download
M Source/core/inspector/InspectorTimelineAgent.cpp View 1 2 2 chunks +13 lines, -2 lines 0 comments Download
M Source/core/inspector/InspectorWorkerAgent.h View 1 2 3 chunks +20 lines, -4 lines 0 comments Download
M Source/core/inspector/InspectorWorkerAgent.cpp View 1 1 chunk +4 lines, -4 lines 0 comments Download
M Source/core/inspector/PageConsoleAgent.h View 1 2 1 chunk +16 lines, -6 lines 0 comments Download
M Source/core/inspector/PageConsoleAgent.cpp View 1 2 2 chunks +12 lines, -1 line 0 comments Download
M Source/core/inspector/PageDebuggerAgent.h View 1 2 3 chunks +16 lines, -3 lines 0 comments Download
M Source/core/inspector/PageDebuggerAgent.cpp View 1 2 2 chunks +9 lines, -3 lines 0 comments Download
M Source/core/inspector/PageRuntimeAgent.h View 1 2 3 chunks +16 lines, -6 lines 0 comments Download
M Source/core/inspector/PageRuntimeAgent.cpp View 1 2 2 chunks +12 lines, -1 line 0 comments Download
M Source/core/inspector/WorkerConsoleAgent.h View 1 2 1 chunk +19 lines, -3 lines 0 comments Download
M Source/core/inspector/WorkerConsoleAgent.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/inspector/WorkerDebuggerAgent.h View 1 2 2 chunks +18 lines, -2 lines 0 comments Download
M Source/core/inspector/WorkerDebuggerAgent.cpp View 1 1 chunk +3 lines, -3 lines 0 comments Download
M Source/core/inspector/WorkerInspectorController.h View 1 2 1 chunk +1 line, -1 line 0 comments Download
M Source/core/inspector/WorkerInspectorController.cpp View 1 2 4 chunks +11 lines, -11 lines 0 comments Download
M Source/core/inspector/WorkerRuntimeAgent.h View 1 2 3 chunks +19 lines, -3 lines 0 comments Download
M Source/core/inspector/WorkerRuntimeAgent.cpp View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 5 (0 generated)
Vladislav Kaznacheev
7 years, 6 months ago (2013-06-14 13:46:18 UTC) #1
Vladislav Kaznacheev
Please take a look
7 years, 6 months ago (2013-06-17 14:10:40 UTC) #2
pfeldman
lgtm https://codereview.chromium.org/17030009/diff/2001/Source/core/inspector/InspectorAgent.h File Source/core/inspector/InspectorAgent.h (right): https://codereview.chromium.org/17030009/diff/2001/Source/core/inspector/InspectorAgent.h#newcode57 Source/core/inspector/InspectorAgent.h:57: static PassRefPtr<InspectorAgent> create(Page* page, InjectedScriptManager* injectedScriptManager, InstrumentingAgents* instrumentingAgents, ...
7 years, 6 months ago (2013-06-18 15:36:06 UTC) #3
pfeldman
Lets hold it a bit and re-evaluate added complexity.
7 years, 6 months ago (2013-06-19 06:00:33 UTC) #4
Vladislav Kaznacheev
7 years, 6 months ago (2013-06-19 12:58:43 UTC) #5 
https://chromiumcodereview.appspot.com/17030009/diff/2001/Source/core/inspect...
File Source/core/inspector/InspectorAgent.h (right):

https://chromiumcodereview.appspot.com/17030009/diff/2001/Source/core/inspect...
Source/core/inspector/InspectorAgent.h:57: static PassRefPtr<InspectorAgent>
create(Page* page, InjectedScriptManager* injectedScriptManager,
InstrumentingAgents* instrumentingAgents, InspectorState* state)
Discussed, agreed this is not worth it.
On 2013/06/18 15:36:06, pfeldman wrote:
> Make agents ref-counted in a separate patch?

https://chromiumcodereview.appspot.com/17030009/diff/2001/Source/core/inspect...
Source/core/inspector/InspectorAgent.h:110: class InspectorAgentController:
public InspectorBaseController<InspectorAgentController, InspectorAgent> {
On 2013/06/18 15:36:06, pfeldman wrote:
> s/Controller/Factory/

Done.

https://chromiumcodereview.appspot.com/17030009/diff/2001/Source/core/inspect...
File Source/core/inspector/InspectorApplicationCacheAgent.h (right):

https://chromiumcodereview.appspot.com/17030009/diff/2001/Source/core/inspect...
Source/core/inspector/InspectorApplicationCacheAgent.h:55:
~InspectorApplicationCacheAgent();
On 2013/06/18 15:36:06, pfeldman wrote:
> virtual

Done.

https://chromiumcodereview.appspot.com/17030009/diff/2001/Source/core/inspect...
Source/core/inspector/InspectorApplicationCacheAgent.h:84: class
InspectorApplicationCacheController: public
InspectorBaseController<InspectorApplicationCacheController,
InspectorApplicationCacheAgent> {
On 2013/06/18 15:36:06, pfeldman wrote:
>  :

Done.

https://chromiumcodereview.appspot.com/17030009/diff/2001/Source/core/inspect...
File Source/core/inspector/InspectorBaseAgent.h (right):

https://chromiumcodereview.appspot.com/17030009/diff/2001/Source/core/inspect...
Source/core/inspector/InspectorBaseAgent.h:53: virtual void
setFrontend(InspectorFrontend*) { }
On 2013/06/18 15:36:06, pfeldman wrote:
> Add a bunch of fixme-s for these and a link to a meta bug.

Done.

https://chromiumcodereview.appspot.com/17030009/diff/2001/Source/core/inspect...
Source/core/inspector/InspectorBaseAgent.h:68:
InspectorBaseControllerInterface(const String& name, InstrumentingAgents*,
InspectorCompositeState*);
We will eventually
On 2013/06/18 15:36:06, pfeldman wrote:
> Nit: I'd push it even further.

https://chromiumcodereview.appspot.com/17030009/diff/2001/Source/core/inspect...
Source/core/inspector/InspectorBaseAgent.h:69:
~InspectorBaseControllerInterface();
On 2013/06/18 15:36:06, pfeldman wrote:
> virtual ?

Done.

https://chromiumcodereview.appspot.com/17030009/diff/2001/Source/core/inspect...
Source/core/inspector/InspectorBaseAgent.h:77: virtual void discardAgent();
Renamed to discardDependencies because that is what it does.
On 2013/06/18 15:36:06, pfeldman wrote:
> discardController

https://chromiumcodereview.appspot.com/17030009/diff/2001/Source/core/inspect...
Source/core/inspector/InspectorBaseAgent.h:107: template<typename T, typename
AGENT>
On 2013/06/18 15:36:06, pfeldman wrote:
> Agent

Done.

https://chromiumcodereview.appspot.com/17030009/diff/2001/Source/core/inspect...
Source/core/inspector/InspectorBaseAgent.h:116: AGENT* getAgent() { return
static_cast<AGENT*>(m_agent.get()); }
On 2013/06/18 15:36:06, pfeldman wrote:
> agent()

Done.

https://chromiumcodereview.appspot.com/17030009/diff/2001/Source/core/inspect...
Source/core/inspector/InspectorBaseAgent.h:119: virtual AGENT*
getCommandHandler() { return getAgent(); }
On 2013/06/18 15:36:06, pfeldman wrote:
> commandHandler

Done.

https://chromiumcodereview.appspot.com/17030009/diff/2001/Source/core/inspect...
File Source/core/inspector/InspectorController.cpp (right):

https://chromiumcodereview.appspot.com/17030009/diff/2001/Source/core/inspect...
Source/core/inspector/InspectorController.cpp:97:
m_agents.append(InspectorCSSController::create(m_instrumentingAgents.get(),
m_state.get(), domController, pageController));
On 2013/06/18 15:36:06, pfeldman wrote:
> m_agent_factories?

Done.

https://chromiumcodereview.appspot.com/17030009/diff/2001/Source/core/inspect...
File Source/core/inspector/InspectorWorkerAgent.h (right):

https://chromiumcodereview.appspot.com/17030009/diff/2001/Source/core/inspect...
Source/core/inspector/InspectorWorkerAgent.h:86: class
InspectorWorkerController: public
InspectorBaseController<InspectorWorkerController, InspectorWorkerAgent> {
On 2013/06/18 15:36:06, pfeldman wrote:
> Space before :

Done.

Powered by Google App Engine
This is Rietveld 408576698