Bug 131559 - Aggregate multiple "respondToChangedSelection" calls to one scan for telephone numbers
Summary: Aggregate multiple "respondToChangedSelection" calls to one scan for telephon...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh All
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-11 13:16 PDT by Brady Eidson
Modified: 2014-04-11 19:27 PDT (History)
3 users (show)

See Also:


Attachments
Patch v1 (4.93 KB, patch)
2014-04-11 13:18 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2014-04-11 13:16:29 PDT
Aggregate multiple "respondToChangedSelection" calls to one scan for telephone numbers

Turns out Editor::respondToChangedSelection is called a lot more often than when the user visible selection changes as the result of a user event.

In fact, it can be called many (hundreds!) of times during one iteration of the run loop.

We should aggregate all of those calls into a single telephone number scan.
Comment 1 Brady Eidson 2014-04-11 13:18:48 PDT
Created attachment 229157 [details]
Patch v1
Comment 2 WebKit Commit Bot 2014-04-11 14:09:43 PDT
Comment on attachment 229157 [details]
Patch v1

Clearing flags on attachment: 229157

Committed r167148: <http://trac.webkit.org/changeset/167148>
Comment 3 WebKit Commit Bot 2014-04-11 14:09:46 PDT
All reviewed patches have been landed.  Closing bug.
Comment 4 Ryosuke Niwa 2014-04-11 16:20:23 PDT
Why don't we just do this in editorUIUpdateTimerFired instead of adding a separate timer?
Comment 5 Brady Eidson 2014-04-11 19:27:29 PDT
(In reply to comment #4)
> Why don't we just do this in editorUIUpdateTimerFired instead of adding a separate timer?

I saw that there was an early return before the m_editorUIUpdateTimer was started, but knew we needed to do this work every time.

Looking at it closer now, the early return was simply a:
    if (m_editorUIUpdateTimer.isActive())
        return;
check.

I agree the existing timer should be good enough here!

(I won't be near a dev environment for a few days, in case anyone else wanted to fix this before then)