DevTools: deoilpanize inspector/v8 and related classes.

DevTools: deoilpanize inspector/v8 and related classes.

Committed: https://crrev.com/7fb9d70481852fcaf711498d3975ef2df15b4256
Cr-Commit-Position: refs/heads/master@{#370445}

[pfeldman, 2016-01-19 23:22:01]

pfeldman@chromium.org changed reviewers:
+ dgozman@chromium.org, yurys@chromium.org

[haraken, 2016-01-19 23:27:44]

haraken@chromium.org changed reviewers:
+ haraken@chromium.org, oilpan-reviews@chromium.org

[sof, 2016-01-20 11:39:49]

sigbjornf@opera.com changed reviewers:
+ sigbjornf@opera.com

[sof, 2016-01-20 11:39:50]

lgtm, conversion appears complete & unproblematic.

https://codereview.chromium.org/1601283003/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/ScriptCallStackFactory.h
(right):

https://codereview.chromium.org/1601283003/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/ScriptCallStackFactory.h:36: #include
"platform/heap/Handle.h"
If we didn't need it before, won't need it now?

https://codereview.chromium.org/1601283003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/frame/FrameConsole.cpp (right):

https://codereview.chromium.org/1601283003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/FrameConsole.cpp:104:
RefPtr<ScriptCallStack> reportedCallStack = nullptr;
(if you like/prefer, there's no need to initialize this RefPtr<> now. here & in
a few other places in this CL.)

https://codereview.chromium.org/1601283003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/inspector/InspectorRuntimeAgent.h (right):

https://codereview.chromium.org/1601283003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/inspector/InspectorRuntimeAgent.h:119:
InjectedScriptManager* m_injectedScriptManager;
( meta comment: would it be worth documenting in the code somewhere the lifetime
of these bare 'manager' pointers? )

https://codereview.chromium.org/1601283003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/inspector/ScriptCallFrame.h (left):

https://codereview.chromium.org/1601283003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/inspector/ScriptCallFrame.h:44:
DISALLOW_NEW_EXCEPT_PLACEMENT_NEW();
It wouldn't hurt to keep this; not Oilpan specific.

[haraken, 2016-01-20 11:48:17]

LGTM

[pfeldman, 2016-01-20 17:53:28]

Thank you for your thorough review!

https://codereview.chromium.org/1601283003/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/ScriptCallStackFactory.h
(right):

https://codereview.chromium.org/1601283003/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/ScriptCallStackFactory.h:36: #include
"platform/heap/Handle.h"
On 2016/01/20 11:39:49, sof wrote:
> If we didn't need it before, won't need it now?

We used to transitively get it from the ScriptCallStack.h above.

https://codereview.chromium.org/1601283003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/frame/FrameConsole.cpp (right):

https://codereview.chromium.org/1601283003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/FrameConsole.cpp:104:
RefPtr<ScriptCallStack> reportedCallStack = nullptr;
On 2016/01/20 11:39:50, sof wrote:
> (if you like/prefer, there's no need to initialize this RefPtr<> now. here &
in
> a few other places in this CL.)

Done.

https://codereview.chromium.org/1601283003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/inspector/InspectorRuntimeAgent.h (right):

https://codereview.chromium.org/1601283003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/inspector/InspectorRuntimeAgent.h:119:
InjectedScriptManager* m_injectedScriptManager;
On 2016/01/20 11:39:50, sof wrote:
> ( meta comment: would it be worth documenting in the code somewhere the
lifetime
> of these bare 'manager' pointers? )

It is pretty clear from the fact that it stands out as a raw pointer and
corresponding OwnPtr location in the agent host. It is also likely to move along
with the inspector/v8.

https://codereview.chromium.org/1601283003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/inspector/ScriptCallFrame.h (left):

https://codereview.chromium.org/1601283003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/inspector/ScriptCallFrame.h:44:
DISALLOW_NEW_EXCEPT_PLACEMENT_NEW();
On 2016/01/20 11:39:50, sof wrote:
> It wouldn't hurt to keep this; not Oilpan specific.

Done.

[pfeldman, 2016-01-20 17:56:40]

The CQ bit was checked by pfeldman@chromium.org

[pfeldman, 2016-01-20 17:56:40]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, sigbjornf@opera.com
Link to the patchset: https://codereview.chromium.org/1601283003/#ps20001
(title: "review comments addressed.")

[commit-bot: I haz the power, 2016-01-20 17:57:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1601283003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1601283003/20001

[commit-bot: I haz the power, 2016-01-20 19:48:45]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-20 19:50:33]

Description was changed from

==========
DevTools: deoilpanize inspector/v8 and related classes.
==========

to

==========
DevTools: deoilpanize inspector/v8 and related classes.

Committed: https://crrev.com/7fb9d70481852fcaf711498d3975ef2df15b4256
Cr-Commit-Position: refs/heads/master@{#370445}
==========

[commit-bot: I haz the power, 2016-01-20 19:50:34]

Patchset 2 (id:??) landed as
https://crrev.com/7fb9d70481852fcaf711498d3975ef2df15b4256
Cr-Commit-Position: refs/heads/master@{#370445}