Expose public interface for composited selection bounds

Expose public interface for composited selection bounds

Currently, the selection bounds are pulled from the WebView at the start of each
frame. Such an approach works properly only if the queried bounds are always
in-sync with the visible content, which will not always hold with accelerated
compositing.

This patch exposes a public interface for enabling and routing composited
selection bounds. Metadata for the composited bounds provides the compositor
with all necessary information to reposition and determine visibility of the
selection endpoints. The data is exposed via the WebLayerTreeView, and will
ultimately be provided by the RenderLayerCompositor.

Note that implementation of the composited bounds computation will be performed
in a separate follow-up patch (see https://codereview.chromium.org/302993003/).

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

[jdduke (slow), 2014-06-25 15:44:10]

aelias@, abarth@, PTAL. This defines the interface for the Blink embedder, as
well as the internal interface that will be used by the RenderLayerCompositor to
push selection updates.

As noted in the original patch (https://codereview.chromium.org/302993003/), I
avoided attaching the selection data to the layer as the selection endpoints are
sparse and global. It seems wrong to clutter the layer with this data when the
selection endpoints are really associated with a given tree and just happen to
be fixed to a given layer at any given moment.  If you have any better ideas on
how to route this information, I'd love to hear it. Thanks.

[abarth-chromium, 2014-06-25 19:54:59]

You've included a bunch of code in this CL that's not related to the Blink API. 
This is now the third time I've asked you to limit the scope of the first CL to
just the public API.

https://codereview.chromium.org/352173002/diff/1/public/platform/WebLayerTree...
File public/platform/WebLayerTreeView.h (right):

https://codereview.chromium.org/352173002/diff/1/public/platform/WebLayerTree...
public/platform/WebLayerTreeView.h:45: struct WebSelectionBound;
WebSelectionBound.h is defined in public/web.  You've introduce a dependency
from public/platform to public/web, which isn't permitted.  If you'd have
included the header rather than used a forward declaration, you would have
gotten a DEPS error.

If you want to use this type for WebLayerTreeView, you'll need to move
WebSelectionBound.h into public/platform

https://codereview.chromium.org/352173002/diff/1/public/web/WebRuntimeFeatures.h
File public/web/WebRuntimeFeatures.h (right):

https://codereview.chromium.org/352173002/diff/1/public/web/WebRuntimeFeature...
public/web/WebRuntimeFeatures.h:49: BLINK_EXPORT static void
enableCompositedSelectionUpdates(bool);
Why do we need a public API for this setting?

[jdduke (slow), 2014-06-25 20:16:09]

On 2014/06/25 19:54:59, abarth wrote:
> You've included a bunch of code in this CL that's not related to the Blink
API. 
> This is now the third time I've asked you to limit the scope of the first CL
to
> just the public API.

Apologies, I took your API reference to mean both blink<->external and
blink<->WebCore.

[jdduke (slow), 2014-06-25 21:50:54]

Alright, sorry for the noise, the patch now reflects the public Blink API only.

https://codereview.chromium.org/352173002/diff/1/public/platform/WebLayerTree...
File public/platform/WebLayerTreeView.h (right):

https://codereview.chromium.org/352173002/diff/1/public/platform/WebLayerTree...
public/platform/WebLayerTreeView.h:45: struct WebSelectionBound;
On 2014/06/25 19:54:59, abarth wrote:
> WebSelectionBound.h is defined in public/web.  You've introduce a dependency
> from public/platform to public/web, which isn't permitted.  If you'd have
> included the header rather than used a forward declaration, you would have
> gotten a DEPS error.
> 
> If you want to use this type for WebLayerTreeView, you'll need to move
> WebSelectionBound.h into public/platform

Done.

https://codereview.chromium.org/352173002/diff/1/public/web/WebRuntimeFeatures.h
File public/web/WebRuntimeFeatures.h (right):

https://codereview.chromium.org/352173002/diff/1/public/web/WebRuntimeFeature...
public/web/WebRuntimeFeatures.h:49: BLINK_EXPORT static void
enableCompositedSelectionUpdates(bool);
On 2014/06/25 19:54:59, abarth wrote:
> Why do we need a public API for this setting?

The composited updates will initially only be enabled on Android.  I was
following the pattern of |enableOverlayFullscreenVideo|, but perhaps there's a
different/better way? In any case, it probably doesn't make sense to expose this
until the feature is actually implemented, so I'll remove it for now.

[abarth-chromium, 2014-06-25 22:27:03]

LGTM

A couple questions below that can likely be addressed by adding some comments.

https://codereview.chromium.org/352173002/diff/20001/public/platform/WebLayer...
File public/platform/WebLayerTreeView.h (right):

https://codereview.chromium.org/352173002/diff/20001/public/platform/WebLayer...
public/platform/WebLayerTreeView.h:130: virtual void registerSelection(const
WebSelectionBound& anchor, const WebSelectionBound& focus) { }
Are there any invariants of these arguments that we should document?  For
example, is it legal to have one with type Caret and the other with type
SelectionLeft?  Can they both be SelectionRight?

https://codereview.chromium.org/352173002/diff/20001/public/platform/WebSelec...
File public/platform/WebSelectionBound.h (right):

https://codereview.chromium.org/352173002/diff/20001/public/platform/WebSelec...
public/platform/WebSelectionBound.h:35: WebRect layerRect;
Is this the rect of the handle itself?  It sounds like it might be a
one-dimensional rect that represents the edge of the selection...

Maybe this class would be better named WebSelectionEdge?

[abarth-chromium, 2014-06-25 22:27:18]

Thanks for breaking this down into small steps.

[jdduke (slow), 2014-06-25 22:58:10]

On 2014/06/25 22:27:18, abarth wrote:
> Thanks for breaking this down into small steps.

No problem, thanks for review!

https://codereview.chromium.org/352173002/diff/20001/public/platform/WebLayer...
File public/platform/WebLayerTreeView.h (right):

https://codereview.chromium.org/352173002/diff/20001/public/platform/WebLayer...
public/platform/WebLayerTreeView.h:130: virtual void registerSelection(const
WebSelectionBound& anchor, const WebSelectionBound& focus) { }
On 2014/06/25 22:27:03, abarth wrote:
> Are there any invariants of these arguments that we should document?  For
> example, is it legal to have one with type Caret and the other with type
> SelectionLeft?  Can they both be SelectionRight? 

Yes, good call, I'll flesh out the comment a bit more.  In particular, anchor
and focus will be identical if one is of type Caret (this mirrors the current
|WebView::selectionBounds()| query), and you can indeed have both be of
SelectionRight type with mixed RTL/LTR selections.

https://codereview.chromium.org/352173002/diff/20001/public/platform/WebSelec...
File public/platform/WebSelectionBound.h (right):

https://codereview.chromium.org/352173002/diff/20001/public/platform/WebSelec...
public/platform/WebSelectionBound.h:35: WebRect layerRect;
On 2014/06/25 22:27:03, abarth wrote:
> Is this the rect of the handle itself?  It sounds like it might be a
> one-dimensional rect that represents the edge of the selection...
> 
> Maybe this class would be better named WebSelectionEdge?

Hmm, I'm not actually sure if they're always one-dimensional (in the existing
unified patch), although for all intents and purposes I believe we've always
ignored the endpoint widths with the existing selection bound pipeline.  I'll
make sure that's enforced, as I can't see what use the width parameter would
have.

I somewhat prefer the name bound for the overall type, if only because it fits
with the existing concept of "selection bounds", but I'll rename the rect
variable to indicate the edge geometry.  Once all platforms are on board and we
can remove the current selection bounds plumbing, it may make sense to do some
renaming.

[jdduke (slow), 2014-06-25 22:58:15]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-06-25 22:58:36]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jdduke@chromium.org/352173002/40001

[commit-bot: I haz the power, 2014-06-26 00:00:31]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/12829)

[commit-bot: I haz the power, 2014-06-26 00:35:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-26 00:35:16]

Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/12838)

[jdduke (slow), 2014-06-26 00:36:38]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-06-26 00:37:05]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jdduke@chromium.org/352173002/40001

[commit-bot: I haz the power, 2014-06-26 03:29:53]

Change committed as 176958