Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2772223003:
Removing mPendingInvalidateContentRect (Closed)
DescriptionRemoving mPendingInvalidateContentRect
Previously when the action mode was hidden invalidating the content rect
was postponed until after the action mode was unhidden. This patch
simplifies that code by invalidating the content rect even when the
action mode is hidden.
Review-Url: https://codereview.chromium.org/2772223003
Cr-Commit-Position: refs/heads/master@{#459974}
Committed: https://chromium.googlesource.com/chromium/src/+/664cdebf1b3d33209cc38e4366bfc332fdc28819
Patch Set 1 #
Messages
Total messages: 19 (10 generated)
The CQ bit was checked by amaralp@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.
amaralp@chromium.org changed reviewers: + boliu@chromium.org, jinsukkim@chromium.org
PTAL
On 2017/03/28 00:05:15, amaralp wrote: > PTAL Honestly not sure what the variable was for... did you test to make sure there is no side effect? It is called by floating paste popup.
lgtm
The CQ bit was checked by amaralp@chromium.org
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 amaralp@chromium.org
On 2017/03/28 at 00:31:34, jinsukkim wrote: > On 2017/03/28 00:05:15, amaralp wrote: > > PTAL > > Honestly not sure what the variable was for... did you test to make sure there is no side effect? It is called by floating paste popup. What do you mean it is called by floating paste popup? I think the invalidating content rect business is only for the selection action mode and not the paste popup.
On 2017/03/28 00:37:58, amaralp wrote: > On 2017/03/28 at 00:31:34, jinsukkim wrote: > > On 2017/03/28 00:05:15, amaralp wrote: > > > PTAL > > > > Honestly not sure what the variable was for... did you test to make sure there > is no side effect? It is called by floating paste popup. > > What do you mean it is called by floating paste popup? I think the invalidating > content rect business is only for the selection action mode and not the paste > popup. My bad. Saw it wrong.
On 2017/03/28 at 00:41:08, jinsukkim wrote: > On 2017/03/28 00:37:58, amaralp wrote: > > On 2017/03/28 at 00:31:34, jinsukkim wrote: > > > On 2017/03/28 00:05:15, amaralp wrote: > > > > PTAL > > > > > > Honestly not sure what the variable was for... did you test to make sure there > > is no side effect? It is called by floating paste popup. > > > > What do you mean it is called by floating paste popup? I think the invalidating > > content rect business is only for the selection action mode and not the paste > > popup. > > My bad. Saw it wrong. I've tested the change on my Nexus 6P and can't tell a difference. The only potential side effect I can think of is that the extra calls to invalidateContentRect could be a performance issue. For example during scrolling you'd have lots of calls. I haven't noticed that so I think the change is safe.
The CQ bit was checked by amaralp@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": 1, "attempt_start_ts": 1490664517937950, "parent_rev":
"76ad12fee73e4512dbb9c7a79ffb6cec33fae6f2", "commit_rev":
"664cdebf1b3d33209cc38e4366bfc332fdc28819"}
Message was sent while issue was closed.
Description was changed from ========== Removing mPendingInvalidateContentRect Previously when the action mode was hidden invalidating the content rect was postponed until after the action mode was unhidden. This patch simplifies that code by invalidating the content rect even when the action mode is hidden. ========== to ========== Removing mPendingInvalidateContentRect Previously when the action mode was hidden invalidating the content rect was postponed until after the action mode was unhidden. This patch simplifies that code by invalidating the content rect even when the action mode is hidden. Review-Url: https://codereview.chromium.org/2772223003 Cr-Commit-Position: refs/heads/master@{#459974} Committed: https://chromium.googlesource.com/chromium/src/+/664cdebf1b3d33209cc38e4366bf... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/664cdebf1b3d33209cc38e4366bf... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
