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

Issue 2828093004: [DMC #1.8] Convert static comparator functions in DocumentMarkerController to lambdas (Closed)

Created:
3 years, 8 months ago by rlanday
Modified:
3 years, 8 months ago
Reviewers:
yosin_UTC9, Xiaocheng
CC:
blink-reviews, chromium-reviews
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Convert static comparator functions in DocumentMarkerController to lambdas After https://codereview.chromium.org/2818873002, we will need to use EndsBefore() in more than one file. yosin advised that these functions are small enough that they should be lambdas rather than trying to add them e.g. as member functions on DocumentMarker to avoid duplication. BUG=707867 Review-Url: https://codereview.chromium.org/2828093004 Cr-Commit-Position: refs/heads/master@{#465960} Committed: https://chromium.googlesource.com/chromium/src/+/74ad4fde9304fc6f71a165462360662c5bfe7924

Patch Set 1 #

Total comments: 1

Patch Set 2 : Convert DMC methods to lambdas #

Patch Set 3 : Add missing files #

Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -30 lines) Patch
M third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp View 1 7 chunks +33 lines, -30 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 19 (10 generated)
rlanday
3 years, 8 months ago (2017-04-20 04:00:40 UTC) #2
Xiaocheng
lgtm
3 years, 8 months ago (2017-04-20 04:04:25 UTC) #3
yosin_UTC9
https://codereview.chromium.org/2828093004/diff/1/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp File third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp (right): https://codereview.chromium.org/2828093004/diff/1/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp#newcode411 third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp:411: list->begin(), list->end(), start_offset, DocumentMarker::EndsBefore); Do we really want to ...
3 years, 8 months ago (2017-04-20 04:58:01 UTC) #4
rlanday
On 2017/04/20 at 04:58:01, yosin wrote: > We could write: > > MarkerList::iterator start_pos = ...
3 years, 8 months ago (2017-04-20 05:06:13 UTC) #5
rlanday
Updated (Ignore my comment in the description for Patch 3 about missing files, there really ...
3 years, 8 months ago (2017-04-20 06:33:04 UTC) #7
Xiaocheng
Still lgtm.
3 years, 8 months ago (2017-04-20 07:13:23 UTC) #10
yosin_UTC9
lgtm Please adjust description to make all lines < 80 chars for git log friendly.
3 years, 8 months ago (2017-04-20 09:02:31 UTC) #13
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/2828093004/40001
3 years, 8 months ago (2017-04-20 09:45:06 UTC) #16
commit-bot: I haz the power
3 years, 8 months ago (2017-04-20 09:48:53 UTC) #19
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/74ad4fde9304fc6f71a165462360...

Powered by Google App Engine
This is Rietveld 408576698