Bug 52955

Summary: Web Inspector: first step of splitting InspectorController
Product: WebKit Reporter: Ilya Tikhonovsky <loislo>
Component: Web Inspector (Deprecated)Assignee: Ilya Tikhonovsky <loislo>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 52510    
Attachments:
Description Flags
[patch] initial version
none
[patch] initial version. Rebased.
pfeldman: review-
[PATCH] More user-friendly diff (shows diff in copy) none

Description Ilya Tikhonovsky 2011-01-22 03:59:04 PST
Splitting InspectorController is a bit tricky process. Especially when 4 people commit a number of patches a day.

As a first step I'll do the next things:
1) rename existing InspectorController to InspectorAgent;
2) s/nspectorController/nspectorAgent/g everywhere in WebCore/inspector but InspectorInstrumentation;
3) create a fake InspectorController as a child of InspectorAgent for the rest of WebCore and WebKit;

The second step is a migration a small set of functions described in bug 52510 from InspectorAgent to InspectorController.

As far as the second step will have significant changes in the functions' implementation I'll do it as a separate patch.
Comment 1 Ilya Tikhonovsky 2011-01-22 04:11:35 PST
Created attachment 79840 [details]
[patch] initial version
Comment 2 Ilya Tikhonovsky 2011-01-22 04:23:52 PST
Created attachment 79841 [details]
[patch] initial version. Rebased.
Comment 3 Ilya Tikhonovsky 2011-01-22 04:33:57 PST
looks like efl has no dependency between InspectorBackendDispatcher and its generator CodeGeneratorInpsector.pm
Comment 4 Pavel Feldman 2011-01-23 00:11:45 PST
Comment on attachment 79841 [details]
[patch] initial version. Rebased. 

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

Please provide proper diff for InspectorAgent copy. Rest looks good.

> Source/WebCore/ChangeLog:5
> +        Web Inspector: first step of splitting InspectorController.

I'd say it is 41st step :)

> Source/WebCore/ChangeLog:11
> +        2) s/nspectorController/nspectorAgent/g everywhere in WebCore/inspector but InspectorInstrumentation;

Why not inspector instrumentation? Too many changes?

> Source/WebCore/ChangeLog:12
> +        3) create a fake InspectorController as a child of InspectorAgent for the rest of WebCore and WebKit;

"as a child of" -> "derived from"

> Source/WebCore/ChangeLog:14
> +        The second step is a migration a small set of functions described in bug 52510 from InspectorAgent to InspectorController.

is a migration _of_ small

> Source/WebCore/WebCore.exp.in:1359
> +__ZN7WebCore14InspectorAgent4showEv

This part is unnecessary. In the end of the day, WebCore should not expose InspectorAgent. But I see that it is a temporary measure.

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:1268
> +		4F2D205412EAE7B3005C2874 /* InspectorAgent.h in Headers */ = {isa = PBXBuildFile; fileRef = 4F2D205212EAE7B3005C2874 /* InspectorAgent.h */; settings = {ATTRIBUTES = (Private, ); }; };

Same as for exports, ATTRIBUTES=Private should not be there after the refactoring.

> Source/WebCore/inspector/InspectorAgent.cpp:1
> +/*

Please add

[diff]
        renames = copies

In the .git/config

so that we could see the changes in the file.
Comment 5 Pavel Feldman 2011-01-23 00:32:20 PST
Created attachment 79864 [details]
[PATCH] More user-friendly diff (shows diff in copy)
Comment 6 Pavel Feldman 2011-01-23 01:37:47 PST
Committed r76459: <http://trac.webkit.org/changeset/76459>