Commit 87007a51 authored by Friedrich Horschig's avatar Friedrich Horschig Committed by Commit Bot

[Passwords] Leak check clean-ups

This CL should have no user-visible changes at all.

This CL addresses post-commit comments from https://crrev.com/c/2087679
and applies some further recommended clean-ups recommended in
https://crrev.com/c/2087623 in all of password_check{,_test}.js:
 - drop params from observer functions
 - drop redundant 'test' prefix from all tests
 - replace uses of throw with assertNotReached()

Bug: 1047726
Change-Id: I93c075a218c6f7c97f4eedc56b39d741e8983b9c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2093445
Commit-Queue: Friedrich [CET] <fhorschig@chromium.org>
Reviewed-by: default avatarJan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753623}
parent d72c6ea2
...@@ -138,7 +138,7 @@ Polymer({ ...@@ -138,7 +138,7 @@ Polymer({
const statusChangeListener = status => this.status_ = status; const statusChangeListener = status => this.status_ = status;
const setLeakedCredentialsListener = compromisedCredentials => { const setLeakedCredentialsListener = compromisedCredentials => {
this.updateList(compromisedCredentials); this.updateList_(compromisedCredentials);
settings.PluralStringProxyImpl.getInstance() settings.PluralStringProxyImpl.getInstance()
.getPluralString('compromisedPasswords', this.leakedPasswords.length) .getPluralString('compromisedPasswords', this.leakedPasswords.length)
...@@ -224,12 +224,11 @@ Polymer({ ...@@ -224,12 +224,11 @@ Polymer({
/** /**
* Returns true if there are any compromised credentials. * Returns true if there are any compromised credentials.
* @param {!Array<!PasswordManagerProxy.CompromisedCredential>} list
* @return {boolean} * @return {boolean}
* @private * @private
*/ */
hasLeakedCredentials_(list) { hasLeakedCredentials_() {
return !!list.length; return !!this.leakedPasswords.length;
}, },
/** /**
...@@ -284,17 +283,14 @@ Polymer({ ...@@ -284,17 +283,14 @@ Polymer({
/** /**
* Returns the icon (warning, info or error) indicating the check status. * Returns the icon (warning, info or error) indicating the check status.
* @param {!PasswordManagerProxy.PasswordCheckStatus} status
* @param {!Array<!PasswordManagerProxy.CompromisedCredential>}
* leakedPasswords
* @return {string} * @return {string}
* @private * @private
*/ */
getStatusIcon_(status, leakedPasswords) { getStatusIcon_() {
if (!this.hasLeaksOrErrors_(status, leakedPasswords)) { if (!this.hasLeaksOrErrors_()) {
return 'settings:check-circle'; return 'settings:check-circle';
} }
if (this.hasLeakedCredentials_(leakedPasswords)) { if (this.hasLeakedCredentials_()) {
return 'cr:warning'; return 'cr:warning';
} }
return 'cr:info'; return 'cr:info';
...@@ -302,17 +298,14 @@ Polymer({ ...@@ -302,17 +298,14 @@ Polymer({
/** /**
* Returns the CSS class used to style the icon (warning, info or error). * Returns the CSS class used to style the icon (warning, info or error).
* @param {!PasswordManagerProxy.PasswordCheckStatus} status
* @param {!Array<!PasswordManagerProxy.CompromisedCredential>}
* leakedPasswords
* @return {string} * @return {string}
* @private * @private
*/ */
getStatusIconClass_(status, leakedPasswords) { getStatusIconClass_() {
if (!this.hasLeaksOrErrors_(status, leakedPasswords)) { if (!this.hasLeaksOrErrors_()) {
return this.waitsForFirstCheck_() ? 'hidden' : 'no-leaks'; return this.waitsForFirstCheck_() ? 'hidden' : 'no-leaks';
} }
if (this.hasLeakedCredentials_(leakedPasswords)) { if (this.hasLeakedCredentials_()) {
return 'has-leaks'; return 'has-leaks';
} }
return ''; return '';
...@@ -355,33 +348,30 @@ Polymer({ ...@@ -355,33 +348,30 @@ Polymer({
/** /**
* Returns true iff a check is running right according to the given |status|. * Returns true iff a check is running right according to the given |status|.
* @param {!PasswordManagerProxy.PasswordCheckStatus} status
* @return {boolean} * @return {boolean}
* @private * @private
*/ */
isCheckInProgress_(status) { isCheckInProgress_() {
return status.state == CheckState.RUNNING; return this.status_.state == CheckState.RUNNING;
}, },
/** /**
* Returns true to show the timestamp when a check was completed successfully. * Returns true to show the timestamp when a check was completed successfully.
* @param {!PasswordManagerProxy.PasswordCheckStatus} status
* @return {boolean} * @return {boolean}
* @private * @private
*/ */
showsTimestamp_(status) { showsTimestamp_() {
return status.state == CheckState.IDLE && return this.status_.state == CheckState.IDLE &&
!!status.elapsedTimeSinceLastCheck; !!this.status_.elapsedTimeSinceLastCheck;
}, },
/** /**
* Returns the button caption indicating it's current functionality. * Returns the button caption indicating it's current functionality.
* @param {!PasswordManagerProxy.PasswordCheckStatus} status
* @return {string} * @return {string}
* @private * @private
*/ */
getButtonText_(status) { getButtonText_() {
switch (status.state) { switch (this.status_.state) {
case CheckState.IDLE: case CheckState.IDLE:
return this.waitsForFirstCheck_() ? this.i18n('checkPasswords') : return this.waitsForFirstCheck_() ? this.i18n('checkPasswords') :
this.i18n('checkPasswordsAgain'); this.i18n('checkPasswordsAgain');
...@@ -397,7 +387,8 @@ Polymer({ ...@@ -397,7 +387,8 @@ Polymer({
case CheckState.QUOTA_LIMIT: case CheckState.QUOTA_LIMIT:
return ''; // Undefined behavior. Don't show any misleading text. return ''; // Undefined behavior. Don't show any misleading text.
} }
assertNotReached('Can\'t find a button text for state: ' + status.state); assertNotReached(
'Can\'t find a button text for state: ' + this.status_.state);
}, },
/** /**
...@@ -453,27 +444,24 @@ Polymer({ ...@@ -453,27 +444,24 @@ Polymer({
* @private * @private
*/ */
shouldShowBanner_() { shouldShowBanner_() {
if (this.hasLeakedCredentials_(this.leakedPasswords)) { if (this.hasLeakedCredentials_()) {
return false; return false;
} }
return this.status_.state == CheckState.CANCELED || return this.status_.state == CheckState.CANCELED ||
!this.hasLeaksOrErrors_(this.status_, this.leakedPasswords); !this.hasLeaksOrErrors_();
}, },
/** /**
* Returns true if there are leaked credentials or the status is unexpected * Returns true if there are leaked credentials or the status is unexpected
* for a regular password check. * for a regular password check.
* @param {!PasswordManagerProxy.PasswordCheckStatus} status
* @param {!Array<PasswordManagerProxy.CompromisedCredential>}
* leakedPasswords
* @return {boolean} * @return {boolean}
* @private * @private
*/ */
hasLeaksOrErrors_(status, leakedPasswords) { hasLeaksOrErrors_() {
if (this.hasLeakedCredentials_(leakedPasswords)) { if (this.hasLeakedCredentials_()) {
return true; return true;
} }
switch (status.state) { switch (this.status_.state) {
case CheckState.IDLE: case CheckState.IDLE:
case CheckState.RUNNING: case CheckState.RUNNING:
return false; return false;
...@@ -485,7 +473,8 @@ Polymer({ ...@@ -485,7 +473,8 @@ Polymer({
case CheckState.OTHER_ERROR: case CheckState.OTHER_ERROR:
return true; return true;
} }
throw 'Not specified whether to state is an error: ' + status.state; assertNotReached(
'Not specified whether to state is an error: ' + this.status_.state);
}, },
/** /**
...@@ -495,7 +484,7 @@ Polymer({ ...@@ -495,7 +484,7 @@ Polymer({
* @private * @private
*/ */
showsPasswordsCount_() { showsPasswordsCount_() {
if (this.hasLeakedCredentials_(this.leakedPasswords)) { if (this.hasLeakedCredentials_()) {
return true; return true;
} }
switch (this.status_.state) { switch (this.status_.state) {
...@@ -559,7 +548,7 @@ Polymer({ ...@@ -559,7 +548,7 @@ Polymer({
* @param {!Array<!PasswordManagerProxy.CompromisedCredential>} newList * @param {!Array<!PasswordManagerProxy.CompromisedCredential>} newList
* @private * @private
*/ */
updateList(newList) { updateList_(newList) {
const oldList = this.leakedPasswords.slice(); const oldList = this.leakedPasswords.slice();
const map = new Map(); const map = new Map();
newList.forEach(item => map.set(item.id, item)); newList.forEach(item => map.set(item.id, item));
...@@ -605,8 +594,7 @@ Polymer({ ...@@ -605,8 +594,7 @@ Polymer({
// Return true if there was a successful check and no compromised passwords // Return true if there was a successful check and no compromised passwords
// were found. // were found.
return !this.hasLeakedCredentials_(this.leakedPasswords) && return !this.hasLeakedCredentials_() && this.showsTimestamp_();
this.showsTimestamp_(this.status_);
}, },
}); });
})(); })();
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment