New issue
Advanced search Search tips

Issue 782224 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Task



Sign in to add a comment

IOS Password Manager code clean-up

Project Member Reported by dvadym@chromium.org, Nov 7 2017

Issue description

This is umbrella bug for all IOS Password Manager code clean-ups.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 7 2017

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

commit 4f8d97ebd8f898ecc1132d11e955cece984a6027
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Tue Nov 07 16:31:40 2017

Removing unused function clearAutofilledPasswordsInForm.

Bug: 782224
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Idf83ec4a3e8af6c2695c735aee9aea3dde8c0a20
Reviewed-on: https://chromium-review.googlesource.com/757135
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514487}
[modify] https://crrev.com/4f8d97ebd8f898ecc1132d11e955cece984a6027/ios/chrome/browser/passwords/js_password_manager.h
[modify] https://crrev.com/4f8d97ebd8f898ecc1132d11e955cece984a6027/ios/chrome/browser/passwords/js_password_manager.mm
[modify] https://crrev.com/4f8d97ebd8f898ecc1132d11e955cece984a6027/ios/chrome/browser/passwords/resources/password_controller.js

Project Member

Comment 2 by bugdroid1@chromium.org, Nov 9 2017

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

commit 2d4212c5b1a600394c349fb85cf69ee69ca2746f
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Thu Nov 09 12:38:33 2017

Remove unused function SelectText.

Bug: 782224
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ia901897d7026cb0dee5a3cbcd6cf848d5a30a8c7
Reviewed-on: https://chromium-review.googlesource.com/758275
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515151}
[modify] https://crrev.com/2d4212c5b1a600394c349fb85cf69ee69ca2746f/ios/chrome/browser/passwords/resources/password_controller.js

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 13 2017

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

commit 71552dd95119459b8d39effe98e57486b2c7b005
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Mon Nov 13 19:36:12 2017

Remove Password Generation leftovers on IOS.

On CL https://chromium-review.googlesource.com/c/chromium/src/+/755595
Password Generation on IOS was removed. This CL removes leftovers.

Bug:  752077 , 700010 ,782224
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Idbd84d45874a877bdc0ab81299123600442f2c98
Reviewed-on: https://chromium-review.googlesource.com/758836
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516006}
[modify] https://crrev.com/71552dd95119459b8d39effe98e57486b2c7b005/ios/chrome/app/strings/ios_strings.grd
[modify] https://crrev.com/71552dd95119459b8d39effe98e57486b2c7b005/ios/chrome/browser/passwords/BUILD.gn
[modify] https://crrev.com/71552dd95119459b8d39effe98e57486b2c7b005/ios/chrome/browser/passwords/js_password_manager.h
[modify] https://crrev.com/71552dd95119459b8d39effe98e57486b2c7b005/ios/chrome/browser/passwords/js_password_manager.mm
[modify] https://crrev.com/71552dd95119459b8d39effe98e57486b2c7b005/ios/chrome/browser/passwords/password_controller.h
[modify] https://crrev.com/71552dd95119459b8d39effe98e57486b2c7b005/ios/chrome/browser/passwords/password_controller.mm
[modify] https://crrev.com/71552dd95119459b8d39effe98e57486b2c7b005/ios/chrome/browser/passwords/password_controller_js_unittest.mm
[modify] https://crrev.com/71552dd95119459b8d39effe98e57486b2c7b005/ios/chrome/browser/passwords/password_controller_unittest.mm
[delete] https://crrev.com/a31797362dd1fcf205cdce92673c0fca888ee40f/ios/chrome/browser/passwords/password_generation_edit_view.h
[delete] https://crrev.com/a31797362dd1fcf205cdce92673c0fca888ee40f/ios/chrome/browser/passwords/password_generation_edit_view.mm
[delete] https://crrev.com/a31797362dd1fcf205cdce92673c0fca888ee40f/ios/chrome/browser/passwords/password_generation_offer_view.h
[delete] https://crrev.com/a31797362dd1fcf205cdce92673c0fca888ee40f/ios/chrome/browser/passwords/password_generation_offer_view.mm
[delete] https://crrev.com/a31797362dd1fcf205cdce92673c0fca888ee40f/ios/chrome/browser/passwords/password_generation_prompt_delegate.h
[delete] https://crrev.com/a31797362dd1fcf205cdce92673c0fca888ee40f/ios/chrome/browser/passwords/password_generation_prompt_view.h
[delete] https://crrev.com/a31797362dd1fcf205cdce92673c0fca888ee40f/ios/chrome/browser/passwords/password_generation_prompt_view.mm
[delete] https://crrev.com/a31797362dd1fcf205cdce92673c0fca888ee40f/ios/chrome/browser/passwords/password_generation_prompt_view_controller.h
[delete] https://crrev.com/a31797362dd1fcf205cdce92673c0fca888ee40f/ios/chrome/browser/passwords/password_generation_prompt_view_controller.mm
[modify] https://crrev.com/71552dd95119459b8d39effe98e57486b2c7b005/ios/chrome/browser/passwords/password_generation_utils.h
[modify] https://crrev.com/71552dd95119459b8d39effe98e57486b2c7b005/ios/chrome/browser/passwords/password_generation_utils.mm
[modify] https://crrev.com/71552dd95119459b8d39effe98e57486b2c7b005/ios/chrome/browser/passwords/password_tab_helper.h
[modify] https://crrev.com/71552dd95119459b8d39effe98e57486b2c7b005/ios/chrome/browser/passwords/password_tab_helper.mm
[delete] https://crrev.com/a31797362dd1fcf205cdce92673c0fca888ee40f/ios/chrome/browser/passwords/passwords_ui_delegate.h
[delete] https://crrev.com/a31797362dd1fcf205cdce92673c0fca888ee40f/ios/chrome/browser/passwords/passwords_ui_delegate_impl.h
[delete] https://crrev.com/a31797362dd1fcf205cdce92673c0fca888ee40f/ios/chrome/browser/passwords/passwords_ui_delegate_impl.mm
[modify] https://crrev.com/71552dd95119459b8d39effe98e57486b2c7b005/ios/chrome/browser/passwords/resources/password_controller.js
[modify] https://crrev.com/71552dd95119459b8d39effe98e57486b2c7b005/ios/chrome/browser/tabs/BUILD.gn
[modify] https://crrev.com/71552dd95119459b8d39effe98e57486b2c7b005/ios/chrome/browser/tabs/tab_helper_util.mm

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 22 2017

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

commit e2eed2c1927f7371977fe28869aa034d1e443c4f
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Wed Nov 22 10:39:00 2017

Moving getElementByNameWithParent to common and simplifying.

1.getElementByNameWithParent is a generic function that's not used
 password_controller.js anymore, so common is the better place for it
(it would also allow to test it).
2.isPassword argument is not needed anymore, so function is simpified.
3.Added a test for it.

Bug: 782224
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ida9a6719f1533d36c4dd468100860046a0db809f
Reviewed-on: https://chromium-review.googlesource.com/758402
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518597}
[modify] https://crrev.com/e2eed2c1927f7371977fe28869aa034d1e443c4f/components/autofill/ios/browser/resources/suggestion_controller.js
[modify] https://crrev.com/e2eed2c1927f7371977fe28869aa034d1e443c4f/ios/chrome/browser/passwords/resources/password_controller.js

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 14 2017

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

commit e2c706ef5b1ae1b3b497f9ceca460ab0251c0556
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Thu Dec 14 19:02:56 2017

password_controller.js small improvements.

This CL contains 3 small cleanups.
1.Removed unused functions isAutofilled/setAutofilled (setAutofilled is used,
but isAutofilled is not used, so no reasons to keep them)
2.Removed try/catch in getPasswordFormDataList, this function is called only
for main frame or same origin iframes. So no need in try/catch. Moreover
function fillPasswordFormWithData_, which is called in the same circumstances,
doesn't have try/catch.
3.Rewritten for loop by iframes with one line with call of .some().

Bug: 782224
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I72a07181ffd643329b7217825ec182e948b3c57b
Reviewed-on: https://chromium-review.googlesource.com/825998
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524133}
[modify] https://crrev.com/e2c706ef5b1ae1b3b497f9ceca460ab0251c0556/ios/chrome/browser/passwords/resources/password_controller.js

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 15 2017

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

commit 851424e77977c2cf9b8466508dc31ed12eaff87b
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Fri Dec 15 23:10:25 2017

Small IOS Password Manager clean-ups.

This CL contains the following tiny clean-ups:
1.Removing checking trustLevel in AutofillAgent, which according to comment
https://chromium-review.googlesource.com/c/chromium/src/+/826476/5/components/autofill/ios/browser/autofill_agent.mm#571
is not useful anymore.
2.Making function getPasswordFormDataList to be private. It's not called
outside of password_controller.js
3.Removing a todo comment in getPasswordFormData, which doesn't have much sense now
(even on other platforms PasswordManager doesn't do anything like that).
4.Fixing comment in form.js


Bug: 782224
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ifda40db4c15b8768282f02d36e2463bf9cf58e54
Reviewed-on: https://chromium-review.googlesource.com/827939
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524505}
[modify] https://crrev.com/851424e77977c2cf9b8466508dc31ed12eaff87b/components/autofill/ios/browser/autofill_agent.mm
[modify] https://crrev.com/851424e77977c2cf9b8466508dc31ed12eaff87b/ios/chrome/browser/passwords/resources/password_controller.js
[modify] https://crrev.com/851424e77977c2cf9b8466508dc31ed12eaff87b/ios/web/web_state/js/resources/form.js

Project Member

Comment 7 by bugdroid1@chromium.org, Feb 13 2018

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

commit fae7ccb327eeb9d300b9e9b540d436e7471f7023
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Tue Feb 13 11:29:09 2018

IOS Autofill small clean-ups.

I'm working on using some Autofill code in Password Manager and found
some things that could be improved
(https://chromium-review.googlesource.com/c/chromium/src/+/901664).
1.Form method is not used anymore, so we can remove extracting it.
2.When forms are extracted from JS, instead of just returning list of
forms, an object with "forms" property is created. So we have 1 more level
indirection without any good reason. This CL changed to returning a list.

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

Sign in to add a comment