Remove ManagePasswordsUIController::NavigateToSmartLockPage

Remove ManagePasswordsUIController::NavigateToSmartLockPage

Replace it with the equivalent ManagePasswordsUIController::NavigateToExternalPasswordManager.

Note: This CL does not remove IDS_PASSWORD_MANAGER_SMART_LOCK_PAGE yet, because it is also used in other pieces of the password manager UI. We need to double-check if it is OK to merge that with chrome::kPasswordManagerAccountDashboardURL, or whether it just should be made a separate constant. Using localisation to get the hl= URL parameter is very likely an overkill, see #0 in http://crbug.com/548259.

R=vasilii@chromium.org
BUG=548259
Committed: https://crrev.com/1bff66d12e811993940e7dd01074768c30e0cd5b
Cr-Commit-Position: refs/heads/master@{#361304}

[vabr (Chromium), 2015-11-23 10:15:23]

Hi Vasilii,

PTAL.

Cheers,
Vaclav

[vasilii, 2015-11-23 10:25:22]

https://codereview.chromium.org/1467993002/diff/1/chrome/browser/ui/passwords...
File chrome/browser/ui/passwords/manage_passwords_ui_controller.cc (left):

https://codereview.chromium.org/1467993002/diff/1/chrome/browser/ui/passwords...
chrome/browser/ui/passwords/manage_passwords_ui_controller.cc:319:
GURL(l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_SMART_LOCK_PAGE)),
Why not changing the string in password_manager_infobar_delegate.cc? Then you
can make it android only.
Or you want to do it in a separate CL?

[vabr (Chromium), 2015-11-23 11:24:47]

Thanks, Vasilii.

Comment addressed, PTAL.

Cheers,
Vaclav

https://codereview.chromium.org/1467993002/diff/1/chrome/browser/ui/passwords...
File chrome/browser/ui/passwords/manage_passwords_ui_controller.cc (left):

https://codereview.chromium.org/1467993002/diff/1/chrome/browser/ui/passwords...
chrome/browser/ui/passwords/manage_passwords_ui_controller.cc:319:
GURL(l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_SMART_LOCK_PAGE)),
On 2015/11/23 10:25:21, vasilii wrote:
> Why not changing the string in password_manager_infobar_delegate.cc? Then you
> can make it android only.
> Or you want to do it in a separate CL?

Good point, keeping the link the same in the bubble and the infobars is a good
idea (I'll need to do the iOS part downstream, though).

I was almost tempted to remove also the last remaining use of
IDS_PASSWORD_MANAGER_SMART_LOCK_PAGE, in the Android account chooser, but I will
first double-check with Sabine, that the difference in the path does not matter
there. So let's keep that one for a follow-up CL. http://crbug.com/548259#c6

[vasilii, 2015-11-23 11:36:29]

lgtm

[vabr (Chromium), 2015-11-23 12:51:01]

vabr@chromium.org changed reviewers:
+ droger@chromium.org, newt@chromium.org, pam@chromium.org

[vabr (Chromium), 2015-11-23 12:51:01]

Thanks, Vasilii.

@newt: Please review chrome/browser/android/password_ui_view_android.cc

@pam: Please review chrome/browser/ui/webui/options/password_manager_handler.cc

@droger: Please review ios/chrome/browser/chrome_url_constants.*

Thanks all!
Vaclav

[vabr (Chromium), 2015-11-23 12:51:24]

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

[commit-bot: I haz the power, 2015-11-23 12:51:37]

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

[droger, 2015-11-23 12:54:39]

//ios LGTM

[commit-bot: I haz the power, 2015-11-23 12:55:26]

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

[commit-bot: I haz the power, 2015-11-23 12:55:27]

Dry run: This issue passed the CQ dry run.

[vabr (Chromium), 2015-11-23 16:45:01]

Evan, could you please stamp
chrome/browser/ui/webui/options/password_manager_handler.cc?

(Removing pam@, as it looks like I chose both wrong OWNERS (too top-level file)
and an inactive account.)

Cheers,
Vaclav

[vabr (Chromium), 2015-11-23 16:45:54]

vabr@chromium.org changed reviewers:
+ estade@chromium.org
- pam@chromium.org

[vabr (Chromium), 2015-11-23 16:45:54]

(Now adding Evan for realz.)

Evan, could you please stamp
chrome/browser/ui/webui/options/password_manager_handler.cc?

Cheers,
Vaclav

[Evan Stade, 2015-11-23 17:00:21]

On 2015/11/23 16:45:54, vabr (Chromium) wrote:
> (Now adding Evan for realz.)
> 
> Evan, could you please stamp
> chrome/browser/ui/webui/options/password_manager_handler.cc?
> 
> Cheers,
> Vaclav

webui lgtm

[newt (away), 2015-11-23 19:37:30]

chrome/browser/android lgtm

[vabr (Chromium), 2015-11-24 09:23:03]

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

[commit-bot: I haz the power, 2015-11-24 09:23:21]

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

[vabr (Chromium), 2015-11-24 09:23:22]

The CQ bit was unchecked by vabr@chromium.org

[vabr (Chromium), 2015-11-24 09:23:29]

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

[commit-bot: I haz the power, 2015-11-24 09:24:10]

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

[commit-bot: I haz the power, 2015-11-24 09:27:42]

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

[commit-bot: I haz the power, 2015-11-24 09:27:43]

Dry run: This issue passed the CQ dry run.

[vabr (Chromium), 2015-11-24 10:12:39]

The CQ bit was checked by vabr@chromium.org

[commit-bot: I haz the power, 2015-11-24 10:14:10]

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

[commit-bot: I haz the power, 2015-11-24 10:21:27]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-11-24 10:22:23]

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