[Web Payment] Selecting the security icon based on the SSL certificate

The security indicator icon in the payment handler UI should match
exactly the icon in the regular URL bar, for any given URL. Only if SSL
certificate state is secure, the security indicator icon would be
appear.

Change-Id: Ic15b0f848ecdf326e5ad63704a28cf8d19416fd2
Bug: 1052493
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2302970
Commit-Queue: Jaeyong Bae <jdragon.bae@gmail.com>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: manuk hovanesian <manukh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795938}

chrome/browser/payments/ssl_validity_checker.cc

@@ -15,21 +15,6 @@
 #include "url/gurl.h"
 
 namespace payments {
-namespace {
-
-// Returns the security level of |web_contents|. The |web_contents| parameter
-// should not be null.
-security_state::SecurityLevel GetSecurityLevel(
-    content::WebContents* web_contents) {
-  DCHECK(web_contents);
-  SecurityStateTabHelper::CreateForWebContents(web_contents);
-  SecurityStateTabHelper* helper =
-      SecurityStateTabHelper::FromWebContents(web_contents);
-  DCHECK(helper);
-  return helper->GetSecurityLevel();
-}
-
-}  // namespace
 
 // static std::string
 std::string SslValidityChecker::GetInvalidSslCertificateErrorMessage(
@@ -97,4 +82,17 @@ bool SslValidityChecker::IsValidPageInPaymentHandlerWindow(
   return true;
 }
 
+// static
+// Returns the security level of |web_contents|. The |web_contents|
+// parameter should not be null.
+security_state::SecurityLevel SslValidityChecker::GetSecurityLevel(
+    content::WebContents* web_contents) {
+  DCHECK(web_contents);
+  SecurityStateTabHelper::CreateForWebContents(web_contents);
+  SecurityStateTabHelper* helper =
+      SecurityStateTabHelper::FromWebContents(web_contents);
+  DCHECK(helper);
+  return helper->GetSecurityLevel();
+}
+
 }  // namespace payments

chrome/browser/payments/ssl_validity_checker.h

@@ -8,6 +8,7 @@
 #include <string>
 
 #include "base/macros.h"
+#include "components/security_state/core/security_state.h"
 
 namespace content {
 class WebContents;
@@ -33,6 +34,9 @@ class SslValidityChecker {
   static bool IsValidPageInPaymentHandlerWindow(
       content::WebContents* web_contents);
 
+  static security_state::SecurityLevel GetSecurityLevel(
+      content::WebContents* web_contents);
+
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(SslValidityChecker);
 };

chrome/browser/ui/views/payments/payment_handler_web_flow_view_controller.cc

@@ -15,11 +15,13 @@
 #include "chrome/browser/ui/views/payments/payment_request_dialog_view.h"
 #include "chrome/browser/ui/views/payments/payment_request_views_util.h"
 #include "chrome/grit/generated_resources.h"
+#include "components/omnibox/browser/location_bar_model_util.h"
 #include "components/payments/content/icon/icon_size.h"
 #include "components/payments/core/features.h"
 #include "components/payments/core/native_error_strings.h"
 #include "components/payments/core/payments_experimental_features.h"
 #include "components/payments/core/url_util.h"
+#include "components/security_state/core/security_state.h"
 #include "components/vector_icons/vector_icons.h"
 #include "components/web_modal/web_contents_modal_dialog_manager.h"
 #include "components/web_modal/web_contents_modal_dialog_manager_delegate.h"
@@ -67,6 +69,7 @@ class ReadOnlyOriginView : public views::View {
   ReadOnlyOriginView(const base::string16& page_title,
                      const GURL& origin,
                      const SkBitmap* icon_bitmap,
+                     security_state::SecurityLevel security_level,
                      SkColor background_color,
                      views::ButtonListener* site_settings_listener) {
     auto title_origin_container = std::make_unique<views::View>();
@@ -105,15 +108,15 @@ class ReadOnlyOriginView : public views::View {
                        1.0, views::GridLayout::ColumnSize::kUsePreferred, 0, 0);
     origin_layout->StartRow(views::GridLayout::kFixedSize, 0);
     if (PaymentsExperimentalFeatures::IsEnabled(
-            features::kPaymentHandlerLockIcon) &&
-        origin.SchemeIs(url::kHttpsScheme)) {
+            features::kPaymentHandlerSecurityIcon)) {
       // TODO(https://crbug.com/1052493):
       // Selecting the correct icon based on the SSL certificate state
       // and adding test coverage for this code path.
-      auto lock_icon = std::make_unique<views::ImageView>();
-      lock_icon->SetImage(gfx::CreateVectorIcon(vector_icons::kLockIcon, 16,
-                                                gfx::kChromeIconGrey));
-      origin_layout->AddView(std::move(lock_icon));
+      auto security_icon = std::make_unique<views::ImageView>();
+      security_icon->SetImage(gfx::CreateVectorIcon(
+          location_bar_model::GetSecurityVectorIcon(security_level), 16,
+          gfx::kChromeIconGrey));
+      origin_layout->AddView(std::move(security_icon));
     }
     auto* origin_label = origin_layout->AddView(
         std::make_unique<views::Label>(base::UTF8ToUTF16(origin.host())));
@@ -264,7 +267,10 @@ PaymentHandlerWebFlowViewController::CreateHeaderContentView(
       GetHeaderBackground(header_view);
   return std::make_unique<ReadOnlyOriginView>(
       GetPaymentHandlerDialogTitle(web_contents()), origin,
-      state()->selected_app()->icon_bitmap(), background->get_color(), this);
+      state()->selected_app()->icon_bitmap(),
+      web_contents() ? SslValidityChecker::GetSecurityLevel(web_contents())
+                     : security_state::NONE,
+      background->get_color(), this);
 }
 
 std::unique_ptr<views::Background>
@@ -293,6 +299,7 @@ bool PaymentHandlerWebFlowViewController::
 void PaymentHandlerWebFlowViewController::VisibleSecurityStateChanged(
     content::WebContents* source) {
   DCHECK_EQ(source, web_contents());
+  UpdateHeaderView();
   if (!SslValidityChecker::IsValidPageInPaymentHandlerWindow(source))
     AbortPayment();
 }

components/omnibox/browser/BUILD.gn

@@ -276,6 +276,8 @@ static_library("location_bar") {
     "location_bar_model_delegate.h",
     "location_bar_model_impl.cc",
     "location_bar_model_impl.h",
+    "location_bar_model_util.cc",
+    "location_bar_model_util.h",
   ]
 
   public_deps = [

components/omnibox/browser/location_bar_model_impl.cc

@@ -15,6 +15,7 @@
 #include "components/dom_distiller/core/url_utils.h"
 #include "components/omnibox/browser/buildflags.h"
 #include "components/omnibox/browser/location_bar_model_delegate.h"
+#include "components/omnibox/browser/location_bar_model_util.h"
 #include "components/omnibox/common/omnibox_features.h"
 #include "components/search_engines/template_url_service.h"
 #include "components/security_state/core/security_state.h"
@@ -29,7 +30,6 @@
 
 #if (!defined(OS_ANDROID) || BUILDFLAG(ENABLE_VR)) && !defined(OS_IOS)
 #include "components/omnibox/browser/vector_icons.h"  // nogncheck
-#include "components/vector_icons/vector_icons.h"     // nogncheck
 #endif
 
 using metrics::OmniboxEventProto;
@@ -205,35 +205,9 @@ const gfx::VectorIcon& LocationBarModelImpl::GetVectorIcon() const {
 
   if (IsOfflinePage())
     return omnibox::kOfflinePinIcon;
-
-  security_state::SecurityLevel security_level = GetSecurityLevel();
-  switch (security_level) {
-    case security_state::NONE:
-      return omnibox::kHttpIcon;
-    case security_state::WARNING:
-      // When kMarkHttpAsParameterDangerWarning is enabled, show a danger
-      // triangle icon.
-      if (security_state::ShouldShowDangerTriangleForWarningLevel()) {
-        return omnibox::kNotSecureWarningIcon;
-      }
-      return omnibox::kHttpIcon;
-    case security_state::SECURE:
-      return omnibox::kHttpsValidIcon;
-    case security_state::SECURE_WITH_POLICY_INSTALLED_CERT:
-      return vector_icons::kBusinessIcon;
-    case security_state::DANGEROUS:
-      return omnibox::kNotSecureWarningIcon;
-    case security_state::SECURITY_LEVEL_COUNT:
-      NOTREACHED();
-      return omnibox::kHttpIcon;
-  }
-  NOTREACHED();
-  return omnibox::kHttpIcon;
-#else
-  NOTREACHED();
-  static const gfx::VectorIcon dummy = {};
-  return dummy;
 #endif
+
+  return location_bar_model::GetSecurityVectorIcon(GetSecurityLevel());
 }
 
 base::string16 LocationBarModelImpl::GetSecureDisplayText() const {

components/omnibox/browser/location_bar_model_util.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/omnibox/browser/location_bar_model_util.h"
+
+#include "build/build_config.h"
+#include "components/omnibox/browser/buildflags.h"
+#include "ui/gfx/vector_icon_types.h"
+
+#if (!defined(OS_ANDROID) || BUILDFLAG(ENABLE_VR)) && !defined(OS_IOS)
+#include "components/omnibox/browser/vector_icons.h"  // nogncheck
+#include "components/vector_icons/vector_icons.h"     // nogncheck
+#endif
+
+namespace location_bar_model {
+
+const gfx::VectorIcon& GetSecurityVectorIcon(
+    security_state::SecurityLevel security_level) {
+#if (!defined(OS_ANDROID) || BUILDFLAG(ENABLE_VR)) && !defined(OS_IOS)
+  switch (security_level) {
+    case security_state::NONE:
+      return omnibox::kHttpIcon;
+    case security_state::WARNING:
+      // When kMarkHttpAsParameterDangerWarning is enabled, show a danger
+      // triangle icon.
+      if (security_state::ShouldShowDangerTriangleForWarningLevel()) {
+        return omnibox::kNotSecureWarningIcon;
+      }
+      return omnibox::kHttpIcon;
+    case security_state::SECURE:
+      return omnibox::kHttpsValidIcon;
+    case security_state::SECURE_WITH_POLICY_INSTALLED_CERT:
+      return vector_icons::kBusinessIcon;
+    case security_state::DANGEROUS:
+      return omnibox::kNotSecureWarningIcon;
+    case security_state::SECURITY_LEVEL_COUNT:
+      NOTREACHED();
+      return omnibox::kHttpIcon;
+  }
+  NOTREACHED();
+  return omnibox::kHttpIcon;
+#else
+  NOTREACHED();
+  static const gfx::VectorIcon dummy = {};
+  return dummy;
+#endif
+}
+
+}  // namespace location_bar_model

components/omnibox/browser/location_bar_model_util.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_OMNIBOX_BROWSER_LOCATION_BAR_MODEL_UTIL_H_
+#define COMPONENTS_OMNIBOX_BROWSER_LOCATION_BAR_MODEL_UTIL_H_
+
+#include "components/security_state/core/security_state.h"
+
+namespace gfx {
+struct VectorIcon;
+}  // namespace gfx
+
+namespace location_bar_model {
+
+// Get the vector icon according to security level.
+// It indicates security state of the page.
+const gfx::VectorIcon& GetSecurityVectorIcon(
+    security_state::SecurityLevel security_level);
+
+}  // namespace location_bar_model
+
+#endif  // COMPONENTS_OMNIBOX_BROWSER_LOCATION_BAR_MODEL_UTIL_H_

components/payments/core/features.cc

@@ -77,8 +77,8 @@ const base::Feature kAllowJITInstallationWhenAppIconIsMissing{
     "AllowJITInstallationWhenAppIconIsMissing",
     base::FEATURE_DISABLED_BY_DEFAULT};
 
-const base::Feature kPaymentHandlerLockIcon{"PaymentHandlerLockIcon",
-                                            base::FEATURE_DISABLED_BY_DEFAULT};
+const base::Feature kPaymentHandlerSecurityIcon{
+    "PaymentHandlerSecurityIcon", base::FEATURE_DISABLED_BY_DEFAULT};
 
 const base::Feature kEnforceFullDelegation{"EnforceFullDelegation",
                                            base::FEATURE_DISABLED_BY_DEFAULT};

components/payments/core/features.h

@@ -79,10 +79,10 @@ extern const base::Feature kPaymentHandlerPopUpSizeWindow;
 // Used to test icon refetch for JIT installed apps with missing icons.
 extern const base::Feature kAllowJITInstallationWhenAppIconIsMissing;
 
-// Desktop only, if enabled the lock icon would be showed next to the
+// Desktop only, if enabled the security icon would be showed next to the
 // payment handler's URL bar. It indicate that only secure content is
 // allowed inside the payment handler.
-extern const base::Feature kPaymentHandlerLockIcon;
+extern const base::Feature kPaymentHandlerSecurityIcon;
 
 // Used to reject the apps with partial delegation.
 extern const base::Feature kEnforceFullDelegation;