wip: Aura-specific implementation of unified touch selection: touch_selection

wip: Aura-specific implementation of unified touch selection: touch_selection

The initial revision of this CL is https://codereview.chromium.org/996373002/.

BUG=399721

[mfomitchev, 2015-03-30 03:28:44]

mfomitchev@chromium.org changed reviewers:
+ jdduke@chromium.org, mohsen@chromium.org, sadrul@chromium.org

[mfomitchev, 2015-03-30 03:28:45]

This is the first stab at the refactoring - just for touch_selection Aura and
the shared Aura/Android classes. It's somewhat different from what I have
originally described in crrev.com/996373002/, so I'd appreciate if you could
take a quick look to confirm you are still okay with this design (jdduke -
especially you :))

The main difference is that TouchSelectionControllerImpl now has supports quick
menu, delegating rendering to the client. Reasons I did it:
- We should probably share as much of the quick menu code as we can.
- Not putting this in TSCImpl makes things on the Aura side awkward.
Another difference is that TouchSelectionControllerAura now inherits from
TouchSelectionControllerImpl and overrides four methods - that's needed to
enable/disable EnvPreTargetHandler.

==================================
More design details/rambling:

The client has two new methods:
- ShowQuickMenu(gfx::Rect anchor_rect)
- HideQuickMenu()
Jared, this is different from what you have in crrev.com/1035013002. It passes
"anchor rect" to the client instead of adding SELECTION_MOVED event type. It
also fades  the menu out when you start scrolling (since the menu is rendered on
the browser process, it won't scroll with the selection well). It fades the menu
out in a bunch of other cases as well. It's basically Aura's implementation. It
will probably need some changes before it is suitable for Android, which is okay
as long as we agree on the general approach.

[jdduke (slow), 2015-03-30 15:46:22]

https://codereview.chromium.org/1046783002/diff/20001/ui/touch_selection/touc...
File ui/touch_selection/touch_selection_controller.h (right):

https://codereview.chromium.org/1046783002/diff/20001/ui/touch_selection/touc...
ui/touch_selection/touch_selection_controller.h:90: virtual void
OnNativeViewMoved() = 0;
So all of these are Aura-specific? It feels a bit incomplete for us to implement
them in the common class but simply never call them on Android. I'd be OK with
an #if defined(USE_AURA) guard, and if/when we adopt them on Android we can
remove the guard.

https://codereview.chromium.org/1046783002/diff/20001/ui/touch_selection/touc...
File ui/touch_selection/touch_selection_controller_android.h (right):

https://codereview.chromium.org/1046783002/diff/20001/ui/touch_selection/touc...
ui/touch_selection/touch_selection_controller_android.h:30: : public
TouchSelectionController {
Aren't we inheriting from TouchSelectionControllerImpl?

https://codereview.chromium.org/1046783002/diff/20001/ui/touch_selection/touc...
File ui/touch_selection/touch_selection_controller_impl.cc (right):

https://codereview.chromium.org/1046783002/diff/20001/ui/touch_selection/touc...
ui/touch_selection/touch_selection_controller_impl.cc:550: void
TouchSelectionControllerImpl::ShowQuickMenu() {
All of this menu code will have to be a no-op on Android for now (i.e., we
shouldn't be firing any timers).

[jdduke (slow), 2015-03-30 15:51:32]

Sigh, I had a reply that got destroyed.

On 2015/03/30 03:28:45, mfomitchev wrote:
> The main difference is that TouchSelectionControllerImpl now has supports
quick
> menu, delegating rendering to the client. Reasons I did it:
> - We should probably share as much of the quick menu code as we can.

I don't anticipate being able to share much of the quick menu code. Almost all
of the code on Android  for that will have to live in Java, and the platform
takes care of showing/hiding for us (when it makes sense).

> - Not putting this in TSCImpl makes things on the Aura side awkward.

Hmm, any more awkward than implementing all this logic in the base class but
ignoring it on Android?

> Another difference is that TouchSelectionControllerAura now inherits from
> TouchSelectionControllerImpl and overrides four methods - that's needed to
> enable/disable EnvPreTargetHandler.
> 

That's fine, but we should probably call it TouchSelectionControllerBase.

> ==================================
> More design details/rambling:
> 
> The client has two new methods:
> - ShowQuickMenu(gfx::Rect anchor_rect)
> - HideQuickMenu()
> Jared, this is different from what you have in crrev.com/1035013002. It passes
> "anchor rect" to the client instead of adding SELECTION_MOVED event type. It
> also fades  the menu out when you start scrolling (since the menu is rendered
on
> the browser process, it won't scroll with the selection well). It fades the
menu
> out in a bunch of other cases as well. It's basically Aura's implementation.
It
> will probably need some changes before it is suitable for Android, which is
okay
> as long as we agree on the general approach.

Again, I don't think we'll be able to share much code/logic here. I'm sad that
we can't make this work in the Aura implementation class, but at this point I
don't really care as long as it's obvious and explicit that this code is unused
on Android.

[mfomitchev, 2015-03-30 17:43:01]

After talking with Jared on the IM - we'd have to adapt this  to make it
reusable for Android. Clank needs to use native Android methods to hide/show
quick menu. The code that has to do with positioning can be shared with Aura,
but the code that has to do with hiding/showing (e.g. timer code) cannot be
reused, so it probably should be in the client. I think I will just put that
code into TouchSelectionControllerAuraClientImpl for now, but we'll need to
figure out a nice way to share it with Aura's Textfield.
@sadrul - please let me know if this sounds good to you.