Adding a bunch of trace events for text selection.

Adding a bunch of trace events for text selection.

BUG=NONE

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=196879

[mfomitchev, 2015-06-09 16:20:21]

mfomitchev@chromium.org changed reviewers:
+ jdduke@chromium.org, leviw@chromium.org, yosin@google.com

[mfomitchev, 2015-06-09 16:20:22]

Please let me know if this looks reasonable. Perhaps there's more selection
methods we should be tracing?

[jdduke (slow), 2015-06-09 16:45:15]

On 2015/06/09 16:20:22, mfomitchev wrote:
> Please let me know if this looks reasonable. Perhaps there's more selection
> methods we should be tracing?

Thanks Mikhail, this definitely covers the primary selection calls on Android. I
suppose there are cases where the FrameSelection can be manipulated outside of
the WebLocalFrame interface (by JS?) but I'm not sure we care as much about
that.

[mfomitchev, 2015-06-09 17:01:12]

On 2015/06/09 16:45:15, jdduke wrote:
> On 2015/06/09 16:20:22, mfomitchev wrote:
> > Please let me know if this looks reasonable. Perhaps there's more selection
> > methods we should be tracing?
> 
> Thanks Mikhail, this definitely covers the primary selection calls on Android.
I
> suppose there are cases where the FrameSelection can be manipulated outside of
> the WebLocalFrame interface (by JS?) but I'm not sure we care as much about
> that.

The main reason I put these into WebLocalFrameImpl is that the hit testing is
done there for a bunch of these methods, and hit-testing typically dominates the
cost. I am not sure how JS calls work, but if they go directly through
FrameSelection, then the costs would be very different, because of the lack of
hit testing, and yeah, I don't think we'd worry about those nearly as much.

[leviw_travelin_and_unemployed, 2015-06-09 18:19:58]

On 2015/06/09 at 17:01:12, mfomitchev wrote:
> On 2015/06/09 16:45:15, jdduke wrote:
> > On 2015/06/09 16:20:22, mfomitchev wrote:
> > > Please let me know if this looks reasonable. Perhaps there's more
selection
> > > methods we should be tracing?
> > 
> > Thanks Mikhail, this definitely covers the primary selection calls on
Android. I
> > suppose there are cases where the FrameSelection can be manipulated outside
of
> > the WebLocalFrame interface (by JS?) but I'm not sure we care as much about
> > that.
> 
> The main reason I put these into WebLocalFrameImpl is that the hit testing is
done there for a bunch of these methods, and hit-testing typically dominates the
cost. I am not sure how JS calls work, but if they go directly through
FrameSelection, then the costs would be very different, because of the lack of
hit testing, and yeah, I don't think we'd worry about those nearly as much.

JS Selections operate on the DOMSelection object. DOMSelection creates visible
positions and interacts with the FrameSelection object directly.

[jdduke (slow), 2015-06-09 19:04:33]

On 2015/06/09 17:01:12, mfomitchev wrote:
> On 2015/06/09 16:45:15, jdduke wrote:
> > On 2015/06/09 16:20:22, mfomitchev wrote:
> > > Please let me know if this looks reasonable. Perhaps there's more
selection
> > > methods we should be tracing?
> > 
> > Thanks Mikhail, this definitely covers the primary selection calls on
Android.
> I
> > suppose there are cases where the FrameSelection can be manipulated outside
of
> > the WebLocalFrame interface (by JS?) but I'm not sure we care as much about
> > that.
> 
> The main reason I put these into WebLocalFrameImpl is that the hit testing is
> done there for a bunch of these methods, and hit-testing typically dominates
the
> cost. I am not sure how JS calls work, but if they go directly through
> FrameSelection, then the costs would be very different, because of the lack of
> hit testing, and yeah, I don't think we'd worry about those nearly as much.

Gotcha, sounds good to me, we can always revisit if we find JS selections
bottlenecking.

[mfomitchev, 2015-06-09 19:11:51]

On 2015/06/09 18:19:58, leviw wrote:
> On 2015/06/09 at 17:01:12, mfomitchev wrote:
> > On 2015/06/09 16:45:15, jdduke wrote:
> > > On 2015/06/09 16:20:22, mfomitchev wrote:
> > > > Please let me know if this looks reasonable. Perhaps there's more
> selection
> > > > methods we should be tracing?
> > > 
> > > Thanks Mikhail, this definitely covers the primary selection calls on
> Android. I
> > > suppose there are cases where the FrameSelection can be manipulated
outside
> of
> > > the WebLocalFrame interface (by JS?) but I'm not sure we care as much
about
> > > that.
> > 
> > The main reason I put these into WebLocalFrameImpl is that the hit testing
is
> done there for a bunch of these methods, and hit-testing typically dominates
the
> cost. I am not sure how JS calls work, but if they go directly through
> FrameSelection, then the costs would be very different, because of the lack of
> hit testing, and yeah, I don't think we'd worry about those nearly as much.
> 
> JS Selections operate on the DOMSelection object. DOMSelection creates visible
> positions and interacts with the FrameSelection object directly.

Looks like JS methods don't do any hit testing and they don't need to because
they get character "offsets", not pixel locations as their arguments. So yeah,
their performance would be entirely different. I guess we could also add traces
to the FrameSelection methods (which used by both JS and Chrome), but if we
don't really need perf data on JS methods this seems like an overkill.

leviw, yosin: can you please lgtm if this looks good to you?

[leviw_travelin_and_unemployed, 2015-06-09 20:57:04]

On 2015/06/09 at 19:11:51, mfomitchev wrote:
> On 2015/06/09 18:19:58, leviw wrote:
> > On 2015/06/09 at 17:01:12, mfomitchev wrote:
> > > On 2015/06/09 16:45:15, jdduke wrote:
> > > > On 2015/06/09 16:20:22, mfomitchev wrote:
> > > > > Please let me know if this looks reasonable. Perhaps there's more
> > selection
> > > > > methods we should be tracing?
> > > > 
> > > > Thanks Mikhail, this definitely covers the primary selection calls on
> > Android. I
> > > > suppose there are cases where the FrameSelection can be manipulated
outside
> > of
> > > > the WebLocalFrame interface (by JS?) but I'm not sure we care as much
about
> > > > that.
> > > 
> > > The main reason I put these into WebLocalFrameImpl is that the hit testing
is
> > done there for a bunch of these methods, and hit-testing typically dominates
the
> > cost. I am not sure how JS calls work, but if they go directly through
> > FrameSelection, then the costs would be very different, because of the lack
of
> > hit testing, and yeah, I don't think we'd worry about those nearly as much.
> > 
> > JS Selections operate on the DOMSelection object. DOMSelection creates
visible
> > positions and interacts with the FrameSelection object directly.
> 
> Looks like JS methods don't do any hit testing and they don't need to because
they get character "offsets", not pixel locations as their arguments. So yeah,
their performance would be entirely different. I guess we could also add traces
to the FrameSelection methods (which used by both JS and Chrome), but if we
don't really need perf data on JS methods this seems like an overkill.

Document::caretRangeFromPoint does hit testing, but doesn't actually change the
selection, so it shouldn't be effected.

I'm not a Source/web owner, but LGTM.

> 
> leviw, yosin: can you please lgtm if this looks good to you?

[yosin_UTC9, 2015-06-10 01:48:32]

yosin@chromium.org changed reviewers:
+ yosin@chromium.org
- yosin@google.com

[yosin_UTC9, 2015-06-10 01:49:08]

yosin@chromium.org changed reviewers:
+ tkent@chromium.org

[yosin_UTC9, 2015-06-10 01:49:08]

lgtm for FrameSelection.cpp
+tkent@ for web/

[tkent, 2015-06-10 01:55:25]

lgtm

[mfomitchev, 2015-06-10 15:52:40]

The CQ bit was checked by mfomitchev@chromium.org

[commit-bot: I haz the power, 2015-06-10 15:52:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1171733002/1

[commit-bot: I haz the power, 2015-06-10 17:10:09]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=196879