Omnibox: Expose IsSecurityInfoInitialized() method in LocationBarModel

For Query in Omnibox, we want to avoid the URL flicker while the page
is navigating and the TLS security state has not been initialized.

This currently works on Android implementation by toggling a boolean
flag on navigation and TLS state update.

However, this duplicates a piece of state that's already
authoritatively stored within the VisibleSecurityState.

This CL:

 1. Exposes an IsSecurityInfoInitialized() method on LocationBarModel

 2. Updates the security_state::SecurityInfo struct to add a
    connection_info_initialized flag that's already present in
    security_state::VisibleSecurityState.

 3. Updates LocationBarModelDelegate to provide a general
    GetSecurityInfo method.

Planned followup work:

 1. Make Query in Omnibox actually use the IsSecurityInfoInitialized
    flag.

 2. Use LocationBarModelDelegate::GetSecurityInfo to remove some
    now-redundant methods such as FailsBillingCheck and
    FailsMalwareCheck.

Bug: 874592
Change-Id: I648b30859ec40bb741de1c31378e0a6cd8baeeaa
Reviewed-on: https://chromium-review.googlesource.com/c/1324879Reviewed-by: Adrienne Porter Felt <felt@chromium.org>
Reviewed-by: Christopher Thompson <cthomp@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607296}

chrome/browser/ui/toolbar/chrome_location_bar_model_delegate.cc

@@ -91,17 +91,17 @@ bool ChromeLocationBarModelDelegate::ShouldDisplayURL() const {
   return !profile || !search::IsInstantNTPURL(url, profile);
 }
 
-security_state::SecurityLevel ChromeLocationBarModelDelegate::GetSecurityLevel()
-    const {
+void ChromeLocationBarModelDelegate::GetSecurityInfo(
+    security_state::SecurityInfo* result) const {
   content::WebContents* web_contents = GetActiveWebContents();
   // If there is no active WebContents (which can happen during toolbar
   // initialization), assume no security style.
-  if (!web_contents)
-    return security_state::NONE;
+  if (!web_contents) {
+    *result = security_state::SecurityInfo();
+    return;
+  }
   auto* helper = SecurityStateTabHelper::FromWebContents(web_contents);
-  security_state::SecurityInfo security_info;
-  helper->GetSecurityInfo(&security_info);
-  return security_info.security_level;
+  helper->GetSecurityInfo(result);
 }
 
 scoped_refptr<net::X509Certificate>

chrome/browser/ui/toolbar/chrome_location_bar_model_delegate.h

@@ -38,7 +38,7 @@ class ChromeLocationBarModelDelegate : public LocationBarModelDelegate {
       const GURL& url,
       const base::string16& formatted_url) const override;
   bool GetURL(GURL* url) const override;
-  SecurityLevel GetSecurityLevel() const override;
+  void GetSecurityInfo(security_state::SecurityInfo* result) const override;
   scoped_refptr<net::X509Certificate> GetCertificate() const override;
   bool FailsBillingCheck() const override;
   bool FailsMalwareCheck() const override;

components/omnibox/browser/location_bar_model.h

@@ -48,6 +48,10 @@ class LocationBarModel {
   virtual security_state::SecurityLevel GetSecurityLevel(
       bool ignore_editing) const = 0;
 
+  // Returns whether the connection security fields have been initialized.
+  // After a navigation, this is false until the TLS state is updated.
+  virtual bool IsSecurityInfoInitialized() const = 0;
+
   // Returns the id of the icon to show to the left of the address, based on the
   // current URL.  When search term replacement is active, this returns a search
   // icon.  This doesn't cover specialized icons while the user is editing; see

components/omnibox/browser/location_bar_model_delegate.cc

@@ -8,9 +8,9 @@ bool LocationBarModelDelegate::ShouldDisplayURL() const {
   return true;
 }
 
-LocationBarModelDelegate::SecurityLevel
-LocationBarModelDelegate::GetSecurityLevel() const {
-  return SecurityLevel::NONE;
+void LocationBarModelDelegate::GetSecurityInfo(
+    security_state::SecurityInfo* result) const {
+  return;
 }
 
 scoped_refptr<net::X509Certificate> LocationBarModelDelegate::GetCertificate()

components/omnibox/browser/location_bar_model_delegate.h

@@ -24,8 +24,6 @@ class X509Certificate;
 // Delegate which is used by LocationBarModel class.
 class LocationBarModelDelegate {
  public:
-  using SecurityLevel = security_state::SecurityLevel;
-
   // Formats |url| using AutocompleteInput::FormattedStringWithEquivalentMeaning
   // providing an appropriate AutocompleteSchemeClassifier for the embedder.
   virtual base::string16 FormattedStringWithEquivalentMeaning(
@@ -40,9 +38,9 @@ class LocationBarModelDelegate {
   // in the location bar.
   virtual bool ShouldDisplayURL() const;
 
-  // Returns the underlying security level of the page without regard to any
+  // Returns the underlying security info of the page without regard to any
   // user edits that may be in progress.
-  virtual SecurityLevel GetSecurityLevel() const;
+  virtual void GetSecurityInfo(security_state::SecurityInfo* result) const;
 
   // Returns the certificate for the current navigation entry.
   virtual scoped_refptr<net::X509Certificate> GetCertificate() const;

components/omnibox/browser/location_bar_model_impl.cc

@@ -95,9 +95,18 @@ GURL LocationBarModelImpl::GetURL() const {
 security_state::SecurityLevel LocationBarModelImpl::GetSecurityLevel(
     bool ignore_editing) const {
   // When editing or empty, assume no security style.
-  return ((input_in_progress() && !ignore_editing) || !ShouldDisplayURL())
-             ? security_state::NONE
-             : delegate_->GetSecurityLevel();
+  if ((input_in_progress() && !ignore_editing) || !ShouldDisplayURL())
+    return security_state::NONE;
+
+  security_state::SecurityInfo info;
+  delegate_->GetSecurityInfo(&info);
+  return info.security_level;
+}
+
+bool LocationBarModelImpl::IsSecurityInfoInitialized() const {
+  security_state::SecurityInfo info;
+  delegate_->GetSecurityInfo(&info);
+  return info.connection_info_initialized;
 }
 
 const gfx::VectorIcon& LocationBarModelImpl::GetVectorIcon() const {

components/omnibox/browser/location_bar_model_impl.h

@@ -33,6 +33,7 @@ class LocationBarModelImpl : public LocationBarModel {
   GURL GetURL() const override;
   security_state::SecurityLevel GetSecurityLevel(
       bool ignore_editing) const override;
+  bool IsSecurityInfoInitialized() const override;
   const gfx::VectorIcon& GetVectorIcon() const override;
   base::string16 GetSecureVerboseText() const override;
   base::string16 GetSecureAccessibilityText() const override;

components/omnibox/browser/test_location_bar_model.cc

@@ -43,6 +43,10 @@ security_state::SecurityLevel TestLocationBarModel::GetSecurityLevel(
   return security_level_;
 }
 
+bool TestLocationBarModel::IsSecurityInfoInitialized() const {
+  return true;
+}
+
 const gfx::VectorIcon& TestLocationBarModel::GetVectorIcon() const {
   return *icon_;
 }

components/omnibox/browser/test_location_bar_model.h

@@ -29,6 +29,7 @@ class TestLocationBarModel : public LocationBarModel {
   GURL GetURL() const override;
   security_state::SecurityLevel GetSecurityLevel(
       bool ignore_editing) const override;
+  bool IsSecurityInfoInitialized() const override;
   const gfx::VectorIcon& GetVectorIcon() const override;
   base::string16 GetSecureVerboseText() const override;
   base::string16 GetSecureAccessibilityText() const override;

components/security_state/core/security_state.cc

@@ -233,6 +233,7 @@ void SecurityInfoForRequest(
     SecurityInfo* security_info) {
   if (!visible_security_state.connection_info_initialized) {
     *security_info = SecurityInfo();
+    security_info->connection_info_initialized = false;
     security_info->malicious_content_status =
         visible_security_state.malicious_content_status;
     if (security_info->malicious_content_status !=
@@ -244,6 +245,7 @@ void SecurityInfoForRequest(
     }
     return;
   }
+  security_info->connection_info_initialized = true;
   security_info->certificate = visible_security_state.certificate;
 
   security_info->sha1_in_chain = visible_security_state.certificate &&
@@ -311,7 +313,8 @@ std::string GetHistogramSuffixForSecurityLevel(
 }  // namespace
 
 SecurityInfo::SecurityInfo()
-    : security_level(NONE),
+    : connection_info_initialized(false),
+      security_level(NONE),
       malicious_content_status(MALICIOUS_CONTENT_STATUS_NONE),
       sha1_in_chain(false),
       mixed_content_status(CONTENT_STATUS_NONE),

components/security_state/core/security_state.h

@@ -104,6 +104,9 @@ enum MaliciousContentStatus {
 struct SecurityInfo {
   SecurityInfo();
   ~SecurityInfo();
+  // Whether the connection security fields are initialized.
+  bool connection_info_initialized;
+  // Describes the overall security state of the page.
   SecurityLevel security_level;
   // Describes the nature of the page's malicious content, if any.
   MaliciousContentStatus malicious_content_status;

ios/chrome/browser/ui/location_bar/location_bar_model_delegate_ios.h

@@ -29,7 +29,7 @@ class LocationBarModelDelegateIOS : public LocationBarModelDelegate {
       const base::string16& formatted_url) const override;
   bool GetURL(GURL* url) const override;
   bool ShouldDisplayURL() const override;
-  SecurityLevel GetSecurityLevel() const override;
+  void GetSecurityInfo(security_state::SecurityInfo* result) const override;
   scoped_refptr<net::X509Certificate> GetCertificate() const override;
   bool FailsMalwareCheck() const override;
   const gfx::VectorIcon* GetVectorIconOverride() const override;

ios/chrome/browser/ui/location_bar/location_bar_model_delegate_ios.mm

@@ -74,17 +74,17 @@ bool LocationBarModelDelegateIOS::ShouldDisplayURL() const {
   return true;
 }
 
-security_state::SecurityLevel LocationBarModelDelegateIOS::GetSecurityLevel()
-    const {
+void LocationBarModelDelegateIOS::GetSecurityInfo(
+    security_state::SecurityInfo* result) const {
   web::WebState* web_state = GetActiveWebState();
   // If there is no active WebState (which can happen during toolbar
   // initialization), assume no security style.
-  if (!web_state)
-    return security_state::NONE;
+  if (!web_state) {
+    *result = security_state::SecurityInfo();
+    return;
+  }
   auto* client = IOSSecurityStateTabHelper::FromWebState(web_state);
-  security_state::SecurityInfo result;
-  client->GetSecurityInfo(&result);
-  return result.security_level;
+  client->GetSecurityInfo(result);
 }
 
 scoped_refptr<net::X509Certificate>