Move password-related methods from core.js to password_controller.js.

Move password-related methods from core.js to password_controller.js.

The following methods are moved:
- hasPasswordField: public API method in gCrWeb
- hasPasswordField_: private implementation

These methods are only used in password_controller.js. Unittests are also moved.

BUG=614092
TEST=Passwords autofill works correctly

Review-Url: https://codereview.chromium.org/2814773002
Cr-Commit-Position: refs/heads/master@{#463746}
Committed: https://chromium.googlesource.com/chromium/src/+/2f7838f65b679f68aca8e1b0e3886442c6e74670

[danyao, 2017-04-11 15:48:19]

danyao@chromium.org changed reviewers:
+ eugenebut@chromium.org

[Eugene But (OOO till 7-30), 2017-04-11 16:10:28]

Description was changed from

==========
Move password-related methods from core.js to password_controller.js.

The following methods are moved:
- hasPasswordField: public API method in gCrWeb
- hasPasswordField_: private implementation

These methods are only used in password_controller.js. Unittests are also moved.

BUG=614092
TESTED=Ran ios_chrome_unittests and passed.
==========

to

==========
Move password-related methods from core.js to password_controller.js.

The following methods are moved:
- hasPasswordField: public API method in gCrWeb
- hasPasswordField_: private implementation

These methods are only used in password_controller.js. Unittests are also moved.

BUG=614092
TEST=Passwords autofill works correctly
==========

[Eugene But (OOO till 7-30), 2017-04-11 16:19:41]

https://codereview.chromium.org/2814773002/diff/1/ios/chrome/browser/password...
File ios/chrome/browser/passwords/password_controller_unittest.mm (right):

https://codereview.chromium.org/2814773002/diff/1/ios/chrome/browser/password...
ios/chrome/browser/passwords/password_controller_unittest.mm:752: struct
TestDataForPasswordFormDetection {
Could you please add comments to this struct and its data

https://codereview.chromium.org/2814773002/diff/1/ios/chrome/browser/password...
ios/chrome/browser/passwords/password_controller_unittest.mm:753: NSString*
pageContent;
s/pageContent/page_content

Same comment form containsPassword, testData

https://codereview.chromium.org/2814773002/diff/1/ios/chrome/browser/password...
ios/chrome/browser/passwords/password_controller_unittest.mm:754: NSNumber*
containsPassword;
Do you want to use bool or BOOL instead of NSNumber?

https://codereview.chromium.org/2814773002/diff/1/ios/chrome/browser/password...
ios/chrome/browser/passwords/password_controller_unittest.mm:757:
TEST_F(PasswordControllerTest, HasPasswordField) {
Could you please add comments to this test.

https://codereview.chromium.org/2814773002/diff/1/ios/chrome/browser/password...
File ios/chrome/browser/passwords/resources/password_controller.js (right):

https://codereview.chromium.org/2814773002/diff/1/ios/chrome/browser/password...
ios/chrome/browser/passwords/resources/password_controller.js:44: *
TODO(danyao): investigate if this method can be completely removed from the
In Chrome for iOS we use |TODO(crbug.com/<bug number>)| format, because when
people leave comments don't get updated :)

https://codereview.chromium.org/2814773002/diff/1/ios/chrome/browser/password...
ios/chrome/browser/passwords/resources/password_controller.js:44: *
TODO(danyao): investigate if this method can be completely removed from the
Per Google's Unit Testing Best Practices: "Test code using real APIs - don’t
define back doors only for tests.". go/unit-test-practices?cl=head#public-apis

Should we keep this method public (w/o TestOnly suffix) until there is a better
way to test the code?

[danyao, 2017-04-11 18:05:06]

https://codereview.chromium.org/2814773002/diff/1/ios/chrome/browser/password...
File ios/chrome/browser/passwords/password_controller_unittest.mm (right):

https://codereview.chromium.org/2814773002/diff/1/ios/chrome/browser/password...
ios/chrome/browser/passwords/password_controller_unittest.mm:752: struct
TestDataForPasswordFormDetection {
On 2017/04/11 16:19:40, Eugene But wrote:
> Could you please add comments to this struct and its data

Done.

https://codereview.chromium.org/2814773002/diff/1/ios/chrome/browser/password...
ios/chrome/browser/passwords/password_controller_unittest.mm:753: NSString*
pageContent;
On 2017/04/11 16:19:40, Eugene But wrote:
> s/pageContent/page_content
> 
> Same comment form containsPassword, testData

Done.

https://codereview.chromium.org/2814773002/diff/1/ios/chrome/browser/password...
ios/chrome/browser/passwords/password_controller_unittest.mm:754: NSNumber*
containsPassword;
On 2017/04/11 16:19:40, Eugene But wrote:
> Do you want to use bool or BOOL instead of NSNumber?

Done.

https://codereview.chromium.org/2814773002/diff/1/ios/chrome/browser/password...
ios/chrome/browser/passwords/password_controller_unittest.mm:757:
TEST_F(PasswordControllerTest, HasPasswordField) {
On 2017/04/11 16:19:40, Eugene But wrote:
> Could you please add comments to this test.

Done.

https://codereview.chromium.org/2814773002/diff/1/ios/chrome/browser/password...
File ios/chrome/browser/passwords/resources/password_controller.js (right):

https://codereview.chromium.org/2814773002/diff/1/ios/chrome/browser/password...
ios/chrome/browser/passwords/resources/password_controller.js:44: *
TODO(danyao): investigate if this method can be completely removed from the
On 2017/04/11 16:19:40, Eugene But wrote:
> Per Google's Unit Testing Best Practices: "Test code using real APIs - don’t
> define back doors only for tests.". go/unit-test-practices?cl=head#public-apis
> 
> Should we keep this method public (w/o TestOnly suffix) until there is a
better
> way to test the code?

Done.

https://codereview.chromium.org/2814773002/diff/1/ios/chrome/browser/password...
ios/chrome/browser/passwords/resources/password_controller.js:44: *
TODO(danyao): investigate if this method can be completely removed from the
On 2017/04/11 16:19:40, Eugene But wrote:
> In Chrome for iOS we use |TODO(crbug.com/<bug number>)| format, because when
> people leave comments don't get updated :)

Done. This seems too small to warrant its own bug. I've added crbug.com/614092
here and will investigate right away after this CL.

[Eugene But (OOO till 7-30), 2017-04-11 18:20:20]

Thank you for cleanup! lgtm

https://codereview.chromium.org/2814773002/diff/60001/ios/chrome/browser/pass...
File ios/chrome/browser/passwords/password_controller_unittest.mm (right):

https://codereview.chromium.org/2814773002/diff/60001/ios/chrome/browser/pass...
ios/chrome/browser/passwords/password_controller_unittest.mm:758: // Test that
the existence (or the lack of ) a password field in the page is
s/Test/Tests

Style Guide has this: "These comments should be descriptive ("Opens the file")
rather than imperative ("Open the file");"

https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head#...

https://codereview.chromium.org/2814773002/diff/60001/ios/chrome/browser/pass...
ios/chrome/browser/passwords/password_controller_unittest.mm:776: // Test that
the existence (or the lack of) a password field in a nested iframe
ditto

[danyao, 2017-04-11 18:52:54]

Thanks for the review!

https://codereview.chromium.org/2814773002/diff/60001/ios/chrome/browser/pass...
File ios/chrome/browser/passwords/password_controller_unittest.mm (right):

https://codereview.chromium.org/2814773002/diff/60001/ios/chrome/browser/pass...
ios/chrome/browser/passwords/password_controller_unittest.mm:758: // Test that
the existence (or the lack of ) a password field in the page is
On 2017/04/11 18:20:20, Eugene But wrote:
> s/Test/Tests
> 
> Style Guide has this: "These comments should be descriptive ("Opens the file")
> rather than imperative ("Open the file");"
> 
>
https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head#...

Done.

https://codereview.chromium.org/2814773002/diff/60001/ios/chrome/browser/pass...
ios/chrome/browser/passwords/password_controller_unittest.mm:776: // Test that
the existence (or the lack of) a password field in a nested iframe
On 2017/04/11 18:20:20, Eugene But wrote:
> ditto

Done.

[danyao, 2017-04-11 18:53:23]

The CQ bit was checked by danyao@chromium.org

[danyao, 2017-04-11 18:53:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
eugenebut@chromium.org
Link to the patchset: https://codereview.chromium.org/2814773002/#ps100001
(title: "Address reviewer comment (fix verb tense in test comment)")

[commit-bot: I haz the power, 2017-04-11 18:54:16]

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-04-11 20:13:06]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1491936803405960,
"parent_rev": "6c66a4bbe7ad2b59f34b53f718f1d031cd835d75", "commit_rev":
"503e83df814b68180a19fe7150adaf76536be5e8"}

[commit-bot: I haz the power, 2017-04-11 20:18:27]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1491936803405960,
"parent_rev": "57ba633b470b05156a7446c816bd74e7e342d923", "commit_rev":
"2f7838f65b679f68aca8e1b0e3886442c6e74670"}

[commit-bot: I haz the power, 2017-04-11 20:19:24]

Description was changed from

==========
Move password-related methods from core.js to password_controller.js.

The following methods are moved:
- hasPasswordField: public API method in gCrWeb
- hasPasswordField_: private implementation

These methods are only used in password_controller.js. Unittests are also moved.

BUG=614092
TEST=Passwords autofill works correctly
==========

to

==========
Move password-related methods from core.js to password_controller.js.

The following methods are moved:
- hasPasswordField: public API method in gCrWeb
- hasPasswordField_: private implementation

These methods are only used in password_controller.js. Unittests are also moved.

BUG=614092
TEST=Passwords autofill works correctly

Review-Url: https://codereview.chromium.org/2814773002
Cr-Commit-Position: refs/heads/master@{#463746}
Committed:
https://chromium.googlesource.com/chromium/src/+/2f7838f65b679f68aca8e1b0e388...
==========

[commit-bot: I haz the power, 2017-04-11 20:19:26]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/2f7838f65b679f68aca8e1b0e388...