Remove password manager's dependency of BrowsingDataHelper

Remove password manager's dependency of BrowsingDataHelper

As a part of the browsing_data/ refactoring in crbug.com/668114,
the BrowsingDataHelper class will no longer exist in its current form.
Before that happens, this CL removes the class' single external
dependency which is password_manager/.

The method BrowsingDataHelper::IsWebScheme() is copied to the password_manager/
codebase together with its tests. Specifically, in ChromePasswordManagerClient,
since this class contains one of its two callsites, and is reachable by
the second callsite.

It's up to discussion whether that is the semantically correct place to put
it; however, the method refers to //content and //extensions, therefore it must
be placed in //chrome/browser. Finding a different definition of a "web scheme"
would avoid that, however, this CL does not attempt that as doing so would
change the functionality of password manager.

BUG=668114
Review-Url: https://codereview.chromium.org/2656453002
Cr-Commit-Position: refs/heads/master@{#445722}
Committed: https://chromium.googlesource.com/chromium/src/+/bf350a623625a7a12a96add5421e59a1faa66fb5

[msramek, 2017-01-23 20:03:15]

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

[commit-bot: I haz the power, 2017-01-23 20:04:31]

Dry run: 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-01-23 20:14:09]

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

[commit-bot: I haz the power, 2017-01-23 20:14:10]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[msramek, 2017-01-23 20:14:31]

msramek@chromium.org changed reviewers:
+ vasilii@chromium.org

[msramek, 2017-01-23 20:14:31]

Hi Василий,

Please have a look! This is the dependency I mentioned to you a few days ago.

Thanks,
Martin

[msramek, 2017-01-23 20:19:57]

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

[commit-bot: I haz the power, 2017-01-23 20:20:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[msramek, 2017-01-23 20:38:29]

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

[commit-bot: I haz the power, 2017-01-23 20:39:05]

Dry run: 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-01-23 22:01:36]

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

[commit-bot: I haz the power, 2017-01-23 22:01:37]

Dry run: This issue passed the CQ dry run.

[vasilii, 2017-01-24 13:24:07]

lgtm

https://codereview.chromium.org/2656453002/diff/20001/chrome/browser/password...
File chrome/browser/password_manager/chrome_password_manager_client_unittest.cc
(right):

https://codereview.chromium.org/2656453002/diff/20001/chrome/browser/password...
chrome/browser/password_manager/chrome_password_manager_client_unittest.cc:563:
// CanShowBubbleOnURL currently only depends on the scheme.
Add some SCOPED_TRACE().

[msramek, 2017-01-24 13:31:30]

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

[commit-bot: I haz the power, 2017-01-24 13:31:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[msramek, 2017-01-24 13:42:32]

Thanks!

https://codereview.chromium.org/2656453002/diff/20001/chrome/browser/password...
File chrome/browser/password_manager/chrome_password_manager_client_unittest.cc
(right):

https://codereview.chromium.org/2656453002/diff/20001/chrome/browser/password...
chrome/browser/password_manager/chrome_password_manager_client_unittest.cc:563:
// CanShowBubbleOnURL currently only depends on the scheme.
On 2017/01/24 13:24:07, vasilii wrote:
> Add some SCOPED_TRACE().

Done.

[msramek, 2017-01-24 13:42:36]

The CQ bit was unchecked by msramek@chromium.org

[msramek, 2017-01-24 13:42:44]

The CQ bit was checked by msramek@chromium.org

[msramek, 2017-01-24 13:42:44]

The patchset sent to the CQ was uploaded after l-g-t-m from vasilii@chromium.org
Link to the patchset: https://codereview.chromium.org/2656453002/#ps40001
(title: "Added SCOPED_TRACE")

[commit-bot: I haz the power, 2017-01-24 13:43:01]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[msramek, 2017-01-24 13:43:28]

The CQ bit was unchecked by msramek@chromium.org

[msramek, 2017-01-24 13:50:02]

https://codereview.chromium.org/2656453002/diff/20001/chrome/browser/password...
File chrome/browser/password_manager/chrome_password_manager_client_unittest.cc
(right):

https://codereview.chromium.org/2656453002/diff/20001/chrome/browser/password...
chrome/browser/password_manager/chrome_password_manager_client_unittest.cc:563:
// CanShowBubbleOnURL currently only depends on the scheme.
On 2017/01/24 13:42:32, msramek wrote:
> On 2017/01/24 13:24:07, vasilii wrote:
> > Add some SCOPED_TRACE().
> 
> Done.

Actually let me replace spec() with possibly_invalid_spec(), as one of the test
cases is exactly for that, as the trybots will show us in a few minutes. Sorry,
forgot to re-run the test locally.

[msramek, 2017-01-24 13:50:07]

The CQ bit was checked by msramek@chromium.org

[msramek, 2017-01-24 13:50:07]

The patchset sent to the CQ was uploaded after l-g-t-m from vasilii@chromium.org
Link to the patchset: https://codereview.chromium.org/2656453002/#ps60001
(title: "s/spec/possibly_invalid_spec/")

[commit-bot: I haz the power, 2017-01-24 13:50:18]

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-01-24 14:36:49]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1485265807349250,
"parent_rev": "f172e5a02aa97817eaa96c1a658f04c612cc2810", "commit_rev":
"bf350a623625a7a12a96add5421e59a1faa66fb5"}

[commit-bot: I haz the power, 2017-01-24 14:37:22]

Description was changed from

==========
Remove password manager's dependency of BrowsingDataHelper

As a part of the browsing_data/ refactoring in crbug.com/668114,
the BrowsingDataHelper class will no longer exist in its current form.
Before that happens, this CL removes the class' single external
dependency which is password_manager/.

The method BrowsingDataHelper::IsWebScheme() is copied to the password_manager/
codebase together with its tests. Specifically, in ChromePasswordManagerClient,
since this class contains one of its two callsites, and is reachable by
the second callsite.

It's up to discussion whether that is the semantically correct place to put
it; however, the method refers to //content and //extensions, therefore it must
be placed in //chrome/browser. Finding a different definition of a "web scheme"
would avoid that, however, this CL does not attempt that as doing so would
change the functionality of password manager.

BUG=668114
==========

to

==========
Remove password manager's dependency of BrowsingDataHelper

As a part of the browsing_data/ refactoring in crbug.com/668114,
the BrowsingDataHelper class will no longer exist in its current form.
Before that happens, this CL removes the class' single external
dependency which is password_manager/.

The method BrowsingDataHelper::IsWebScheme() is copied to the password_manager/
codebase together with its tests. Specifically, in ChromePasswordManagerClient,
since this class contains one of its two callsites, and is reachable by
the second callsite.

It's up to discussion whether that is the semantically correct place to put
it; however, the method refers to //content and //extensions, therefore it must
be placed in //chrome/browser. Finding a different definition of a "web scheme"
would avoid that, however, this CL does not attempt that as doing so would
change the functionality of password manager.

BUG=668114

Review-Url: https://codereview.chromium.org/2656453002
Cr-Commit-Position: refs/heads/master@{#445722}
Committed:
https://chromium.googlesource.com/chromium/src/+/bf350a623625a7a12a96add5421e...
==========

[commit-bot: I haz the power, 2017-01-24 14:37:23]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/bf350a623625a7a12a96add5421e...