Add new API for only moving the selection extent point.

Update touch selection to only modify one selection point at a time.

This is made in preparation for enabling selection auto scroll using touch
handles.

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

[christiank, 2014-10-15 11:10:25]

christiank@opera.com changed reviewers:
+ aelias@chromium.org, jdduke@chromium.org

[christiank, 2014-10-15 11:10:26]

Added new selection API for only moving the selection extent.

This is the Blink portion of this review:
https://codereview.chromium.org/657803002/

[mlamouri (slow - plz ping), 2014-10-15 12:13:51]

mlamouri@chromium.org changed reviewers:
+ mlamouri@chromium.org

[mlamouri (slow - plz ping), 2014-10-15 12:13:51]

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

https://codereview.chromium.org/653383002/diff/1/public/web/WebFrame.h#newcod...
public/web/WebFrame.h:458: virtual void moveSelectionExtent(const WebPoint&) =
0;
Can't you define that in public/web/WebLocalFrame.h instead of WebFrame.h? Given
that you Chromium CL is using a WebLocalFrame and it's not defined in
WebRemoteFrame, it sounds like you could by pass entirely
WebFrame/WebRemoteFrame here.

[jdduke (slow), 2014-10-15 17:49:47]

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

https://codereview.chromium.org/653383002/diff/1/public/web/WebFrame.h#newcod...
public/web/WebFrame.h:458: virtual void moveSelectionExtent(const WebPoint&) =
0;
On 2014/10/15 12:13:51, Mounir Lamouri wrote:
> Can't you define that in public/web/WebLocalFrame.h instead of WebFrame.h?
Given
> that you Chromium CL is using a WebLocalFrame and it's not defined in
> WebRemoteFrame, it sounds like you could by pass entirely
> WebFrame/WebRemoteFrame here.

I think it would be a little odd to restrict this method to WebLocalFrame, when
all of the related methods are declared in WebFrame.

Looking at the naming, I wonder if |moveRangeSelectionExtent| fits better with
the existing scheme, maybe not?

[dcheng, 2014-10-15 19:36:48]

On 2014/10/15 at 17:49:47, jdduke wrote:
> https://codereview.chromium.org/653383002/diff/1/public/web/WebFrame.h
> File public/web/WebFrame.h (right):
> 
>
https://codereview.chromium.org/653383002/diff/1/public/web/WebFrame.h#newcod...
> public/web/WebFrame.h:458: virtual void moveSelectionExtent(const WebPoint&) =
0;
> On 2014/10/15 12:13:51, Mounir Lamouri wrote:
> > Can't you define that in public/web/WebLocalFrame.h instead of WebFrame.h?
Given
> > that you Chromium CL is using a WebLocalFrame and it's not defined in
> > WebRemoteFrame, it sounds like you could by pass entirely
> > WebFrame/WebRemoteFrame here.
> 
> I think it would be a little odd to restrict this method to WebLocalFrame,
when all of the related methods are declared in WebFrame.
> 
> Looking at the naming, I wonder if |moveRangeSelectionExtent| fits better with
the existing scheme, maybe not?

Please restrict this method to WebLocalFrame. Remote frames cannot have
selections; the fact that the WebFrame currently hosts most of these is because
the OOPI project is still in progress and has not yet had a chance to migrate
them. New APIs should be defined in the correct place to start, since they have
no legacy baggage.

[jdduke (slow), 2014-10-15 19:41:41]

On 2014/10/15 19:36:48, dcheng wrote:
> Please restrict this method to WebLocalFrame. Remote frames cannot have
> selections; the fact that the WebFrame currently hosts most of these is
because
> the OOPI project is still in progress and has not yet had a chance to migrate
> them. New APIs should be defined in the correct place to start, since they
have
> no legacy baggage.

Fair enough, though I think you might want to sprinkle some warnings in such
areas if you expect developers unfamiliar with OOPI details to know how best to
augment existing APIs =/.

[christiank, 2014-10-17 14:30:25]

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

https://codereview.chromium.org/653383002/diff/1/public/web/WebFrame.h#newcod...
public/web/WebFrame.h:458: virtual void moveSelectionExtent(const WebPoint&) =
0;
On 2014/10/15 17:49:47, jdduke wrote:
> On 2014/10/15 12:13:51, Mounir Lamouri wrote:
> > Can't you define that in public/web/WebLocalFrame.h instead of WebFrame.h?
> Given
> > that you Chromium CL is using a WebLocalFrame and it's not defined in
> > WebRemoteFrame, it sounds like you could by pass entirely
> > WebFrame/WebRemoteFrame here.
> 
> I think it would be a little odd to restrict this method to WebLocalFrame,
when
> all of the related methods are declared in WebFrame.
> 
> Looking at the naming, I wonder if |moveRangeSelectionExtent| fits better with
> the existing scheme, maybe not?

Sure, I'll move it to WebLocalFrame and rename it to moveRangeSelectionExtent
while at it.

[mlamouri (slow - plz ping), 2014-10-18 08:43:52]

lgtm with nits

https://codereview.chromium.org/653383002/diff/20001/Source/web/WebLocalFrame...
File Source/web/WebLocalFrameImpl.cpp (right):

https://codereview.chromium.org/653383002/diff/20001/Source/web/WebLocalFrame...
Source/web/WebLocalFrameImpl.cpp:1173: // Prevent the selection from collapsing.
nit: could that be expressed in the WebLocalFrame.h declaration comment?

[christiank, 2014-10-22 06:11:33]

The CQ bit was checked by christiank@opera.com

[commit-bot: I haz the power, 2014-10-22 06:12:31]

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

[commit-bot: I haz the power, 2014-10-22 06:52:07]

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

[commit-bot: I haz the power, 2014-10-22 06:52:08]

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

[christiank, 2014-10-22 07:37:41]

christiank@opera.com changed reviewers:
+ tkent@chromium.org

[christiank, 2014-10-22 07:38:49]

Ooops, tried to commit a bit prematurely.

tkent would you mind taking a look?

[tkent, 2014-10-22 07:45:08]

lgtm

[christiank, 2014-10-22 07:56:58]

The CQ bit was checked by christiank@opera.com

[commit-bot: I haz the power, 2014-10-22 07:57:44]

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

[commit-bot: I haz the power, 2014-10-22 10:56:30]

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

[commit-bot: I haz the power, 2014-10-22 10:56:30]

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

[christiank, 2014-10-22 12:16:35]

The CQ bit was checked by christiank@opera.com

[commit-bot: I haz the power, 2014-10-22 12:17:22]

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

[commit-bot: I haz the power, 2014-10-22 13:04:25]

Committed patchset #3 (id:40001) as 184179