Enable gesture events handling on Android.

Enable gesture events handling on Android.

This add gesture related functionality to the java side to ContentViewCore and also
adds some missing calls on the routing path of the gesture events to webkit. The WebKit side
for related parts of the WebCompositorInputHandler has already been upstreamed.

BUG=136680
TEST=

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=148861

[Yusuf, 2012-07-19 00:47:53]

This completes the path for gestures from android framework to renderer, sending
gesture events to webkit.

+jam for content/
+aelias for clank side of gesture handling

[aelias_OOO_until_Jul13, 2012-07-19 03:43:51]

http://codereview.chromium.org/10790066/diff/1/content/browser/android/conten...
File content/browser/android/content_startup_flags.cc (right):

http://codereview.chromium.org/10790066/diff/1/content/browser/android/conten...
content/browser/android/content_startup_flags.cc:72: #if
!defined(ANDROID_UPSTREAM_BRINGUP)
Let's not leave this around certain flags, we'll forget about them.  The only
one that should be commented out is kEnablePerTilePainting (currently also
disabled downstream).

http://codereview.chromium.org/10790066/diff/1/content/browser/android/conten...
File content/browser/android/content_view_core_impl.cc (right):

http://codereview.chromium.org/10790066/diff/1/content/browser/android/conten...
content/browser/android/content_view_core_impl.cc:180: // This throws out scroll
and pinch events if the initial touch is rejected
This whole comment is no longer in its place here, please delete it.

http://codereview.chromium.org/10790066/diff/1/content/public/android/java/sr...
File
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
(left):

http://codereview.chromium.org/10790066/diff/1/content/public/android/java/sr...
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:224:
mContainerView.setWillNotDraw(false);
Is deleting this line intentional?

[Yusuf, 2012-07-19 05:01:52]

http://codereview.chromium.org/10790066/diff/1/content/browser/android/conten...
File content/browser/android/content_startup_flags.cc (right):

http://codereview.chromium.org/10790066/diff/1/content/browser/android/conten...
content/browser/android/content_startup_flags.cc:72: #if
!defined(ANDROID_UPSTREAM_BRINGUP)
On 2012/07/19 03:43:51, aelias wrote:
> Let's not leave this around certain flags, we'll forget about them.  The only
> one that should be commented out is kEnablePerTilePainting (currently also
> disabled downstream).

Done.

http://codereview.chromium.org/10790066/diff/1/content/browser/android/conten...
File content/browser/android/content_view_core_impl.cc (right):

http://codereview.chromium.org/10790066/diff/1/content/browser/android/conten...
content/browser/android/content_view_core_impl.cc:180: // This throws out scroll
and pinch events if the initial touch is rejected
On 2012/07/19 03:43:51, aelias wrote:
> This whole comment is no longer in its place here, please delete it.

Done.

http://codereview.chromium.org/10790066/diff/1/content/public/android/java/sr...
File
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
(left):

http://codereview.chromium.org/10790066/diff/1/content/public/android/java/sr...
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:224:
mContainerView.setWillNotDraw(false);
Yes, before removing this it was not capturing TouchEvents and the parent layout
on the content_shell.apk was consuming all of them.

On 2012/07/19 03:43:51, aelias wrote:
> Is deleting this line intentional?

[jochen (gone - plz use gerrit), 2012-07-19 14:50:06]

drive-by content/ comments (except for the .java files)

https://chromiumcodereview.appspot.com/10790066/diff/10001/content/browser/an...
File content/browser/android/content_view_core_impl.cc (right):

https://chromiumcodereview.appspot.com/10790066/diff/10001/content/browser/an...
content/browser/android/content_view_core_impl.cc:22: #include
"third_party/WebKit/Source/WebKit/chromium/public/WebInputEvent.h"
alphabetical ordering

https://chromiumcodereview.appspot.com/10790066/diff/10001/content/browser/an...
content/browser/android/content_view_core_impl.cc:108:
GetRenderWidgetHostViewAndroid() {
please keep the class and method name together on one line

https://chromiumcodereview.appspot.com/10790066/diff/10001/content/browser/an...
content/browser/android/content_view_core_impl.cc:175: jboolean
ContentViewCoreImpl::TouchEvent(JNIEnv* env, jobject obj,
one parameter per line

https://chromiumcodereview.appspot.com/10790066/diff/10001/content/browser/an...
content/browser/android/content_view_core_impl.cc:195: 0);
the 0); fits on the previous line

https://chromiumcodereview.appspot.com/10790066/diff/10001/content/browser/an...
content/browser/android/content_view_core_impl.cc:202:
SendGestureEvent(WebInputEvent::GestureScrollBegin, time_ms, x, y, 0, 0,
I would format this as 
SendGestureEvent(
    WebInputEvent::GestureScrollBegin, ....);

https://chromiumcodereview.appspot.com/10790066/diff/10001/content/browser/an...
content/browser/android/content_view_core_impl.cc:422: handled);
indenting off. also only one parameter per line

https://chromiumcodereview.appspot.com/10790066/diff/10001/content/browser/an...
File content/browser/android/content_view_core_impl.h (right):

https://chromiumcodereview.appspot.com/10790066/diff/10001/content/browser/an...
content/browser/android/content_view_core_impl.h:21: #include
"third_party/WebKit/Source/WebKit/chromium/public/WebInputEvent.h"
nit. alphabetical ordering

https://chromiumcodereview.appspot.com/10790066/diff/10001/content/browser/an...
content/browser/android/content_view_core_impl.h:36: virtual void
Destroy(JNIEnv* env, jobject obj);
nit. missing override

https://chromiumcodereview.appspot.com/10790066/diff/10001/content/browser/an...
content/browser/android/content_view_core_impl.h:62: jboolean TouchEvent(JNIEnv*
env, jobject obj, jlong time_ms, jint type,
c++ style guide says one parameter per line if it doesn't fit one line

https://chromiumcodereview.appspot.com/10790066/diff/10001/content/browser/an...
content/browser/android/content_view_core_impl.h:115: void
ConfirmTouchEvent(bool handled) OVERRIDE;
why are these marked OVERRIDE?

https://chromiumcodereview.appspot.com/10790066/diff/10001/content/browser/an...
content/browser/android/content_view_core_impl.h:189: };  // namespace content
no ;

https://chromiumcodereview.appspot.com/10790066/diff/10001/content/browser/re...
File content/browser/renderer_host/render_widget_host_view_android.cc (right):

https://chromiumcodereview.appspot.com/10790066/diff/10001/content/browser/re...
content/browser/renderer_host/render_widget_host_view_android.cc:332: // Methods
called from the host to the render
I'd also remove this comment

https://chromiumcodereview.appspot.com/10790066/diff/10001/content/browser/re...
File content/browser/renderer_host/render_widget_host_view_android.h (right):

https://chromiumcodereview.appspot.com/10790066/diff/10001/content/browser/re...
content/browser/renderer_host/render_widget_host_view_android.h:109: //
Non-virtual methods
please remove this comment

https://chromiumcodereview.appspot.com/10790066/diff/10001/content/public/com...
File content/public/common/content_switches.h (right):

https://chromiumcodereview.appspot.com/10790066/diff/10001/content/public/com...
content/public/common/content_switches.h:206: extern const char
kGraphicsModeValueBasic[];
are those values needed outside of content?

[yusufo, 2012-07-19 19:01:08]

https://chromiumcodereview.appspot.com/10790066/diff/10001/content/public/com...
File content/public/common/content_switches.h (right):

https://chromiumcodereview.appspot.com/10790066/diff/10001/content/public/com...
content/public/common/content_switches.h:206: extern const char
kGraphicsModeValueBasic[];
No, AFAIK they are used in content only.

On 2012/07/19 14:50:06, jochen wrote:
> are those values needed outside of content?

[yusufo, 2012-07-19 20:36:03]

After discussing this with @bulach, I am removing 2 java files which contain
most of the java side implementation of gestures from this Issue. These files
will be upstreamed with their tests separately and then I will submit this patch
last as it depends on them. Feel free to send reviews for the current state or
hold off until the java files and tests have landed.

[Yusuf, 2012-07-26 21:46:04]

OK, so the CL that this depends on is about to go in, so please do continue with
the review.

[Yusuf, 2012-07-27 01:03:19]

To clarify further, LongPressDetector.java and its test file has landed
separately, but I added ContentViewGestureHandler.java and its test back to this
one, as I thought without that the changes in ContentViewCore look a little bit
confusing.

jam@ or jochen@ Please let me know if I need to add anyone else to review.

[Ted C, 2012-07-27 01:31:53]

Java looks good aside from the nits.

I'll let Alex look through the native portions.

http://codereview.chromium.org/10790066/diff/14001/content/browser/android/co...
File content/browser/android/content_view_core_impl.cc (right):

http://codereview.chromium.org/10790066/diff/14001/content/browser/android/co...
content/browser/android/content_view_core_impl.cc:107:
RenderWidgetHostViewAndroid*
ContentViewCoreImpl::GetRenderWidgetHostViewAndroid() {
>100 chars

http://codereview.chromium.org/10790066/diff/14001/content/browser/android/co...
File content/browser/android/content_view_core_impl.h (right):

http://codereview.chromium.org/10790066/diff/14001/content/browser/android/co...
content/browser/android/content_view_core_impl.h:67: 
why the blank line here and above?

http://codereview.chromium.org/10790066/diff/14001/content/public/android/jav...
File
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
(right):

http://codereview.chromium.org/10790066/diff/14001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:514:
if (mNativeContentViewCore != 0) {
I would exit early here...

if (mNativeContentViewCore == 0) return false;

http://codereview.chromium.org/10790066/diff/14001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:600:
//TODO(yusufo):Stubbed out for now. Upstream when computeHorizontalScrollOffset
is
space after the // and :
same for line other todos and the following line.

[aelias_OOO_until_Jul13, 2012-07-27 01:35:03]

http://codereview.chromium.org/10790066/diff/14001/content/browser/android/co...
File content/browser/android/content_startup_flags.cc (left):

http://codereview.chromium.org/10790066/diff/14001/content/browser/android/co...
content/browser/android/content_startup_flags.cc:76:
SetCommandLineSwitch(switches::kEnablePerTilePainting);
Please remove the #if and make this exactly match the downstream version to
avoid merge conflicts.

http://codereview.chromium.org/10790066/diff/14001/content/browser/android/co...
File content/browser/android/content_view_core_impl.cc (right):

http://codereview.chromium.org/10790066/diff/14001/content/browser/android/co...
content/browser/android/content_view_core_impl.cc:111: } else if
(web_contents_->ShowingInterstitialPage()) {
Please delete this whole else block.  This is the wrong way to do it and we have
another plan for handling interstitial pages (sievers@ is working on it).

[jam, 2012-07-27 21:02:13]

lgtm for content/public/common. i'll defer to android folks for
render_widget_host_view_android*

http://codereview.chromium.org/10790066/diff/14004/content/browser/renderer_h...
File content/browser/renderer_host/render_widget_host_view_android.h (right):

http://codereview.chromium.org/10790066/diff/14004/content/browser/renderer_h...
content/browser/renderer_host/render_widget_host_view_android.h:108: virtual
void DidSetNeedTouchEvents(bool need_touch_events) OVERRIDE;
nit: put all overridden functions from the same interface together

[aelias_OOO_until_Jul13, 2012-07-27 21:34:16]

LGTM for render_widget_host_view_android*

[commit-bot: I haz the power, 2012-07-27 21:36:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yusufo@chromium.org/10790066/24001

[commit-bot: I haz the power, 2012-07-28 01:20:31]

Change committed as 148861