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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 7089017: Change source look-up design (Closed)

Created:
9 years, 6 months ago by Peter Rybin
Modified:
9 years, 6 months ago
Reviewers:
apavlov
CC:
chromedevtools-dev_googlegroups.com
Visibility:
Public.

Description

Change source look-up design Committed: http://code.google.com/p/chromedevtools/source/detail?r=650

Patch Set 1 #

Patch Set 2 : clean #

Patch Set 3 : clean #

Total comments: 6

Patch Set 4 : merge #

Patch Set 5 : fcr #

Unified diffs Side-by-side diffs Delta from patch set Stats (+357 lines, -175 lines) Patch
M plugins/org.chromium.debug.core/src/org/chromium/debug/core/ChromiumDebugPlugin.java View 2 chunks +5 lines, -9 lines 0 comments Download
M plugins/org.chromium.debug.core/src/org/chromium/debug/core/ChromiumSourceDirector.java View 1 2 3 3 chunks +80 lines, -25 lines 0 comments Download
M plugins/org.chromium.debug.core/src/org/chromium/debug/core/ReverseSourceLookup.java View 1 chunk +1 line, -1 line 0 comments Download
M plugins/org.chromium.debug.core/src/org/chromium/debug/core/VProjectSourceContainer.java View 1 2 1 chunk +1 line, -5 lines 0 comments Download
M plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/BreakpointSynchronizer.java View 1 2 3 4 8 chunks +42 lines, -3 lines 0 comments Download
M plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/ChromiumLineBreakpoint.java View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/ConnectedTargetData.java View 2 chunks +3 lines, -1 line 0 comments Download
M plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/ResourceManager.java View 1 2 10 chunks +57 lines, -25 lines 0 comments Download
M plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/VProjectWorkspaceBridge.java View 3 chunks +5 lines, -4 lines 0 comments Download
M plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/VmResourceId.java View 1 2 3 1 chunk +32 lines, -74 lines 0 comments Download
M plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/WorkspaceBridge.java View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M plugins/org.chromium.debug.core/src/org/chromium/debug/core/sourcemap/PositionMapBuilderImpl.java View 6 chunks +49 lines, -7 lines 0 comments Download
M plugins/org.chromium.debug.core/src/org/chromium/debug/core/util/ChromiumDebugPluginUtil.java View 1 2 3 4 1 chunk +16 lines, -0 lines 0 comments Download
M plugins/org.chromium.debug.core/src/org/chromium/debug/core/util/ScriptTargetMapping.java View 1 2 3 4 4 chunks +35 lines, -12 lines 0 comments Download
M plugins/org.chromium.debug.ui/src/org/chromium/debug/ui/actions/PushChangesAction.java View 2 chunks +5 lines, -2 lines 0 comments Download
M plugins/org.chromium.debug.ui/src/org/chromium/debug/ui/liveedit/PushResultParser.java View 1 chunk +1 line, -1 line 0 comments Download
M plugins/org.chromium.debug.ui/src/org/chromium/debug/ui/propertypages/Messages.java View 1 chunk +2 lines, -0 lines 0 comments Download
M plugins/org.chromium.debug.ui/src/org/chromium/debug/ui/propertypages/ScriptFilePage.java View 1 2 2 chunks +19 lines, -5 lines 0 comments Download
M plugins/org.chromium.debug.ui/src/org/chromium/debug/ui/propertypages/messages.properties View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Peter Rybin
9 years, 6 months ago (2011-06-10 23:40:49 UTC) #1
apavlov
LGTM with comments http://codereview.chromium.org/7089017/diff/4001/plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/BreakpointSynchronizer.java File plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/BreakpointSynchronizer.java (right): http://codereview.chromium.org/7089017/diff/4001/plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/BreakpointSynchronizer.java#newcode402 plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/BreakpointSynchronizer.java:402: enum Problem { blank above http://codereview.chromium.org/7089017/diff/4001/plugins/org.chromium.debug.core/src/org/chromium/debug/core/util/ChromiumDebugPluginUtil.java ...
9 years, 6 months ago (2011-06-14 14:54:36 UTC) #2
Peter Rybin
9 years, 6 months ago (2011-06-16 08:50:24 UTC) #3 
http://codereview.chromium.org/7089017/diff/4001/plugins/org.chromium.debug.c...
File
plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/BreakpointSynchronizer.java
(right):

http://codereview.chromium.org/7089017/diff/4001/plugins/org.chromium.debug.c...
plugins/org.chromium.debug.core/src/org/chromium/debug/core/model/BreakpointSynchronizer.java:402:
enum Problem {
On 2011/06/14 14:54:36, apavlov wrote:
> blank above

Done.

http://codereview.chromium.org/7089017/diff/4001/plugins/org.chromium.debug.c...
File
plugins/org.chromium.debug.core/src/org/chromium/debug/core/util/ChromiumDebugPluginUtil.java
(right):

http://codereview.chromium.org/7089017/diff/4001/plugins/org.chromium.debug.c...
plugins/org.chromium.debug.core/src/org/chromium/debug/core/util/ChromiumDebugPluginUtil.java:373:
return one.equals(two);
On 2011/06/14 14:54:36, apavlov wrote:
> this line is more than suspicious. The usual parameter names are "left" and
> "right" for similar equality checks.

Done.

http://codereview.chromium.org/7089017/diff/4001/plugins/org.chromium.debug.c...
File
plugins/org.chromium.debug.core/src/org/chromium/debug/core/util/ScriptTargetMapping.java
(right):

http://codereview.chromium.org/7089017/diff/4001/plugins/org.chromium.debug.c...
plugins/org.chromium.debug.core/src/org/chromium/debug/core/util/ScriptTargetMapping.java:20:
/** Original file that is being mapped, may be workspace file or virtual project
file. */
On 2011/06/14 14:54:36, apavlov wrote:
> blank line above

Done.

Powered by Google App Engine
This is Rietveld 408576698