Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2657703003:
Clean up updateSelectionIfSelectAll using SelectionBuilder (Closed)
|
Created:
3 years, 11 months ago by yoichio Modified:
3 years, 11 months ago Reviewers:
yosin_UTC9 CC:
blink-reviews, chromium-reviews Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionClean up updateSelectionIfSelectAll using SelectionBuilder
This CL does:
1. Rename updateSelectionIfSelectAll to applySelectAll and have the function
returning VisibleSelection.
2. Use SelectionBuilder to update selection so that we avoid call
VisibleSelection::validate twice.
TEST=No change in behavior
Review-Url: https://codereview.chromium.org/2657703003
Cr-Commit-Position: refs/heads/master@{#446204}
Committed: https://chromium.googlesource.com/chromium/src/+/3c72dd4fcdb2ece2e571bf9b92a23949b9ea438b
Patch Set 1 #
Total comments: 2
Patch Set 2 : update #Messages
Total messages: 19 (13 generated)
The CQ bit was checked by yoichio@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== cleanup applyselectall BUG= ========== to ========== Clean up updateSelectionIfSelectAll using SelectionBuilder This CL does: 1. Rename updateSelectionIfSelectAll to applySelectAll and have the function returning VisibleSelection. 2. Use SelectionBuilder to update selection so that we avoid call VisibleSelection::validate twice. TEST=No change in behavior ==========
yoichio@chromium.org changed reviewers: + yosin@chromium.org
https://codereview.chromium.org/2657703003/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/editing/SelectionController.cpp (right): https://codereview.chromium.org/2657703003/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/editing/SelectionController.cpp:281: static VisibleSelectionInFlatTree applySelectAll( Could you make |applySelectAll()| to return |SelecitonInFlatTree| to reduce number of usage of createVisibleSeleciton()? Since there is only one call site of |applySelectAll()|, we can have only one createVS().
The CQ bit was checked by yoichio@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2657703003/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/editing/SelectionController.cpp (right): https://codereview.chromium.org/2657703003/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/editing/SelectionController.cpp:281: static VisibleSelectionInFlatTree applySelectAll( On 2017/01/25 06:41:05, Yosi_UTC9 wrote: > Could you make |applySelectAll()| to return |SelecitonInFlatTree| to reduce > number of usage of createVisibleSeleciton()? > > Since there is only one call site of |applySelectAll()|, we can have only one > createVS(). Done.
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by yoichio@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1485395614403840,
"parent_rev": "a27eadfd9c7fa06a37d22f717c34b1a68370bc03", "commit_rev":
"3c72dd4fcdb2ece2e571bf9b92a23949b9ea438b"}
Message was sent while issue was closed.
Description was changed from ========== Clean up updateSelectionIfSelectAll using SelectionBuilder This CL does: 1. Rename updateSelectionIfSelectAll to applySelectAll and have the function returning VisibleSelection. 2. Use SelectionBuilder to update selection so that we avoid call VisibleSelection::validate twice. TEST=No change in behavior ========== to ========== Clean up updateSelectionIfSelectAll using SelectionBuilder This CL does: 1. Rename updateSelectionIfSelectAll to applySelectAll and have the function returning VisibleSelection. 2. Use SelectionBuilder to update selection so that we avoid call VisibleSelection::validate twice. TEST=No change in behavior Review-Url: https://codereview.chromium.org/2657703003 Cr-Commit-Position: refs/heads/master@{#446204} Committed: https://chromium.googlesource.com/chromium/src/+/3c72dd4fcdb2ece2e571bf9b92a2... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/3c72dd4fcdb2ece2e571bf9b92a2... |
