Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(33)

Issue 2833043002: [DMC #1.93] Make DocumentMarkerController::AddMarker() take a pointer instead of a const ref (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 months ago by rlanday
Modified:
3 months, 4 weeks ago
Reviewers:
yosin_UTC9, Xiaocheng
CC:
blink-reviews, chromium-reviews
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Make DocumentMarkerController::AddMarker() take a pointer instead of a const ref Right now this method can take a const reference despite DMC possibly later needing to edit the marker because the current implementation always copies the marker (to construct a RenderedDocumentMarker). However, this is not guaranteed by the API, and will be changed in the future once we no longer construct RenderedDocumentMarkers for all MarkerTypes. So this method needs to be changed to take a non-const pointer (could use a ref but I think doing so is discouraged by the style guide). Also, the methods that call AddMarker() need to pass pointers to heap-allocated objects. BUG=707867 Review-Url: https://codereview.chromium.org/2833043002 Cr-Commit-Position: refs/heads/master@{#466596} Committed: https://chromium.googlesource.com/chromium/src/+/9ad9f6860b4d6a4ec7f7f975b2c99672e02d5d49

Patch Set 1 #

Patch Set 2 : Rebase #

Patch Set 3 : Remove dep #

Messages

Total messages: 17 (13 generated)
rlanday
4 months ago (2017-04-21 08:15:29 UTC) #3
yosin_UTC9
lgtm
3 months, 4 weeks ago (2017-04-24 04:05:45 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2833043002/40001
3 months, 4 weeks ago (2017-04-24 06:00:58 UTC) #14
commit-bot: I haz the power
3 months, 4 weeks ago (2017-04-24 08:31:00 UTC) #17
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/9ad9f6860b4d6a4ec7f7f975b2c9...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b