Hide password saving UI for automated tests.

Hide password saving UI for automated tests.

BUG=chromedriver:1015
Review-Url: https://codereview.chromium.org/2722293004
Cr-Commit-Position: refs/heads/master@{#454935}
Committed: https://chromium.googlesource.com/chromium/src/+/58f544ff4a6a83322f90229ab7736d9d1d77d6bf

[samuong, 2017-03-03 00:03:05]

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

[samuong, 2017-03-03 00:03:05]

https://codereview.chromium.org/2722293004/diff/20001/chrome/browser/password...
File chrome/browser/password_manager/chrome_password_manager_client.cc (left):

https://codereview.chromium.org/2722293004/diff/20001/chrome/browser/password...
chrome/browser/password_manager/chrome_password_manager_client.cc:214:
IsFillingEnabledForCurrentPage();
I didn't want to add the HasSwitch check to this very long && statement, and for
consistency I converted it into a series of if statements. Let me know if you
disagree.

[vabr (Chromium), 2017-03-04 20:00:20]

Thanks. LGTM once you address my comment.
Cheers,
Vaclav

https://codereview.chromium.org/2722293004/diff/20001/chrome/browser/password...
File chrome/browser/password_manager/chrome_password_manager_client.cc (left):

https://codereview.chromium.org/2722293004/diff/20001/chrome/browser/password...
chrome/browser/password_manager/chrome_password_manager_client.cc:214:
IsFillingEnabledForCurrentPage();
On 2017/03/03 00:03:05, samuong wrote:
> I didn't want to add the HasSwitch check to this very long && statement, and
for
> consistency I converted it into a series of if statements. Let me know if you
> disagree.

Adding the test for kEnableAutomation as a separate block before the current
return statement sounds good. But please keep the rest (lines 211-214 in the
original file) as is, there is no need to split that. It will also make git
history/blame easier to follow.

[samuong, 2017-03-06 18:32:01]

https://codereview.chromium.org/2722293004/diff/20001/chrome/browser/password...
File chrome/browser/password_manager/chrome_password_manager_client.cc (left):

https://codereview.chromium.org/2722293004/diff/20001/chrome/browser/password...
chrome/browser/password_manager/chrome_password_manager_client.cc:214:
IsFillingEnabledForCurrentPage();
On 2017/03/04 20:00:20, vabr (Chromium) wrote:
> On 2017/03/03 00:03:05, samuong wrote:
> > I didn't want to add the HasSwitch check to this very long && statement, and
> for
> > consistency I converted it into a series of if statements. Let me know if
you
> > disagree.
> 
> Adding the test for kEnableAutomation as a separate block before the current
> return statement sounds good. But please keep the rest (lines 211-214 in the
> original file) as is, there is no need to split that. It will also make git
> history/blame easier to follow.

Done.

[samuong, 2017-03-06 18:32:05]

The CQ bit was checked by samuong@chromium.org

[samuong, 2017-03-06 18:32:06]

The patchset sent to the CQ was uploaded after l-g-t-m from vabr@chromium.org
Link to the patchset: https://codereview.chromium.org/2722293004/#ps40001
(title: "don't split up the && expression")

[commit-bot: I haz the power, 2017-03-06 18:32:59]

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-03-06 20:20:46]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1488825125853610,
"parent_rev": "16beff659fc7bc169da48a13d926e05b01053bea", "commit_rev":
"58f544ff4a6a83322f90229ab7736d9d1d77d6bf"}

[commit-bot: I haz the power, 2017-03-06 20:21:29]

Description was changed from

==========
Hide password saving UI for automated tests.

BUG=chromedriver:1015
==========

to

==========
Hide password saving UI for automated tests.

BUG=chromedriver:1015

Review-Url: https://codereview.chromium.org/2722293004
Cr-Commit-Position: refs/heads/master@{#454935}
Committed:
https://chromium.googlesource.com/chromium/src/+/58f544ff4a6a83322f90229ab773...
==========

[commit-bot: I haz the power, 2017-03-06 20:21:30]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/58f544ff4a6a83322f90229ab773...