[Web Payment] Revert header view changes.

This reverts https://crrev.com/c/2351959, https://crrev.com/c/2345928,
https://crrev.com/c/2302970, and https://crrev.com/c/2257090 because
they are causing hard-to-diagnose crashes in production for a feature
that is supposed to be still turned off. The patches should be relanded
with more test coverage.

TBR=jdragon.bae@gmail.com, manukh@chromium.org, maxlg@chromium.org

Bug: 1114556, 1052493
Change-Id: Ic221c99e28246676d24bcd2185773657abef4781
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2354149
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797651}

chrome/browser/payments/ssl_validity_checker.cc

@@ -15,6 +15,21 @@
 #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(
@@ -82,17 +97,4 @@ 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,7 +8,6 @@
 #include <string>
 
 #include "base/macros.h"
-#include "components/security_state/core/security_state.h"
 
 namespace content {
 class WebContents;
@@ -34,9 +33,6 @@ 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

@@ -10,21 +10,14 @@
 #include "chrome/app/vector_icons/vector_icons.h"
 #include "chrome/browser/payments/ssl_validity_checker.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/themes/theme_service.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
-#include "chrome/browser/ui/omnibox/omnibox_theme.h"
 #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"
 #include "content/public/browser/navigation_handle.h"
@@ -36,7 +29,6 @@
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/image/image_skia.h"
-#include "ui/gfx/paint_vector_icon.h"
 #include "ui/views/background.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/image_view.h"
@@ -71,8 +63,6 @@ class ReadOnlyOriginView : public views::View {
   ReadOnlyOriginView(const base::string16& page_title,
                      const GURL& origin,
                      const SkBitmap* icon_bitmap,
-                     Profile* profile,
-                     security_state::SecurityLevel security_level,
                      SkColor background_color,
                      views::ButtonListener* site_settings_listener) {
     auto title_origin_container = std::make_unique<views::View>();
@@ -100,30 +90,8 @@ class ReadOnlyOriginView : public views::View {
       title_label->SetEnabledColor(foreground);
     }
 
-    auto origin_container = std::make_unique<views::View>();
-    views::GridLayout* origin_layout = origin_container->SetLayoutManager(
-        std::make_unique<views::GridLayout>());
-
-    columns = origin_layout->AddColumnSet(0);
-    columns->AddColumn(views::GridLayout::LEADING, views::GridLayout::CENTER,
-                       1.0, views::GridLayout::ColumnSize::kUsePreferred, 0, 0);
-    columns->AddColumn(views::GridLayout::LEADING, views::GridLayout::LEADING,
-                       1.0, views::GridLayout::ColumnSize::kUsePreferred, 0, 0);
-    origin_layout->StartRow(views::GridLayout::kFixedSize, 0);
-    if (PaymentsExperimentalFeatures::IsEnabled(
-            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 security_icon = std::make_unique<views::ImageView>();
-      const ui::ThemeProvider& theme_provider =
-          ThemeService::GetThemeProviderForProfile(profile);
-      security_icon->SetImage(gfx::CreateVectorIcon(
-          location_bar_model::GetSecurityVectorIcon(security_level), 16,
-          GetOmniboxSecurityChipColor(&theme_provider, security_level)));
-      origin_layout->AddView(std::move(security_icon));
-    }
-    auto* origin_label = origin_layout->AddView(
+    title_origin_layout->StartRow(views::GridLayout::kFixedSize, 0);
+    auto* origin_label = title_origin_layout->AddView(
         std::make_unique<views::Label>(base::UTF8ToUTF16(origin.host())));
     origin_label->SetElideBehavior(gfx::ELIDE_HEAD);
     if (!title_is_valid) {
@@ -140,8 +108,6 @@ class ReadOnlyOriginView : public views::View {
     origin_label->SetAutoColorReadabilityEnabled(false);
     origin_label->SetEnabledColor(foreground);
     origin_label->SetBackgroundColor(background_color);
-    title_origin_layout->StartRow(views::GridLayout::kFixedSize, 0);
-    title_origin_layout->AddView(std::move(origin_container));
 
     views::GridLayout* top_level_layout =
         SetLayoutManager(std::make_unique<views::GridLayout>());
@@ -272,10 +238,7 @@ PaymentHandlerWebFlowViewController::CreateHeaderContentView(
       GetHeaderBackground(header_view);
   return std::make_unique<ReadOnlyOriginView>(
       GetPaymentHandlerDialogTitle(web_contents()), origin,
-      state()->selected_app()->icon_bitmap(), profile_,
-      web_contents() ? SslValidityChecker::GetSecurityLevel(web_contents())
-                     : security_state::NONE,
-      background->get_color(), this);
+      state()->selected_app()->icon_bitmap(), background->get_color(), this);
 }
 
 std::unique_ptr<views::Background>
@@ -304,11 +267,8 @@ bool PaymentHandlerWebFlowViewController::
 void PaymentHandlerWebFlowViewController::VisibleSecurityStateChanged(
     content::WebContents* source) {
   DCHECK_EQ(source, web_contents());
-  if (!SslValidityChecker::IsValidPageInPaymentHandlerWindow(source)) {
+  if (!SslValidityChecker::IsValidPageInPaymentHandlerWindow(source))
     AbortPayment();
-  } else {
-    UpdateHeaderView();
-  }
 }
 
 void PaymentHandlerWebFlowViewController::DidStartNavigation(

components/omnibox/browser/BUILD.gn

@@ -275,8 +275,6 @@ 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,7 +15,6 @@
 #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"
@@ -30,6 +29,7 @@
 
 #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,9 +205,35 @@ const gfx::VectorIcon& LocationBarModelImpl::GetVectorIcon() const {
 
   if (IsOfflinePage())
     return omnibox::kOfflinePinIcon;
-#endif
 
-  return location_bar_model::GetSecurityVectorIcon(GetSecurityLevel());
+  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
 }
 
 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,9 +77,6 @@ const base::Feature kAllowJITInstallationWhenAppIconIsMissing{
     "AllowJITInstallationWhenAppIconIsMissing",
     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,11 +79,6 @@ 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 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 kPaymentHandlerSecurityIcon;
-
 // Used to reject the apps with partial delegation.
 extern const base::Feature kEnforceFullDelegation;