Bug 127605 – Web Inspector: Move InspectorRuntimeAgent into JavaScriptCore

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

All requirements (InjectedScripts and ScriptDebugServer) are met. We can now move down InspectorRuntimeAgent.

[Radar WebKit Bug Importer]

<rdar://problem/15908355>

[Joseph Pecoraro]

Created attachment 222189 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Created attachment 222190 [details]
[PATCH] For Bots 1

[WebKit Commit Bot]

Attachment 222190 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/inspector/agents/JSGlobalObjectRuntimeAgent.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/inspector/WorkerRuntimeAgent.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Timothy Hatcher]

Comment on attachment 222189 [details]
[PATCH] Proposed Fix

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

> Source/JavaScriptCore/inspector/agents/InspectorRuntimeAgent.cpp:2
> + * Copyright (C) 2013 Apple Inc. All rights reserved.

2013, 2014?

> Source/JavaScriptCore/inspector/agents/JSGlobalObjectRuntimeAgent.cpp:2
> + * Copyright (C) 2013 Apple Inc. All rights reserved.

2014

> Source/JavaScriptCore/inspector/agents/JSGlobalObjectRuntimeAgent.h:2
> + * Copyright (C) 2013 Apple Inc. All rights reserved.

2014

> Source/WebCore/inspector/PageRuntimeAgent.cpp:56
> +PageRuntimeAgent::PageRuntimeAgent(InjectedScriptManager* injectedScriptManager, Page* page, InspectorPageAgent* pageAgent)

We might want to consider putting Inspector back in these subclass names. Page or Web?

> Source/WebCore/inspector/PageRuntimeAgent.h:70
> +    virtual JSC::VM* globalVM() override;

final too?

> Source/WebCore/inspector/WorkerRuntimeAgent.h:59
> +    virtual JSC::VM* globalVM() override;

Ditto.

[Timothy Hatcher]

Comment on attachment 222189 [details]
[PATCH] Proposed Fix

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

> Source/WebCore/inspector/PageRuntimeAgent.h:56
> +class PageRuntimeAgent final : public Inspector::InspectorRuntimeAgent {

Oh, does final on the class apply to all the override virtual functions?

[Joseph Pecoraro]

(In reply to comment #6)
> (From update of attachment 222189 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=222189&action=review
> 
> > Source/WebCore/inspector/PageRuntimeAgent.h:56
> > +class PageRuntimeAgent final : public Inspector::InspectorRuntimeAgent {
> 
> Oh, does final on the class apply to all the override virtual functions?

Yep!

[Joseph Pecoraro]

> 
> > Source/JavaScriptCore/inspector/agents/JSGlobalObjectRuntimeAgent.cpp:2
> > + * Copyright (C) 2013 Apple Inc. All rights reserved.
> 
> 2014

Hehe, good catch on these. I had written this in 2013 though, I didn't copy/paste error it =). I'll update them.


> > Source/WebCore/inspector/PageRuntimeAgent.cpp:56
> > +PageRuntimeAgent::PageRuntimeAgent(InjectedScriptManager* injectedScriptManager, Page* page, InspectorPageAgent* pageAgent)
> 
> We might want to consider putting Inspector back in these subclass names. Page or Web?

Names could get ridiculously long. Lets keep these JSGlobalObject/Web/Page/Worker. And if we need to change we can change all at once.

[Joseph Pecoraro]

Landed <http://trac.webkit.org/changeset/162767>.