New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 807623 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Task

Blocking:
issue 708605
issue 708606
issue 708634
issue 709012
issue 709031


Show other hotlists

Hotlists containing this issue:
Hotlist-8


Sign in to add a comment

IOS password form parsing rewriting

Project Member Reported by dvadym@chromium.org, Jan 31 2018

Issue description

Passoword form parsing on IOS is much worse than on other platforms and many bugs might be fixed with rewriting it.

Details in go/ios-password-form-parsing
This bug is for tracking work.
 
Blocking: 709012
Blocking: 708605
Blocking: 708606
Cc: jyqu...@chromium.org dvadym@chromium.org
 Issue 564578  has been merged into this issue.
Blocking: 709031
Project Member

Comment 6 by bugdroid1@chromium.org, Feb 5 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/799bb66dd1f461e60e4679c9de26cd9499a7a1df

commit 799bb66dd1f461e60e4679c9de26cd9499a7a1df
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Mon Feb 05 11:42:31 2018

[IOS Password Manager] Form parser.

This is implementation of new form parsing algorithms.
More details in go/ios-password-form-parsing sections
  - changes in code
  - form parsing algorithms

Bug:  807623 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I073b375ba4f6ba1b0fabe02e80ed171363438ceb
Reviewed-on: https://chromium-review.googlesource.com/860138
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534367}
[modify] https://crrev.com/799bb66dd1f461e60e4679c9de26cd9499a7a1df/ios/chrome/browser/passwords/BUILD.gn
[add] https://crrev.com/799bb66dd1f461e60e4679c9de26cd9499a7a1df/ios/chrome/browser/passwords/form_parser.cc
[add] https://crrev.com/799bb66dd1f461e60e4679c9de26cd9499a7a1df/ios/chrome/browser/passwords/form_parser.h
[add] https://crrev.com/799bb66dd1f461e60e4679c9de26cd9499a7a1df/ios/chrome/browser/passwords/form_parser_unittest.cc

Thanks, Vadym! It looks like this bug could potentially fix 4 bugs. Please mark this one and the other four appropriately so that the testing team could verify them when ready. 
Cc: -jyqu...@chromium.org pinkerton@chromium.org noyau@chromium.org
Labels: -Pri-3 Pri-2
Project Member

Comment 9 by bugdroid1@chromium.org, Mar 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8029f38aa8f97fa2f90119b0d3cd2eba7220eb7c

commit 8029f38aa8f97fa2f90119b0d3cd2eba7220eb7c
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Thu Mar 08 16:00:21 2018

Move part of autofill_controller.js to fill.js.

On CL https://chromium-review.googlesource.com/c/chromium/src/+/901664 added
a call of function __gCrWeb.autofill.webFormElementToFormData from
password_controller.js. On codereview it was suggested
https://chromium-review.googlesource.com/c/chromium/src/+/901664/27/ios/chrome/browser/passwords/resources/password_controller.js#296
to move this function to fill.js. It makes sense, since fill.js is supposed
to contain common parts between Autofill and Password Manager. This CL moves
webFormElementToFormData and all dependencies to fill.js. The order from
autofill_controller.js is preserved.

Bug:  807623 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ic538ac44471d19c4e899d4d3450b92bed7da5c0e
Reviewed-on: https://chromium-review.googlesource.com/951609
Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541806}
[modify] https://crrev.com/8029f38aa8f97fa2f90119b0d3cd2eba7220eb7c/components/autofill/ios/browser/resources/autofill_controller.js
[modify] https://crrev.com/8029f38aa8f97fa2f90119b0d3cd2eba7220eb7c/components/autofill/ios/fill/resources/fill.js
[modify] https://crrev.com/8029f38aa8f97fa2f90119b0d3cd2eba7220eb7c/ios/chrome/browser/autofill/autofill_controller_js_unittest.mm

Blocking: 708634
Project Member

Comment 11 by bugdroid1@chromium.org, Mar 29 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0ae027437e1c95625fca8eb857b9303e60250714

commit 0ae027437e1c95625fca8eb857b9303e60250714
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Thu Mar 29 18:47:40 2018

[IOS Password Manager] Plug-in FormParser in Password form parsing.

On CL https://chromium-review.googlesource.com/860138 FormParser was implemented.
This CL makes use of FormParser in Password Manager. It contains:
1.Minor fixes in FormParser
2.Moving JSON->FormData functions from autofill_agent.mm to autofill_util (since they will
be used in PasswordManager).
3.Calling Autofill convert utils for converting DOM->JSON from PasswordController.js
4.Calling Autofill JSON->FormData convert utils from PassswordController.mm.
5.Removing all old password form parsing code.
6.Unittests update.

More details in go/ios-password-form-parsing.

Bug:  807623 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I256301ae09f9bdad06522847022d155fa0d78ba7
Reviewed-on: https://chromium-review.googlesource.com/901664
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546893}
[modify] https://crrev.com/0ae027437e1c95625fca8eb857b9303e60250714/components/autofill/ios/browser/autofill_agent.mm
[modify] https://crrev.com/0ae027437e1c95625fca8eb857b9303e60250714/components/autofill/ios/browser/autofill_util.h
[modify] https://crrev.com/0ae027437e1c95625fca8eb857b9303e60250714/components/autofill/ios/browser/autofill_util.mm
[modify] https://crrev.com/0ae027437e1c95625fca8eb857b9303e60250714/components/autofill/ios/fill/BUILD.gn
[add] https://crrev.com/0ae027437e1c95625fca8eb857b9303e60250714/components/autofill/ios/fill/fill_js_unittest.mm
[modify] https://crrev.com/0ae027437e1c95625fca8eb857b9303e60250714/components/autofill/ios/fill/resources/fill.js
[modify] https://crrev.com/0ae027437e1c95625fca8eb857b9303e60250714/ios/chrome/browser/passwords/form_parser.cc
[modify] https://crrev.com/0ae027437e1c95625fca8eb857b9303e60250714/ios/chrome/browser/passwords/form_parser_unittest.cc
[modify] https://crrev.com/0ae027437e1c95625fca8eb857b9303e60250714/ios/chrome/browser/passwords/password_controller.mm
[modify] https://crrev.com/0ae027437e1c95625fca8eb857b9303e60250714/ios/chrome/browser/passwords/password_controller_js_unittest.mm
[modify] https://crrev.com/0ae027437e1c95625fca8eb857b9303e60250714/ios/chrome/browser/passwords/password_controller_unittest.mm
[modify] https://crrev.com/0ae027437e1c95625fca8eb857b9303e60250714/ios/chrome/browser/passwords/resources/password_controller.js

Status: Fixed (was: Started)
Project Member

Comment 13 by bugdroid1@chromium.org, Apr 18 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6a78f7f38b7c61ccf4d4950e5b771357a729509b

commit 6a78f7f38b7c61ccf4d4950e5b771357a729509b
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Wed Apr 18 13:31:49 2018

Skip non-text field in form parsing.

Password Manager has to use only text fields. This CL implements
skipping all other fields during form parsing. It already works this
way in form parsing in the Renderer process.

Bug:  807623 ,  707925 

Change-Id: I31e5f06a3a2bc8ee8368a0e2df84f58abc900faf
Reviewed-on: https://chromium-review.googlesource.com/1013580
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551656}
[modify] https://crrev.com/6a78f7f38b7c61ccf4d4950e5b771357a729509b/components/autofill/core/common/form_field_data.cc
[modify] https://crrev.com/6a78f7f38b7c61ccf4d4950e5b771357a729509b/components/autofill/core/common/form_field_data.h
[modify] https://crrev.com/6a78f7f38b7c61ccf4d4950e5b771357a729509b/components/autofill/core/common/form_field_data_unittest.cc
[modify] https://crrev.com/6a78f7f38b7c61ccf4d4950e5b771357a729509b/components/password_manager/core/browser/form_parsing/ios_form_parser.cc
[modify] https://crrev.com/6a78f7f38b7c61ccf4d4950e5b771357a729509b/components/password_manager/core/browser/form_parsing/ios_form_parser_unittest.cc

Project Member

Comment 14 by bugdroid1@chromium.org, Apr 29 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/59d9af6de66ddcbb516fa02fc6c06d58cb26ec02

commit 59d9af6de66ddcbb516fa02fc6c06d58cb26ec02
Author: Vaclav Brozek <vabr@chromium.org>
Date: Sun Apr 29 12:37:02 2018

Address comments in form parsing

This CL addresses comments raised on https://crrev.com/c/1013580 after
it landed:

* Changing name and the declaration comment of a FormFieldData method
  to reflect its effect on textarea fields.
* Adding some std::vector::reserve calls to avoid extra heap
  allocations when calling push_back in a loop with known maximal
  iteration count.

Bug:  807623 
Change-Id: Ibc9fefbf29676563bbc317e1fd8a0386d245bdd2
Reviewed-on: https://chromium-review.googlesource.com/1032791
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Roger McFarlane <rogerm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554670}
[modify] https://crrev.com/59d9af6de66ddcbb516fa02fc6c06d58cb26ec02/components/autofill/core/common/form_field_data.cc
[modify] https://crrev.com/59d9af6de66ddcbb516fa02fc6c06d58cb26ec02/components/autofill/core/common/form_field_data.h
[modify] https://crrev.com/59d9af6de66ddcbb516fa02fc6c06d58cb26ec02/components/autofill/core/common/form_field_data_unittest.cc
[modify] https://crrev.com/59d9af6de66ddcbb516fa02fc6c06d58cb26ec02/components/password_manager/core/browser/form_parsing/ios_form_parser.cc

Sign in to add a comment