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

Issue 795493003: Don't create multiple WebRange objects for getting selection text. (Closed)

Created:
6 years ago by r.kasibhatla
Modified:
6 years ago
CC:
chromium-reviews, creis+watch_chromium.org, darin-cc_chromium.org, jam, mkwst+moarreviews-renderer_chromium.org, mlamouri+watch-content_chromium.org, nasko+codewatch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Don't create multiple WebRange objects for getting selection text. Editable HTML content is very laggy when it is relatively large. Lag is caused by creation of WebRange object with given location and offset. Bigger the editable content and farther the location, the more the lag. Currently we create WebRange object twice - once for checking presence of text (plain) and again for fetching actual plain text. It causes the lag to double the actual value. This patch removes unneccessary double creation of WebRange. Sample page and traces showing the lag and improvement with this patch are attached with the bug. BUG=396051 R=avi,jochen,yosin TBR=jochen TESTS=None Committed: https://crrev.com/c38791ac14cf1a9b3dd991d9a8cff558e3ce4d00 Cr-Commit-Position: refs/heads/master@{#308261}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -2 lines) Patch
M content/renderer/render_frame_impl.cc View 1 chunk +1 line, -2 lines 0 comments Download

Messages

Total messages: 16 (5 generated)
r.kasibhatla
PTAL
6 years ago (2014-12-10 11:56:41 UTC) #2
r.kasibhatla
PTAL. Thanks!
6 years ago (2014-12-10 18:14:38 UTC) #4
Avi (use Gerrit)
On 2014/12/10 18:14:38, r.kasibhatla wrote: > PTAL. Thanks! This looks reasonable, but I don't know ...
6 years ago (2014-12-10 18:49:50 UTC) #5
r.kasibhatla
+ jochen as blink reviewer
6 years ago (2014-12-11 03:54:21 UTC) #7
r.kasibhatla
+ yosin for blink review.
6 years ago (2014-12-11 13:06:59 UTC) #9
r.kasibhatla
On 2014/12/11 13:06:59, r.kasibhatla wrote: > + yosin for blink review. Gentle ping!!! Jochen/Yosin, Can ...
6 years ago (2014-12-12 04:25:43 UTC) #10
jochen (gone - plz use gerrit)
lgtm
6 years ago (2014-12-12 14:22:26 UTC) #11
Avi (use Gerrit)
On 2014/12/12 14:22:26, jochen (slow) wrote: > lgtm Jochen is a reviewer, but I'll stamp ...
6 years ago (2014-12-12 15:13:08 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/795493003/1
6 years ago (2014-12-13 05:53:34 UTC) #14
commit-bot: I haz the power
Committed patchset #1 (id:1)
6 years ago (2014-12-13 08:15:13 UTC) #15
commit-bot: I haz the power
6 years ago (2014-12-13 08:17:18 UTC) #16
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/c38791ac14cf1a9b3dd991d9a8cff558e3ce4d00
Cr-Commit-Position: refs/heads/master@{#308261}

Powered by Google App Engine
This is Rietveld 408576698