Relanding [Android] Adding Smart GO/NEXT feature in Chrome

Relanding [Android] Adding Smart GO/NEXT feature in Chrome

Smart Go/Next brings better user experience to the user during form submitting applications.
For navigating between form elements, user can use NEXT/PREVIOUS button from IME
without touching on individual fields. This will avoid unnecessary form submissions before
filling or visiting all fields in the form.

Additionally it will save user time and avoid redundant network requests before actually
filling/attending entire fields in the form

Design Document: https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSPNg/edit?usp=sharing

Initial patch is reviewed @ https://codereview.chromium.org/2839993002/ and
https://codereview.chromium.org/2948593002/

Scrolling performance is getting effected due to call to
NextFocusableElementInForm() from InputMethodController. hence splitting it into multiple patches.
This is the initial landing without enabling functionality.
In next patch functionality will be enabled with necessary test coverage.

BUG=410785, 648986, 733222
Review-Url: https://codereview.chromium.org/2967493002
Cr-Commit-Position: refs/heads/master@{#484020}
Committed: https://chromium.googlesource.com/chromium/src/+/933aca75233fbb5145c29c1e5a06e5f36c308cc0

[AKVT, 2017-06-29 18:11:06]

ajith.v@samsung.com changed reviewers:
+ changwan@chromium.org, dcheng@chromium.org, kochi@chromium.org,
nasko@chromium.org, tkent@chromium.org, yosin@chromium.org

[AKVT, 2017-06-29 18:11:07]

PTAL!

This patch is initially reviewed @ https://codereview.chromium.org/2948593002/.
Due to power regression, in this CL functionality is not enabled. In next CL
functionality will be enabled after fixing power regression of
https://bugs.chromium.org/p/chromium/issues/detail?id=737388

[AKVT, 2017-06-29 18:15:05]

Description was changed from

==========
Relanding [Android] Adding Smart GO/NEXT feature in Chrome

Smart Go/Next brings better user experience to the user during form submitting
applications.
For navigating between form elements, user can use NEXT/PREVIOUS button from IME
without touching on individual fields. This will avoid unnecessary form
submissions before
filling or visiting all fields in the form.

Additionally it will save user time and avoid redundant network requests before
actually
filling/attending entire fields in the form

Design Document:
https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSP...

Initial patch is reviewed @ https://codereview.chromium.org/2839993002/ and
https://codereview.chromium.org/2948593002/

Due to performance regression, splitting it into multiple patches. This is the
initial landing
without enabling functionality. In next patch functionality will be enabled with
necessary test coverage.

BUG=410785, 648986, 733222
==========

to

==========
Relanding [Android] Adding Smart GO/NEXT feature in Chrome

Smart Go/Next brings better user experience to the user during form submitting
applications.
For navigating between form elements, user can use NEXT/PREVIOUS button from IME
without touching on individual fields. This will avoid unnecessary form
submissions before
filling or visiting all fields in the form.

Additionally it will save user time and avoid redundant network requests before
actually
filling/attending entire fields in the form

Design Document:
https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSP...

Initial patch is reviewed @ https://codereview.chromium.org/2839993002/ and
https://codereview.chromium.org/2948593002/

Due to performance regression, splitting it into multiple patches. This is the
initial landing
without enabling functionality. In next patch functionality will be enabled with
necessary test coverage.

BUG=410785, 648986, 733222
==========

[nasko, 2017-06-29 18:46:38]

> Due to performance regression, splitting it into multiple patches. 
> This is the initial landing without enabling functionality. 
> In next patch functionality will be enabled with necessary test coverage.

Do we understand what causes the regression? Just landing code in pieces without
understanding whether the regression is something that can be addressed doesn't
make much sense.

[AKVT, 2017-06-29 19:32:55]

On 2017/06/29 18:46:38, nasko (slow) wrote:
> > Due to performance regression, splitting it into multiple patches. 
> > This is the initial landing without enabling functionality. 
> > In next patch functionality will be enabled with necessary test coverage.
> 
> Do we understand what causes the regression? Just landing code in pieces
without
> understanding whether the regression is something that can be addressed
doesn't
> make much sense.

Scrolling performance is getting effected due to call to
NextFocusableElementInForm() from InputMethodController. I am identifying the
optimization cases (Only for form control elements or for input text field).
Since it is a big patch, kochi@ suggested to split in multiple chunks for
landing easily without disturbing performance.

[AKVT, 2017-06-30 14:02:00]

kochi@, changwan@, tkent@, nasko@ - Could you PTAL. As discussed I will be
enabling functionality in next patch.

[nasko, 2017-06-30 17:18:38]

On 2017/06/29 19:32:55, AKVT wrote:
> On 2017/06/29 18:46:38, nasko (slow) wrote:
> > > Due to performance regression, splitting it into multiple patches. 
> > > This is the initial landing without enabling functionality. 
> > > In next patch functionality will be enabled with necessary test coverage.
> > 
> > Do we understand what causes the regression? Just landing code in pieces
> without
> > understanding whether the regression is something that can be addressed
> doesn't
> > make much sense.
> 
> Scrolling performance is getting effected due to call to
> NextFocusableElementInForm() from InputMethodController. I am identifying the
> optimization cases (Only for form control elements or for input text field).
> Since it is a big patch, kochi@ suggested to split in multiple chunks for
> landing easily without disturbing performance.

Please include some of this context in the CL description. If the source
of the perf regression is well understood and can be fixed subsequently, 
then landing this seems fine. LGTM.

[AKVT, 2017-06-30 19:52:33]

Description was changed from

==========
Relanding [Android] Adding Smart GO/NEXT feature in Chrome

Smart Go/Next brings better user experience to the user during form submitting
applications.
For navigating between form elements, user can use NEXT/PREVIOUS button from IME
without touching on individual fields. This will avoid unnecessary form
submissions before
filling or visiting all fields in the form.

Additionally it will save user time and avoid redundant network requests before
actually
filling/attending entire fields in the form

Design Document:
https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSP...

Initial patch is reviewed @ https://codereview.chromium.org/2839993002/ and
https://codereview.chromium.org/2948593002/

Due to performance regression, splitting it into multiple patches. This is the
initial landing
without enabling functionality. In next patch functionality will be enabled with
necessary test coverage.

BUG=410785, 648986, 733222
==========

to

==========
Relanding [Android] Adding Smart GO/NEXT feature in Chrome

Smart Go/Next brings better user experience to the user during form submitting
applications.
For navigating between form elements, user can use NEXT/PREVIOUS button from IME
without touching on individual fields. This will avoid unnecessary form
submissions before
filling or visiting all fields in the form.

Additionally it will save user time and avoid redundant network requests before
actually
filling/attending entire fields in the form

Design Document:
https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSP...

Initial patch is reviewed @ https://codereview.chromium.org/2839993002/ and
https://codereview.chromium.org/2948593002/

Scrolling performance is getting effected due to call to
NextFocusableElementInForm() from InputMethodController.

hence splitting it into multiple patches. This is the initial landing
without enabling functionality. In next patch functionality will be enabled with
necessary test coverage.

BUG=410785, 648986, 733222
==========

[AKVT, 2017-06-30 19:53:04]

Description was changed from

==========
Relanding [Android] Adding Smart GO/NEXT feature in Chrome

Smart Go/Next brings better user experience to the user during form submitting
applications.
For navigating between form elements, user can use NEXT/PREVIOUS button from IME
without touching on individual fields. This will avoid unnecessary form
submissions before
filling or visiting all fields in the form.

Additionally it will save user time and avoid redundant network requests before
actually
filling/attending entire fields in the form

Design Document:
https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSP...

Initial patch is reviewed @ https://codereview.chromium.org/2839993002/ and
https://codereview.chromium.org/2948593002/

Scrolling performance is getting effected due to call to
NextFocusableElementInForm() from InputMethodController.

hence splitting it into multiple patches. This is the initial landing
without enabling functionality. In next patch functionality will be enabled with
necessary test coverage.

BUG=410785, 648986, 733222
==========

to

==========
Relanding [Android] Adding Smart GO/NEXT feature in Chrome

Smart Go/Next brings better user experience to the user during form submitting
applications.
For navigating between form elements, user can use NEXT/PREVIOUS button from IME
without touching on individual fields. This will avoid unnecessary form
submissions before
filling or visiting all fields in the form.

Additionally it will save user time and avoid redundant network requests before
actually
filling/attending entire fields in the form

Design Document:
https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSP...

Initial patch is reviewed @ https://codereview.chromium.org/2839993002/ and
https://codereview.chromium.org/2948593002/

Scrolling performance is getting effected due to call to
NextFocusableElementInForm() from InputMethodController. hence splitting it into
multiple patches. This is the initial landing
without enabling functionality. In next patch functionality will be enabled with
necessary test coverage.

BUG=410785, 648986, 733222
==========

[AKVT, 2017-06-30 19:53:34]

Description was changed from

==========
Relanding [Android] Adding Smart GO/NEXT feature in Chrome

Smart Go/Next brings better user experience to the user during form submitting
applications.
For navigating between form elements, user can use NEXT/PREVIOUS button from IME
without touching on individual fields. This will avoid unnecessary form
submissions before
filling or visiting all fields in the form.

Additionally it will save user time and avoid redundant network requests before
actually
filling/attending entire fields in the form

Design Document:
https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSP...

Initial patch is reviewed @ https://codereview.chromium.org/2839993002/ and
https://codereview.chromium.org/2948593002/

Scrolling performance is getting effected due to call to
NextFocusableElementInForm() from InputMethodController. hence splitting it into
multiple patches. This is the initial landing
without enabling functionality. In next patch functionality will be enabled with
necessary test coverage.

BUG=410785, 648986, 733222
==========

to

==========
Relanding [Android] Adding Smart GO/NEXT feature in Chrome

Smart Go/Next brings better user experience to the user during form submitting
applications.
For navigating between form elements, user can use NEXT/PREVIOUS button from IME
without touching on individual fields. This will avoid unnecessary form
submissions before
filling or visiting all fields in the form.

Additionally it will save user time and avoid redundant network requests before
actually
filling/attending entire fields in the form

Design Document:
https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSP...

Initial patch is reviewed @ https://codereview.chromium.org/2839993002/ and
https://codereview.chromium.org/2948593002/

Scrolling performance is getting effected due to call to
NextFocusableElementInForm() from InputMethodController. hence splitting it into
multiple patches.
This is the initial landing
without enabling functionality.
In next patch functionality will be enabled with necessary test coverage.

BUG=410785, 648986, 733222
==========

[AKVT, 2017-06-30 19:53:55]

Description was changed from

==========
Relanding [Android] Adding Smart GO/NEXT feature in Chrome

Smart Go/Next brings better user experience to the user during form submitting
applications.
For navigating between form elements, user can use NEXT/PREVIOUS button from IME
without touching on individual fields. This will avoid unnecessary form
submissions before
filling or visiting all fields in the form.

Additionally it will save user time and avoid redundant network requests before
actually
filling/attending entire fields in the form

Design Document:
https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSP...

Initial patch is reviewed @ https://codereview.chromium.org/2839993002/ and
https://codereview.chromium.org/2948593002/

Scrolling performance is getting effected due to call to
NextFocusableElementInForm() from InputMethodController. hence splitting it into
multiple patches.
This is the initial landing
without enabling functionality.
In next patch functionality will be enabled with necessary test coverage.

BUG=410785, 648986, 733222
==========

to

==========
Relanding [Android] Adding Smart GO/NEXT feature in Chrome

Smart Go/Next brings better user experience to the user during form submitting
applications.
For navigating between form elements, user can use NEXT/PREVIOUS button from IME
without touching on individual fields. This will avoid unnecessary form
submissions before
filling or visiting all fields in the form.

Additionally it will save user time and avoid redundant network requests before
actually
filling/attending entire fields in the form

Design Document:
https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSP...

Initial patch is reviewed @ https://codereview.chromium.org/2839993002/ and
https://codereview.chromium.org/2948593002/

Scrolling performance is getting effected due to call to
NextFocusableElementInForm() from InputMethodController. hence splitting it into
multiple patches.
This is the initial landing without enabling functionality.
In next patch functionality will be enabled with necessary test coverage.

BUG=410785, 648986, 733222
==========

[tkent, 2017-07-02 22:55:03]

On 2017/06/30 at 14:02:00, ajith.v wrote:
> kochi@, changwan@, tkent@, nasko@ - Could you PTAL. As discussed I will be
enabling functionality in next patch.

Please specify who should review what part of the CL.

Anyway, third_party/WebKit lgtm.

[AKVT, 2017-07-03 09:09:21]

changwan@ - Could you PTAL content/public changes again for landing initial
patch ?

[Takayoshi Kochi (Google), 2017-07-03 09:09:36]

kochi@google.com changed reviewers:
+ kochi@google.com

[Takayoshi Kochi (Google), 2017-07-03 09:09:37]

FocusController change LGTM

changwan@, could you check Android-specific part?

[Changwan Ryu, 2017-07-03 09:33:01]

On 2017/07/03 09:09:37, Takayoshi Kochi (Google) wrote:
> FocusController change LGTM
> 
> changwan@, could you check Android-specific part?

/input/ lgtm assuming imetest will be added soon - we're enabling functionality
on advancefocusinform() side of the cl.

It might make sense to land the fix to swap inputtype and inputflags in a
separate cl.

[AKVT, 2017-07-03 09:48:54]

On 2017/07/03 09:33:01, Changwan Ryu wrote:
> On 2017/07/03 09:09:37, Takayoshi Kochi (Google) wrote:
> > FocusController change LGTM
> > 
> > changwan@, could you check Android-specific part?
> 
> /input/ lgtm assuming imetest will be added soon - we're enabling
functionality
> on advancefocusinform() side of the cl.
> 
> It might make sense to land the fix to swap inputtype and inputflags in a
> separate cl.

changwan@ - ASFAIU, Even though advancefocusinform() method we are calling from
ImeAdapter, because of absence of IME_ACTION_NEXT, functionality won't be
executed with this patch. I will land test coverage along with functional
enabling of feature.

Could you plese clarify below statement

"It might make sense to land the fix to swap inputtype and inputflags in a
separate cl." ?

[AKVT, 2017-07-03 15:05:55]

The CQ bit was checked by ajith.v@samsung.com

[commit-bot: I haz the power, 2017-07-03 15:06:04]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-03 17:05:31]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1499094355451660, "parent_rev":
"7c72ce06f5f925b7309ede559ba7d8880ea1146c", "commit_rev":
"933aca75233fbb5145c29c1e5a06e5f36c308cc0"}

[commit-bot: I haz the power, 2017-07-03 17:05:46]

Description was changed from

==========
Relanding [Android] Adding Smart GO/NEXT feature in Chrome

Smart Go/Next brings better user experience to the user during form submitting
applications.
For navigating between form elements, user can use NEXT/PREVIOUS button from IME
without touching on individual fields. This will avoid unnecessary form
submissions before
filling or visiting all fields in the form.

Additionally it will save user time and avoid redundant network requests before
actually
filling/attending entire fields in the form

Design Document:
https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSP...

Initial patch is reviewed @ https://codereview.chromium.org/2839993002/ and
https://codereview.chromium.org/2948593002/

Scrolling performance is getting effected due to call to
NextFocusableElementInForm() from InputMethodController. hence splitting it into
multiple patches.
This is the initial landing without enabling functionality.
In next patch functionality will be enabled with necessary test coverage.

BUG=410785, 648986, 733222
==========

to

==========
Relanding [Android] Adding Smart GO/NEXT feature in Chrome

Smart Go/Next brings better user experience to the user during form submitting
applications.
For navigating between form elements, user can use NEXT/PREVIOUS button from IME
without touching on individual fields. This will avoid unnecessary form
submissions before
filling or visiting all fields in the form.

Additionally it will save user time and avoid redundant network requests before
actually
filling/attending entire fields in the form

Design Document:
https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSP...

Initial patch is reviewed @ https://codereview.chromium.org/2839993002/ and
https://codereview.chromium.org/2948593002/

Scrolling performance is getting effected due to call to
NextFocusableElementInForm() from InputMethodController. hence splitting it into
multiple patches.
This is the initial landing without enabling functionality.
In next patch functionality will be enabled with necessary test coverage.

BUG=410785, 648986, 733222

Review-Url: https://codereview.chromium.org/2967493002
Cr-Commit-Position: refs/heads/master@{#484020}
Committed:
https://chromium.googlesource.com/chromium/src/+/933aca75233fbb5145c29c1e5a06...
==========

[commit-bot: I haz the power, 2017-07-03 17:05:48]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/933aca75233fbb5145c29c1e5a06...