Unification of accessibility and secure chip logic.

This CL simplifies the logic that determines what text is displayed in
the HTTPS UI indicator. It explicitly merges the logic for determining
what text is visually displayed with that of the accessibility label
into one function.

Bug: 812447
Change-Id: I72125b81003223ecd00d290a6900533dfb424d10
Reviewed-on: https://chromium-review.googlesource.com/c/1331569
Commit-Queue: Joe DeBlasio <jdeblasio@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607456}

chrome/browser/ui/views/location_bar/location_icon_view.cc

@@ -96,13 +96,12 @@ void LocationIconView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
     return;
   }
 
-  security_state::SecurityLevel security_level =
-      delegate_->GetLocationBarModel()->GetSecurityLevel(false);
-  if (label()->text().empty() && (security_level == security_state::EV_SECURE ||
-                                  security_level == security_state::SECURE)) {
-    node_data->AddStringAttribute(
-        ax::mojom::StringAttribute::kDescription,
-        l10n_util::GetStringUTF8(IDS_SECURE_VERBOSE_STATE));
+  // If no display text exists, ensure that the accessibility label is added.
+  auto accessibility_label = base::UTF16ToUTF8(
+      delegate_->GetLocationBarModel()->GetSecureAccessibilityText());
+  if (label()->text().empty() && !accessibility_label.empty()) {
+    node_data->AddStringAttribute(ax::mojom::StringAttribute::kDescription,
+                                  accessibility_label);
   }
 
   IconLabelBubbleView::GetAccessibleNodeData(node_data);
@@ -141,7 +140,7 @@ bool LocationIconView::ShouldShowText() const {
       return true;
   }
 
-  return !location_bar_model->GetSecureVerboseText().empty();
+  return !location_bar_model->GetSecureDisplayText().empty();
 }
 
 base::string16 LocationIconView::GetText() const {
@@ -166,7 +165,7 @@ base::string16 LocationIconView::GetText() const {
       return extension_name;
   }
 
-  return delegate_->GetLocationBarModel()->GetSecureVerboseText();
+  return delegate_->GetLocationBarModel()->GetSecureDisplayText();
 }
 
 bool LocationIconView::ShouldAnimateTextVisibilityChange() const {

components/omnibox/browser/location_bar_model.h

@@ -60,7 +60,7 @@ class LocationBarModel {
 
   // Returns text for the omnibox secure verbose chip, displayed next to the
   // security icon on certain platforms.
-  virtual base::string16 GetSecureVerboseText() const = 0;
+  virtual base::string16 GetSecureDisplayText() const = 0;
 
   // Returns text describing the security state for accessibility.
   virtual base::string16 GetSecureAccessibilityText() const = 0;

components/omnibox/browser/location_bar_model_impl.cc

@@ -159,64 +159,66 @@ base::string16 LocationBarModelImpl::GetEVCertName() const {
       base::UTF8ToUTF16(cert->subject().country_name));
 }
 
-base::string16 LocationBarModelImpl::GetSecureText() const {
+LocationBarModelImpl::SecureChipText LocationBarModelImpl::GetSecureChipText()
+    const {
+  // Note that displayed text (the first output) will be implicitly used as the
+  // accessibility text unless no display text has been specified.
+
+  // Security UI study (https://crbug.com/803501): Change EV/Secure text.
+  const std::string securityUIStudyParam =
+      base::FeatureList::IsEnabled(omnibox::kSimplifyHttpsIndicator)
+          ? base::GetFieldTrialParamValueByFeature(
+                omnibox::kSimplifyHttpsIndicator,
+                OmniboxFieldTrial::kSimplifyHttpsIndicatorParameterName)
+          : std::string();
+
+  if (IsOfflinePage())
+    return SecureChipText(l10n_util::GetStringUTF16(IDS_OFFLINE_VERBOSE_STATE));
+
   switch (GetSecurityLevel(false)) {
     case security_state::HTTP_SHOW_WARNING:
-      return l10n_util::GetStringUTF16(IDS_NOT_SECURE_VERBOSE_STATE);
+      return SecureChipText(
+          l10n_util::GetStringUTF16(IDS_NOT_SECURE_VERBOSE_STATE));
     case security_state::EV_SECURE:
-      return GetEVCertName();
+      if (securityUIStudyParam ==
+          OmniboxFieldTrial::kSimplifyHttpsIndicatorParameterEvToSecure)
+        return SecureChipText(
+            l10n_util::GetStringUTF16(IDS_SECURE_VERBOSE_STATE));
+      if (securityUIStudyParam ==
+          OmniboxFieldTrial::kSimplifyHttpsIndicatorParameterBothToLock)
+        return SecureChipText(base::string16(), l10n_util::GetStringUTF16(
+                                                    IDS_SECURE_VERBOSE_STATE));
+      return SecureChipText(GetEVCertName());
     case security_state::SECURE:
-      return l10n_util::GetStringUTF16(IDS_SECURE_VERBOSE_STATE);
+      if (securityUIStudyParam !=
+          OmniboxFieldTrial::kSimplifyHttpsIndicatorParameterKeepSecureChip)
+        return SecureChipText(base::string16(), l10n_util::GetStringUTF16(
+                                                    IDS_SECURE_VERBOSE_STATE));
+      return SecureChipText(
+          l10n_util::GetStringUTF16(IDS_SECURE_VERBOSE_STATE));
     case security_state::DANGEROUS:
       if (delegate_->FailsMalwareCheck())
-        return l10n_util::GetStringUTF16(IDS_DANGEROUS_VERBOSE_STATE);
+        return SecureChipText(
+            l10n_util::GetStringUTF16(IDS_DANGEROUS_VERBOSE_STATE));
       // Don't show any text in the security indicator for sites on the billing
       // interstitial list.
-      return delegate_->FailsBillingCheck()
-                 ? base::string16()
-                 : l10n_util::GetStringUTF16(IDS_NOT_SECURE_VERBOSE_STATE);
+      return SecureChipText(
+          delegate_->FailsBillingCheck()
+              ? base::string16()
+              : l10n_util::GetStringUTF16(IDS_NOT_SECURE_VERBOSE_STATE));
     default:
-      return base::string16();
+      return SecureChipText(base::string16());
   }
 }
 
-base::string16 LocationBarModelImpl::GetSecureVerboseText() const {
-  if (IsOfflinePage())
-    return l10n_util::GetStringUTF16(IDS_OFFLINE_VERBOSE_STATE);
-
-  // Security UI study (https://crbug.com/803501): Change EV/Secure text.
-  const std::string parameter =
-      base::FeatureList::IsEnabled(omnibox::kSimplifyHttpsIndicator)
-          ? base::GetFieldTrialParamValueByFeature(
-                omnibox::kSimplifyHttpsIndicator,
-                OmniboxFieldTrial::kSimplifyHttpsIndicatorParameterName)
-          : std::string();
-
-  auto security_level = GetSecurityLevel(false);
-  if (security_level == security_state::EV_SECURE) {
-    if (parameter ==
-        OmniboxFieldTrial::kSimplifyHttpsIndicatorParameterEvToSecure) {
-      return l10n_util::GetStringUTF16(IDS_SECURE_VERBOSE_STATE);
-    }
-    if (parameter ==
-        OmniboxFieldTrial::kSimplifyHttpsIndicatorParameterBothToLock) {
-      return base::string16();
-    }
-  }
-  if (security_level == security_state::SECURE) {
-    if (parameter !=
-        OmniboxFieldTrial::kSimplifyHttpsIndicatorParameterKeepSecureChip) {
-      return base::string16();
-    }
-  }
-  return GetSecureText();
+base::string16 LocationBarModelImpl::GetSecureDisplayText() const {
+  return GetSecureChipText().display_text_;
 }
 
 base::string16 LocationBarModelImpl::GetSecureAccessibilityText() const {
-  if (IsOfflinePage())
-    return l10n_util::GetStringUTF16(IDS_OFFLINE_VERBOSE_STATE);
-
-  return GetSecureText();
+  auto labels = GetSecureChipText();
+  return labels.display_text_.empty() ? labels.accessibility_label_
+                                      : labels.display_text_;
 }
 
 bool LocationBarModelImpl::ShouldDisplayURL() const {

components/omnibox/browser/location_bar_model_impl.h

@@ -35,15 +35,27 @@ class LocationBarModelImpl : public LocationBarModel {
       bool ignore_editing) const override;
   bool IsSecurityInfoInitialized() const override;
   const gfx::VectorIcon& GetVectorIcon() const override;
-  base::string16 GetSecureVerboseText() const override;
+  base::string16 GetSecureDisplayText() const override;
   base::string16 GetSecureAccessibilityText() const override;
   base::string16 GetEVCertName() const override;
   bool ShouldDisplayURL() const override;
   bool IsOfflinePage() const override;
 
  private:
-  // Get the security text describing the current security state.
-  base::string16 GetSecureText() const;
+  struct SecureChipText {
+    base::string16 display_text_;
+    base::string16 accessibility_label_;
+    SecureChipText(base::string16 display_text,
+                   base::string16 accessibility_label)
+        : display_text_(display_text),
+          accessibility_label_(accessibility_label) {}
+    SecureChipText(base::string16 display_text)
+        : display_text_(display_text), accessibility_label_(display_text) {}
+  };
+
+  // Get the security chip labels for the current security state.
+  SecureChipText GetSecureChipText() const;
+
   base::string16 GetFormattedURL(
       url_formatter::FormatUrlTypes format_types) const;
 

components/omnibox/browser/test_location_bar_model.cc

@@ -51,7 +51,7 @@ const gfx::VectorIcon& TestLocationBarModel::GetVectorIcon() const {
   return *icon_;
 }
 
-base::string16 TestLocationBarModel::GetSecureVerboseText() const {
+base::string16 TestLocationBarModel::GetSecureDisplayText() const {
   return base::string16();
 }
 

components/omnibox/browser/test_location_bar_model.h

@@ -31,7 +31,7 @@ class TestLocationBarModel : public LocationBarModel {
       bool ignore_editing) const override;
   bool IsSecurityInfoInitialized() const override;
   const gfx::VectorIcon& GetVectorIcon() const override;
-  base::string16 GetSecureVerboseText() const override;
+  base::string16 GetSecureDisplayText() const override;
   base::string16 GetSecureAccessibilityText() const override;
   base::string16 GetEVCertName() const override;
   bool ShouldDisplayURL() const override;