Remove dependency on Finch experiment PasswordLinkInSettings

Remove dependency on Finch experiment PasswordLinkInSettings

This experiment has been ended, its launch bug is closed, we are ready to bake in the "enabled" state of the experiment. The P.M. approval is at http://crbug.com/480552#c27.

BUG=480552
Committed: https://crrev.com/2f74cdc4d932520d4cd2988f84207d3ff1210ef8
Cr-Commit-Position: refs/heads/master@{#354491}

[vabr (Chromium), 2015-10-14 14:24:58]

The CQ bit was checked by vabr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-14 14:25:08]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1396403004/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1396403004/1

[commit-bot: I haz the power, 2015-10-14 15:05:05]

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

[commit-bot: I haz the power, 2015-10-14 15:05:05]

Dry run: This issue passed the CQ dry run.

[vabr (Chromium), 2015-10-14 18:14:11]

vabr@chromium.org changed reviewers:
+ khorimoto@chromium.org

[vabr (Chromium), 2015-10-14 18:17:19]

Kyle, could you please review
chrome/browser/extensions/api/passwords_private/passwords_private_api.cc ?

Do you think the whole PasswordsPrivateCanPasswordAccountBeManagedFunction and
the JS API call it represents should be removed?

Thanks,
Vaclav

[vabr (Chromium), 2015-10-14 18:18:16]

vabr@chromium.org changed reviewers:
+ newt@chromium.org

[vabr (Chromium), 2015-10-14 18:19:15]

newt@, could you please review
chrome/browser/android/password_ui_view_android.cc
and *.java?

Thanks,
Vaclav

[vabr (Chromium), 2015-10-14 18:21:22]

vabr@chromium.org changed reviewers:
+ pkasting@chromium.org

[vabr (Chromium), 2015-10-14 18:21:22]

Peter, could you please review 
chrome/browser/ui/webui/options/password_manager_handler.cc?

Do you think I should remove the whole setManageAccountLinkVisibility?

Thanks,
Vaclav

[vabr (Chromium), 2015-10-14 18:22:27]

vabr@chromium.org changed reviewers:
+ dvadym@chromium.org

[vabr (Chromium), 2015-10-14 18:22:28]

Vadym, could you please review password_manager code and
chrome/browser/about_flags.cc?

Thanks!
Vaclav

[Kyle Horimoto, 2015-10-14 18:29:13]

On 2015/10/14 18:17:19, vabr (Chromium) wrote:
> Kyle, could you please review
> chrome/browser/extensions/api/passwords_private/passwords_private_api.cc ?
> 
> Do you think the whole PasswordsPrivateCanPasswordAccountBeManagedFunction and
> the JS API call it represents should be removed?
> 
> Thanks,
> Vaclav

lgtm as-is. Can you check to see if
passwordsPrivate.canPasswordAccountBeManaged() is ever called? If it is not, it
should be safe to remove this function entirely, but it should be part of a
separate CL.

[Peter Kasting, 2015-10-14 19:21:18]

On 2015/10/14 18:21:22, vabr (Chromium) wrote:
> Peter, could you please review 
> chrome/browser/ui/webui/options/password_manager_handler.cc?

I can, but I'm not a good webui reviewer in general.  Pick an owner from
https://chromium.googlesource.com/chromium/src/+/master/chrome/browser/ui/web...
.

> Do you think I should remove the whole setManageAccountLinkVisibility?

If it's unconditionally set to true now, then yes, I think you should remove it.

[Dan Beam, 2015-10-14 20:46:40]

On 2015/10/14 19:21:18, Peter Kasting wrote:
> On 2015/10/14 18:21:22, vabr (Chromium) wrote:
> > Peter, could you please review 
> > chrome/browser/ui/webui/options/password_manager_handler.cc?
> 
> I can, but I'm not a good webui reviewer in general.  Pick an owner from
>
https://chromium.googlesource.com/chromium/src/+/master/chrome/browser/ui/web...

webui stuff lgtm

> .
> 
> > Do you think I should remove the whole setManageAccountLinkVisibility?
> 
> If it's unconditionally set to true now, then yes, I think you should remove
it.

if you want to remove this call (as it seems to always run when the page is
initialized and always do the same thing), remove hidden here:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...

and remove these lines:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...

[dvadym, 2015-10-15 07:30:53]

On 2015/10/14 18:22:28, vabr (Chromium) wrote:
> Vadym, could you please review password_manager code and
> chrome/browser/about_flags.cc?
> 
> Thanks!
> Vaclav

password_manager and about_flags.cc LGTM

[vabr (Chromium), 2015-10-15 13:06:15]

Thanks all so far!

newt@ -- Looking forward to your opinion on
chrome/browser/android/password_ui_view_android.cc and the 2 *.java files! :)

Peter -- sorry, I misread the full path. Thanks for your answer anyway!

Vadym -- thanks for your review

Kyle -- thanks as well, I'll check out the use of
passwordsPrivate.canPasswordAccountBeManaged and send you a CL if appropriate

Dan -- I'm blown away, you fixed both my OWNERS parsing and provided a very
useful feedback! Really appreciated! I did as you told me, let me know if that
still needs changes.

Cheers,
Vaclav

[vabr (Chromium), 2015-10-15 13:06:33]

vabr@chromium.org changed reviewers:
- pkasting@chromium.org

[newt (away), 2015-10-15 18:43:00]

one comment, then password_ui_view_android.cc and Java files lgtm

https://codereview.chromium.org/1396403004/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java
(right):

https://codereview.chromium.org/1396403004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java:334:
mSavePasswordsSwitch.setDrawDivider(false);
You can remove this line as "false" is the default.

[vabr (Chromium), 2015-10-16 09:44:58]

Thanks, newt@.

Comment addressed, sending to the CQ.

Cheers,
Vaclav

https://codereview.chromium.org/1396403004/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java
(right):

https://codereview.chromium.org/1396403004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java:334:
mSavePasswordsSwitch.setDrawDivider(false);
On 2015/10/15 18:43:00, newt (OOO through 10-23) wrote:
> You can remove this line as "false" is the default.

Thanks, done. I also removed the code comment above, which seems to be only
relevant to the removed code line.

[vabr (Chromium), 2015-10-16 09:45:19]

The CQ bit was checked by vabr@chromium.org

[vabr (Chromium), 2015-10-16 09:45:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
khorimoto@chromium.org, dbeam@chromium.org, dvadym@chromium.org,
newt@chromium.org
Link to the patchset: https://codereview.chromium.org/1396403004/#ps40001
(title: "Java comment addressed")

[commit-bot: I haz the power, 2015-10-16 09:45:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1396403004/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1396403004/40001

[commit-bot: I haz the power, 2015-10-16 11:07:23]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-10-16 11:08:26]

Patchset 3 (id:??) landed as
https://crrev.com/2f74cdc4d932520d4cd2988f84207d3ff1210ef8
Cr-Commit-Position: refs/heads/master@{#354491}

[Eugene But (OOO till 7-30), 2015-10-19 16:40:29]

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

[Eugene But (OOO till 7-30), 2015-10-19 16:40:29]

https://codereview.chromium.org/1396403004/diff/40001/components/password_man...
File components/password_manager/core/common/password_manager_switches.cc
(left):

https://codereview.chromium.org/1396403004/diff/40001/components/password_man...
components/password_manager/core/common/password_manager_switches.cc:67: const
char kEnablePasswordLink[] = "enable-password-link";
Should this be also removed from the header?

[vabr (Chromium), 2015-10-19 17:19:03]

https://codereview.chromium.org/1396403004/diff/40001/components/password_man...
File components/password_manager/core/common/password_manager_switches.cc
(left):

https://codereview.chromium.org/1396403004/diff/40001/components/password_man...
components/password_manager/core/common/password_manager_switches.cc:67: const
char kEnablePasswordLink[] = "enable-password-link";
On 2015/10/19 16:40:29, eugenebut wrote:
> Should this be also removed from the header?

Ah! It should, thanks for pointing this out.
I'm not at my MacBook at the moment, will fix that tomorrow.

[vabr (Chromium), 2015-10-19 18:53:35]

On 2015/10/19 17:19:03, vabr (Chromium) wrote:
>
https://codereview.chromium.org/1396403004/diff/40001/components/password_man...
> File components/password_manager/core/common/password_manager_switches.cc
> (left):
> 
>
https://codereview.chromium.org/1396403004/diff/40001/components/password_man...
> components/password_manager/core/common/password_manager_switches.cc:67: const
> char kEnablePasswordLink[] = "enable-password-link";
> On 2015/10/19 16:40:29, eugenebut wrote:
> > Should this be also removed from the header?
> 
> Ah! It should, thanks for pointing this out.
> I'm not at my MacBook at the moment, will fix that tomorrow.

Fix is in https://codereview.chromium.org/1418483002/ (did not realise that this
was the upstream CL, for that I did not need a MacBook).