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/

BUG=410785, 648986, 733222
Review-Url: https://codereview.chromium.org/2948593002
Cr-Commit-Position: refs/heads/master@{#481868}
Committed: https://chromium.googlesource.com/chromium/src/+/4eb6812cc312e4ea919518a4e3087944e31b3b72

[AKVT, 2017-06-19 16:59:12]

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

[AKVT, 2017-06-19 16:59:13]

PTAL!

FYI: Same has been reviewed @ https://codereview.chromium.org/2839993002/. But
due to clusterfuzz bugs, has to revert back.

[AKVT, 2017-06-19 17:01:08]

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/

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

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/

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

[AKVT, 2017-06-19 17:01:08]

ajith.v@samsung.com changed reviewers:
+ tedchoc@chromium.org, yfriedman@chromium.org

[Takayoshi Kochi (Google), 2017-06-20 01:49:28]

On 2017/06/19 16:59:13, AKVT wrote:
> PTAL!
> 
> FYI: Same has been reviewed @ https://codereview.chromium.org/2839993002/. But
> due to clusterfuzz bugs, has to revert back.

It would be easier to review if you upload the 2839993002 as is, then
apply the new change and upload again.  Then we can view the new diff
very easily.

[AKVT, 2017-06-20 03:06:57]

On 2017/06/20 01:49:28, Takayoshi Kochi (Google) wrote:
> On 2017/06/19 16:59:13, AKVT wrote:
> > PTAL!
> > 
> > FYI: Same has been reviewed @ https://codereview.chromium.org/2839993002/.
But
> > due to clusterfuzz bugs, has to revert back.
> 
> It would be easier to review if you upload the 2839993002 as is, then
> apply the new change and upload again.  Then we can view the new diff
> very easily.

@kochi - Sure. I will do that ASAP

[AKVT, 2017-06-20 11:58:51]

Patchset #1 (id:1) has been deleted

[AKVT, 2017-06-20 13:55:10]

@kochi and all PTAL!

I have uploaded initial patch from https://codereview.chromium.org/2839993002/
and latest patch with Cluster Fuzz issue fix.

[Takayoshi Kochi (Google), 2017-06-21 02:02:04]

On 2017/06/20 13:55:10, AKVT wrote:
> @kochi and all PTAL!
> 
> I have uploaded initial patch from https://codereview.chromium.org/2839993002/
> and latest patch with Cluster Fuzz issue fix.

Thanks!

As the previous CL got l-g-t-ms from all, and addition is essentially
https://chromium-review.googlesource.com/c/536395/
(minus FRIEND_TEST_ALL_PREFIXES declaration, which was not necessary),
so LGTM from me.

[yosin_UTC9, 2017-06-21 02:23:26]

yosin@chromium.org changed reviewers:
+ kochi@google.com

[yosin_UTC9, 2017-06-21 02:23:27]

lgtm for core/editing; there are no changes.

[AKVT, 2017-06-21 07:21:18]

On 2017/06/21 02:23:27, yosin_UTC9 wrote:
> lgtm for core/editing; there are no changes.

@kochi - Do we need to add SVG element in WebViewTest also ?

[Takayoshi Kochi (Google), 2017-06-21 07:31:26]

On 2017/06/21 07:21:18, AKVT wrote:
> On 2017/06/21 02:23:27, yosin_UTC9 wrote:
> > lgtm for core/editing; there are no changes.
> 
> @kochi - Do we need to add SVG element in WebViewTest also ?

SVG elements in form should be skipped in smart go/next feature, and
that is already covered in FocusControllerTest.
So I think it would be optional.

[AKVT, 2017-06-21 15:14:41]

ajith.v@samsung.com changed reviewers:
+ nasko@chromium.org

[AKVT, 2017-06-21 15:15:18]

On 2017/06/21 07:31:26, Takayoshi Kochi (Google) wrote:
> On 2017/06/21 07:21:18, AKVT wrote:
> > On 2017/06/21 02:23:27, yosin_UTC9 wrote:
> > > lgtm for core/editing; there are no changes.
> > 
> > @kochi - Do we need to add SVG element in WebViewTest also ?
> 
> SVG elements in form should be skipped in smart go/next feature, and
> that is already covered in FocusControllerTest.
> So I think it would be optional.

Thanks kochi@ for the update.

[AKVT, 2017-06-21 15:15:51]

@all PTAL!

[Changwan Ryu, 2017-06-21 15:46:49]

On 2017/06/21 15:15:51, AKVT wrote:
> @all PTAL!

lgtm for input/

[AKVT, 2017-06-22 07:41:06]

@tkent, @nasko, @dcheng, @tedchoc, @yaron - PTAL content/ and WebKit/ changes

[AKVT, 2017-06-22 07:43:06]

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/

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

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/

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

[tkent, 2017-06-22 07:44:53]

third_party/WebKit lgtm

[nasko, 2017-06-22 15:30:31]

Rubberstamp LGTM on content/, as there are no changes.

[AKVT, 2017-06-22 15:34:05]

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

[commit-bot: I haz the power, 2017-06-22 15:34:23]

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-06-22 17:10:10]

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

[commit-bot: I haz the power, 2017-06-22 17:10:11]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Takayoshi Kochi (Google), 2017-06-23 07:00:13]

You forgot to add test files in third_party/WebKit/Source/web/tests/data/
in the original CL?

[AKVT, 2017-06-23 07:32:35]

On 2017/06/23 07:00:13, Takayoshi Kochi (Google) wrote:
> You forgot to add test files in third_party/WebKit/Source/web/tests/data/
> in the original CL?

Thanks kochi@ for pointing it. I was just cherry picked the patch from older
review link and applied fixes. I just noticed now test html files are missed in
PS2. I will upload in PS3

[AKVT, 2017-06-23 07:50:22]

On 2017/06/23 07:32:35, AKVT wrote:
> On 2017/06/23 07:00:13, Takayoshi Kochi (Google) wrote:
> > You forgot to add test files in third_party/WebKit/Source/web/tests/data/
> > in the original CL?
> 
> Thanks kochi@ for pointing it. I was just cherry picked the patch from older
> review link and applied fixes. I just noticed now test html files are missed
in
> PS2. I will upload in PS3

@kochi - PTAL PS3 for landing.

[Takayoshi Kochi (Google), 2017-06-23 08:29:00]

The CQ bit was checked by kochi@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-23 08:29:03]

Dry run: 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-06-23 08:31:12]

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

[commit-bot: I haz the power, 2017-06-23 08:31:14]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[Takayoshi Kochi (Google), 2017-06-23 08:32:43]

On 2017/06/23 07:50:22, AKVT wrote:
> On 2017/06/23 07:32:35, AKVT wrote:
> > On 2017/06/23 07:00:13, Takayoshi Kochi (Google) wrote:
> > > You forgot to add test files in third_party/WebKit/Source/web/tests/data/
> > > in the original CL?
> > 
> > Thanks kochi@ for pointing it. I was just cherry picked the patch from older
> > review link and applied fixes. I just noticed now test html files are missed
> in
> > PS2. I will upload in PS3
> 
> @kochi - PTAL PS3 for landing.

Thanks, you forgot to "git add" newly created files after patching manually?

Just in case you didn't know, there is a convenient method to apply
existing CL patch locally.
% git cl patch 2839993002
will load the latest CL from codereview and apply locally, then commits.
This branch will be associated with the CL number, but you can reset by
% git cl issue 0
to dissociate from the old review, then add more local fixes, commit, and
% git cl upload
will create a new code review CL.

[Takayoshi Kochi (Google), 2017-06-23 08:38:26]

Could you try rebasing?

[AKVT, 2017-06-23 08:59:01]

On 2017/06/23 08:32:43, Takayoshi Kochi (Google) wrote:
> On 2017/06/23 07:50:22, AKVT wrote:
> > On 2017/06/23 07:32:35, AKVT wrote:
> > > On 2017/06/23 07:00:13, Takayoshi Kochi (Google) wrote:
> > > > You forgot to add test files in
third_party/WebKit/Source/web/tests/data/
> > > > in the original CL?
> > > 
> > > Thanks kochi@ for pointing it. I was just cherry picked the patch from
older
> > > review link and applied fixes. I just noticed now test html files are
missed
> > in
> > > PS2. I will upload in PS3
> > 
> > @kochi - PTAL PS3 for landing.
> 
> Thanks, you forgot to "git add" newly created files after patching manually?
> 
> Just in case you didn't know, there is a convenient method to apply
> existing CL patch locally.
> % git cl patch 2839993002
> will load the latest CL from codereview and apply locally, then commits.
> This branch will be associated with the CL number, but you can reset by
> % git cl issue 0
> to dissociate from the old review, then add more local fixes, commit, and
> % git cl upload
> will create a new code review CL.

Thanks kochi@ for the tips. I was following above points except "git cl issue
0".

[AKVT, 2017-06-23 08:59:26]

On 2017/06/23 08:38:26, Takayoshi Kochi (Google) wrote:
> Could you try rebasing?

Rebased the patch in PS4

[Takayoshi Kochi (Google), 2017-06-23 09:12:09]

The CQ bit was checked by kochi@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-23 09:12:15]

Dry run: 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-06-23 12:04:59]

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

[commit-bot: I haz the power, 2017-06-23 12:05:01]

Dry run: This issue passed the CQ dry run.

[AKVT, 2017-06-23 13:33:31]

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

[AKVT, 2017-06-23 13:33:32]

The patchset sent to the CQ was uploaded after l-g-t-m from kochi@google.com,
tkent@chromium.org, changwan@chromium.org, nasko@chromium.org,
yosin@chromium.org
Link to the patchset: https://codereview.chromium.org/2948593002/#ps80001
(title: "Rebased the patch")

[commit-bot: I haz the power, 2017-06-23 13:33:39]

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-06-23 13:38:19]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1498224811393030,
"parent_rev": "c3d21f501b7259e80f600dfed9e7eb42074c63e5", "commit_rev":
"4eb6812cc312e4ea919518a4e3087944e31b3b72"}

[commit-bot: I haz the power, 2017-06-23 13:38:35]

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/

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/

BUG=410785,648986, 733222

Review-Url: https://codereview.chromium.org/2948593002
Cr-Commit-Position: refs/heads/master@{#481868}
Committed:
https://chromium.googlesource.com/chromium/src/+/4eb6812cc312e4ea919518a4e308...
==========

[commit-bot: I haz the power, 2017-06-23 13:38:36]

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/4eb6812cc312e4ea919518a4e308...

[Takayoshi Kochi (Google), 2017-06-28 05:43:53]

A revert of this CL (patchset #4 id:80001) has been created in
https://codereview.chromium.org/2955283002/ by kochi@google.com.

The reason for reverting is: This caused perf regression on
browse_social_facebook_infinite_scroll
test on most platforms.

See crbug.com/737388 for details..