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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 330029: DevTools: Remove base/ dependencies from glue/devtools (Closed)

Created:
11 years, 1 month ago by pfeldman
Modified:
9 years, 7 months ago
Reviewers:
yurys
CC:
chromium-reviews_googlegroups.com, darin (slow to review), pam+watch_chromium.org, Paweł Hajdan Jr.
Visibility:
Public.

Description

DevTools: Remove base/ dependencies from glue/devtools BUG=24622, 24597 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=30330

Patch Set 1 #

Patch Set 2 : '' #

Patch Set 3 : '' #

Patch Set 4 : '' #

Patch Set 5 : '' #

Patch Set 6 : '' #

Patch Set 7 : '' #

Total comments: 20

Patch Set 8 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+242 lines, -792 lines) Patch
M chrome/renderer/devtools_client.h View 3 4 5 6 7 3 chunks +16 lines, -12 lines 0 comments Download
M chrome/renderer/devtools_client.cc View 4 5 6 7 4 chunks +12 lines, -11 lines 0 comments Download
A + webkit/api/public/WebDevToolsFrontend.h View 4 5 6 7 1 chunk +50 lines, -27 lines 0 comments Download
A + webkit/api/public/WebDevToolsFrontendClient.h View 4 5 6 7 1 chunk +51 lines, -26 lines 0 comments Download
M webkit/glue/devtools/bound_object.h View 1 2 3 4 5 6 7 2 chunks +3 lines, -2 lines 0 comments Download
M webkit/glue/devtools/debugger_agent_impl.h View 1 2 3 4 5 6 7 3 chunks +2 lines, -3 lines 0 comments Download
M webkit/glue/devtools/debugger_agent_impl.cc View 2 chunks +1 line, -2 lines 0 comments Download
M webkit/glue/devtools/debugger_agent_manager.h View 1 2 3 4 5 6 7 4 chunks +3 lines, -6 lines 0 comments Download
M webkit/glue/devtools/debugger_agent_manager.cc View 6 chunks +7 lines, -8 lines 0 comments Download
M webkit/glue/devtools/devtools_rpc.h View 1 2 3 4 5 6 7 6 chunks +5 lines, -11 lines 0 comments Download
M webkit/glue/devtools/devtools_rpc_js.h View 1 2 3 4 5 6 7 3 chunks +2 lines, -3 lines 0 comments Download
M webkit/glue/webdevtoolsagent_impl.h View 1 2 3 4 5 6 7 3 chunks +3 lines, -2 lines 0 comments Download
M webkit/glue/webdevtoolsclient.h View 1 2 3 1 chunk +0 lines, -39 lines 0 comments Download
M webkit/glue/webdevtoolsclient_delegate.h View 1 2 3 1 chunk +0 lines, -36 lines 0 comments Download
M webkit/glue/webdevtoolsclient_impl.h View 1 2 3 1 chunk +0 lines, -94 lines 0 comments Download
M webkit/glue/webdevtoolsclient_impl.cc View 1 2 3 1 chunk +0 lines, -422 lines 0 comments Download
A + webkit/glue/webdevtoolsfrontend_impl.h View 6 chunks +12 lines, -13 lines 0 comments Download
A + webkit/glue/webdevtoolsfrontend_impl.cc View 15 chunks +71 lines, -71 lines 0 comments Download
M webkit/webkit.gyp View 2 chunks +4 lines, -4 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
pfeldman
11 years, 1 month ago (2009-10-27 11:22:49 UTC) #1
yurys
LGTM if the pointed issues are resolved http://codereview.chromium.org/330029/diff/8001/9018 File chrome/renderer/devtools_client.cc (right): http://codereview.chromium.org/330029/diff/8001/9018#newcode17 Line 17: using ...
11 years, 1 month ago (2009-10-27 15:34:20 UTC) #2
pfeldman
11 years, 1 month ago (2009-10-27 16:03:10 UTC) #3 
Thanks for the review!

http://codereview.chromium.org/330029/diff/8001/9018
File chrome/renderer/devtools_client.cc (right):

http://codereview.chromium.org/330029/diff/8001/9018#newcode17
Line 17: using WebKit::WebDevToolsFrontend;
On 2009/10/27 15:34:20, Yury Semikhatsky wrote:
> wrong order of using statements

Done.

http://codereview.chromium.org/330029/diff/8001/9013
File chrome/renderer/devtools_client.h (right):

http://codereview.chromium.org/330029/diff/8001/9013#newcode39
Line 39: // WebDevToolsFrontendDelegate implementation
On 2009/10/27 15:34:20, Yury Semikhatsky wrote:
>  WebDevToolsFrontendDelegate implementation -> WebDevToolsFrontendClient
> implementation

Done.

http://codereview.chromium.org/330029/diff/8001/9015
File webkit/api/public/WebDevToolsFrontend.h (right):

http://codereview.chromium.org/330029/diff/8001/9015#newcode42
Line 42: // WebDevToolsFrontend represents DevTools frontend sitting in the
Glue. It provides
On 2009/10/27 15:34:20, Yury Semikhatsky wrote:
> the comment is a bit confusing

Done.

http://codereview.chromium.org/330029/diff/8001/9014
File webkit/api/public/WebDevToolsFrontendClient.h (right):

http://codereview.chromium.org/330029/diff/8001/9014#newcode56
Line 56: ~WebDevToolsFrontendClient() {}
On 2009/10/27 15:34:20, Yury Semikhatsky wrote:
> should be  virtual

Done.

http://codereview.chromium.org/330029/diff/8001/9010
File webkit/glue/devtools/bound_object.h (left):

http://codereview.chromium.org/330029/diff/8001/9010#oldcode28
Line 28: DISALLOW_COPY_AND_ASSIGN(BoundObject);
On 2009/10/27 15:34:20, Yury Semikhatsky wrote:
> is there a counterpart for this macros that we could use instead? I think
> inheriting from Noncopyable is the preferred way.

Done.

http://codereview.chromium.org/330029/diff/8001/9011
File webkit/glue/devtools/debugger_agent_impl.h (left):

http://codereview.chromium.org/330029/diff/8001/9011#oldcode76
Line 76: DISALLOW_COPY_AND_ASSIGN(DebuggerAgentImpl);
On 2009/10/27 15:34:20, Yury Semikhatsky wrote:
> inherit from Noncopyable instead

Done.

http://codereview.chromium.org/330029/diff/8001/9003
File webkit/glue/devtools/debugger_agent_manager.h (left):

http://codereview.chromium.org/330029/diff/8001/9003#oldcode99
Line 99: DISALLOW_COPY_AND_ASSIGN(DebuggerAgentManager);
On 2009/10/27 15:34:20, Yury Semikhatsky wrote:
> inherit from Noncopyable instead

Done.

http://codereview.chromium.org/330029/diff/8001/9005
File webkit/glue/devtools/devtools_rpc.h (left):

http://codereview.chromium.org/330029/diff/8001/9005#oldcode290
Line 290: DISALLOW_COPY_AND_ASSIGN(DevToolsRpc);
On 2009/10/27 15:34:20, Yury Semikhatsky wrote:
> inherit from Noncopyable instead

Done.

http://codereview.chromium.org/330029/diff/8001/9004
File webkit/glue/devtools/devtools_rpc_js.h (left):

http://codereview.chromium.org/330029/diff/8001/9004#oldcode109
Line 109: DISALLOW_COPY_AND_ASSIGN(Js##Class##BoundObj); \
On 2009/10/27 15:34:20, Yury Semikhatsky wrote:
> inherit from Noncopyable instead

Done.

http://codereview.chromium.org/330029/diff/8001/9009
File webkit/glue/webdevtoolsagent_impl.h (left):

http://codereview.chromium.org/330029/diff/8001/9009#oldcode122
Line 122: DISALLOW_COPY_AND_ASSIGN(WebDevToolsAgentImpl);
On 2009/10/27 15:34:20, Yury Semikhatsky wrote:
> inherit from Noncopyable instead

Done.

Powered by Google App Engine
This is Rietveld 408576698