From 8801c1d6a3b3ec8043278af5fb95e1cf16bd5c1e Mon Sep 17 00:00:00 2001 From: blundell Date: Fri, 31 Jul 2015 02:10:27 -0700 Subject: [PATCH] Split //chrome-specific parts of ToolbarModel into ChromeToolbarModel The ToolbarModel interface is used in omnibox code shared with iOS and thus needs to be componentized. However, it has one //chrome-level method (GetSecurityLevel()). As a precusor to componentization of ToolbarModel, this CL moves that method into a ChromeToolbarController subclass. BUG=511938 Review URL: https://codereview.chromium.org/1264633002 Cr-Commit-Position: refs/heads/master@{#341302} --- .../ui/cocoa/location_bar/location_bar_view_mac.mm | 6 ++-- .../browser/ui/cocoa/omnibox/omnibox_view_mac.mm | 6 ++-- chrome/browser/ui/toolbar/chrome_toolbar_model.cc | 9 ++++++ chrome/browser/ui/toolbar/chrome_toolbar_model.h | 33 ++++++++++++++++++++++ chrome/browser/ui/toolbar/test_toolbar_model.cc | 10 ++----- chrome/browser/ui/toolbar/test_toolbar_model.h | 6 ++-- chrome/browser/ui/toolbar/toolbar_model.h | 13 --------- chrome/browser/ui/toolbar/toolbar_model_impl.h | 10 ++++--- .../ui/views/location_bar/location_bar_view.cc | 4 ++- .../ui/views/location_bar/location_bar_view.h | 2 +- .../browser/ui/views/omnibox/omnibox_view_views.cc | 11 ++++++-- .../browser/ui/views/omnibox/omnibox_view_views.h | 5 +++- chrome/chrome_browser_ui.gypi | 2 ++ 13 files changed, 78 insertions(+), 39 deletions(-) create mode 100644 chrome/browser/ui/toolbar/chrome_toolbar_model.cc create mode 100644 chrome/browser/ui/toolbar/chrome_toolbar_model.h diff --git a/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm b/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm index f36ad33bae17..864b405e68c7 100644 --- a/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm +++ b/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm @@ -52,7 +52,7 @@ #import "chrome/browser/ui/omnibox/omnibox_popup_model.h" #include "chrome/browser/ui/passwords/manage_passwords_ui_controller.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" -#include "chrome/browser/ui/toolbar/toolbar_model.h" +#include "chrome/browser/ui/toolbar/chrome_toolbar_model.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/pref_names.h" #include "components/search_engines/template_url.h" @@ -427,13 +427,15 @@ void LocationBarViewMac::Layout() { } const bool is_keyword_hint = omnibox_view_->model()->is_keyword_hint(); + ChromeToolbarModel* chrome_toolbar_model = + static_cast(GetToolbarModel()); if (!keyword.empty() && !is_keyword_hint) { // Switch from location icon to keyword mode. location_icon_decoration_->SetVisible(false); selected_keyword_decoration_->SetVisible(true); selected_keyword_decoration_->SetKeyword(short_name, is_extension_keyword); selected_keyword_decoration_->SetImage(GetKeywordImage(keyword)); - } else if (GetToolbarModel()->GetSecurityLevel(false) == + } else if (chrome_toolbar_model->GetSecurityLevel(false) == connection_security::EV_SECURE) { // Switch from location icon to show the EV bubble instead. location_icon_decoration_->SetVisible(false); diff --git a/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm b/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm index 480ed54d06ba..b4c5e9f2b715 100644 --- a/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm +++ b/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm @@ -20,7 +20,7 @@ #include "chrome/browser/ui/omnibox/chrome_omnibox_client.h" #include "chrome/browser/ui/omnibox/omnibox_edit_controller.h" #include "chrome/browser/ui/omnibox/omnibox_popup_model.h" -#include "chrome/browser/ui/toolbar/toolbar_model.h" +#include "chrome/browser/ui/toolbar/chrome_toolbar_model.h" #include "chrome/grit/generated_resources.h" #include "components/omnibox/browser/autocomplete_input.h" #include "components/omnibox/browser/autocomplete_match.h" @@ -539,11 +539,13 @@ void OmniboxViewMac::ApplyTextAttributes( } } + ChromeToolbarModel* chrome_toolbar_model = + static_cast(controller()->GetToolbarModel()); // TODO(shess): GTK has this as a member var, figure out why. // [Could it be to not change if no change? If so, I'm guessing // AppKit may already handle that.] const connection_security::SecurityLevel security_level = - controller()->GetToolbarModel()->GetSecurityLevel(false); + chrome_toolbar_model->GetSecurityLevel(false); // Emphasize the scheme for security UI display purposes (if necessary). if (!model()->user_input_in_progress() && model()->CurrentTextIsURL() && diff --git a/chrome/browser/ui/toolbar/chrome_toolbar_model.cc b/chrome/browser/ui/toolbar/chrome_toolbar_model.cc new file mode 100644 index 000000000000..1902fecb0fe9 --- /dev/null +++ b/chrome/browser/ui/toolbar/chrome_toolbar_model.cc @@ -0,0 +1,9 @@ +// Copyright 2015 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 "chrome/browser/ui/toolbar/chrome_toolbar_model.h" + +ChromeToolbarModel::ChromeToolbarModel() {} + +ChromeToolbarModel::~ChromeToolbarModel() {} diff --git a/chrome/browser/ui/toolbar/chrome_toolbar_model.h b/chrome/browser/ui/toolbar/chrome_toolbar_model.h new file mode 100644 index 000000000000..736ac57d6d49 --- /dev/null +++ b/chrome/browser/ui/toolbar/chrome_toolbar_model.h @@ -0,0 +1,33 @@ +// Copyright 2015 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 CHROME_BROWSER_UI_TOOLBAR_CHROME_TOOLBAR_MODEL_H_ +#define CHROME_BROWSER_UI_TOOLBAR_CHROME_TOOLBAR_MODEL_H_ + +#include "chrome/browser/ssl/connection_security.h" +#include "chrome/browser/ui/toolbar/toolbar_model.h" + +// This class is a //chrome-specific extension of the ToolbarModel interface. +// TODO(blundell): If connection_security::SecurityLevel gets componentized, +// GetSecurityLevel() can be folded into ToolbarModel and this class can go +// away. crbug.com/515071 +class ChromeToolbarModel : public ToolbarModel { + public: + ~ChromeToolbarModel() override; + + // Returns the security level that the toolbar should display. If + // |ignore_editing| is true, the result reflects the underlying state of the + // page without regard to any user edits that may be in progress in the + // omnibox. + virtual connection_security::SecurityLevel GetSecurityLevel( + bool ignore_editing) const = 0; + + protected: + ChromeToolbarModel(); + + private: + DISALLOW_COPY_AND_ASSIGN(ChromeToolbarModel); +}; + +#endif // CHROME_BROWSER_UI_TOOLBAR_CHROME_TOOLBAR_MODEL_H_ diff --git a/chrome/browser/ui/toolbar/test_toolbar_model.cc b/chrome/browser/ui/toolbar/test_toolbar_model.cc index 68887fa8addb..9803a78ffd03 100644 --- a/chrome/browser/ui/toolbar/test_toolbar_model.cc +++ b/chrome/browser/ui/toolbar/test_toolbar_model.cc @@ -7,12 +7,11 @@ #include "grit/theme_resources.h" TestToolbarModel::TestToolbarModel() - : ToolbarModel(), + : ChromeToolbarModel(), perform_search_term_replacement_(false), security_level_(connection_security::NONE), icon_(IDR_LOCATION_BAR_HTTP), - should_display_url_(true) { -} + should_display_url_(true) {} TestToolbarModel::~TestToolbarModel() {} @@ -46,11 +45,6 @@ int TestToolbarModel::GetIcon() const { return icon_; } -int TestToolbarModel::GetIconForSecurityLevel( - connection_security::SecurityLevel level) const { - return icon_; -} - base::string16 TestToolbarModel::GetEVCertName() const { return (security_level_ == connection_security::EV_SECURE) ? ev_cert_name_ : base::string16(); diff --git a/chrome/browser/ui/toolbar/test_toolbar_model.h b/chrome/browser/ui/toolbar/test_toolbar_model.h index 2d676268e57d..0e41c02ef8fa 100644 --- a/chrome/browser/ui/toolbar/test_toolbar_model.h +++ b/chrome/browser/ui/toolbar/test_toolbar_model.h @@ -7,12 +7,12 @@ #include "base/compiler_specific.h" #include "base/strings/string16.h" -#include "chrome/browser/ui/toolbar/toolbar_model.h" +#include "chrome/browser/ui/toolbar/chrome_toolbar_model.h" // A ToolbarModel that is backed by instance variables, which are initialized // with some basic values that can be changed with the provided setters. This // should be used only for testing. -class TestToolbarModel : public ToolbarModel { +class TestToolbarModel : public ChromeToolbarModel { public: TestToolbarModel(); ~TestToolbarModel() override; @@ -24,8 +24,6 @@ class TestToolbarModel : public ToolbarModel { connection_security::SecurityLevel GetSecurityLevel( bool ignore_editing) const override; int GetIcon() const override; - int GetIconForSecurityLevel( - connection_security::SecurityLevel level) const override; base::string16 GetEVCertName() const override; bool ShouldDisplayURL() const override; diff --git a/chrome/browser/ui/toolbar/toolbar_model.h b/chrome/browser/ui/toolbar/toolbar_model.h index 582c65a1e595..18060007cdcd 100644 --- a/chrome/browser/ui/toolbar/toolbar_model.h +++ b/chrome/browser/ui/toolbar/toolbar_model.h @@ -9,7 +9,6 @@ #include "base/basictypes.h" #include "base/strings/string16.h" -#include "chrome/browser/ssl/connection_security.h" #include "url/gurl.h" namespace net { @@ -59,24 +58,12 @@ class ToolbarModel { // URL because of search term replacement. bool WouldReplaceURL() const; - // Returns the security level that the toolbar should display. If - // |ignore_editing| is true, the result reflects the underlying state of the - // page without regard to any user edits that may be in progress in the - // omnibox. - virtual connection_security::SecurityLevel GetSecurityLevel( - bool ignore_editing) const = 0; - // Returns the resource_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 OmniboxView::GetIcon(). virtual int GetIcon() const = 0; - // As |GetIcon()|, but returns the icon only taking into account the security - // |level| given, ignoring search term replacement state. - virtual int GetIconForSecurityLevel( - connection_security::SecurityLevel level) const = 0; - // Returns the name of the EV cert holder. This returns an empty string if // the security level is not EV_SECURE. virtual base::string16 GetEVCertName() const = 0; diff --git a/chrome/browser/ui/toolbar/toolbar_model_impl.h b/chrome/browser/ui/toolbar/toolbar_model_impl.h index b663240854d0..3c6396fb2871 100644 --- a/chrome/browser/ui/toolbar/toolbar_model_impl.h +++ b/chrome/browser/ui/toolbar/toolbar_model_impl.h @@ -10,7 +10,7 @@ #include "base/basictypes.h" #include "base/compiler_specific.h" #include "base/strings/string16.h" -#include "chrome/browser/ui/toolbar/toolbar_model.h" +#include "chrome/browser/ui/toolbar/chrome_toolbar_model.h" #include "url/gurl.h" class Profile; @@ -28,7 +28,7 @@ class X509Certificate; // This class is the model used by the toolbar, location bar and autocomplete // edit. It populates its states from the current navigation entry retrieved // from the navigation controller returned by GetNavigationController(). -class ToolbarModelImpl : public ToolbarModel { +class ToolbarModelImpl : public ChromeToolbarModel { public: explicit ToolbarModelImpl(ToolbarModelDelegate* delegate); ~ToolbarModelImpl() override; @@ -43,11 +43,13 @@ class ToolbarModelImpl : public ToolbarModel { connection_security::SecurityLevel GetSecurityLevel( bool ignore_editing) const override; int GetIcon() const override; - int GetIconForSecurityLevel( - connection_security::SecurityLevel level) const override; base::string16 GetEVCertName() const override; bool ShouldDisplayURL() const override; + // As |GetIcon()|, but returns the icon only taking into account the security + // level| given, ignoring search term replacement state. + int GetIconForSecurityLevel(connection_security::SecurityLevel level) const; + // Returns the navigation controller used to retrieve the navigation entry // from which the states are retrieved. // If this returns NULL, default values are used. diff --git a/chrome/browser/ui/views/location_bar/location_bar_view.cc b/chrome/browser/ui/views/location_bar/location_bar_view.cc index b1fc20af5b95..b7007ff879ef 100644 --- a/chrome/browser/ui/views/location_bar/location_bar_view.cc +++ b/chrome/browser/ui/views/location_bar/location_bar_view.cc @@ -1063,7 +1063,9 @@ bool LocationBarView::ShouldShowKeywordBubble() const { } bool LocationBarView::ShouldShowEVBubble() const { - return (GetToolbarModel()->GetSecurityLevel(false) == + const ChromeToolbarModel* chrome_toolbar_model = + static_cast(GetToolbarModel()); + return (chrome_toolbar_model->GetSecurityLevel(false) == connection_security::EV_SECURE); } diff --git a/chrome/browser/ui/views/location_bar/location_bar_view.h b/chrome/browser/ui/views/location_bar/location_bar_view.h index 8c4304f86a91..45a80021bdb2 100644 --- a/chrome/browser/ui/views/location_bar/location_bar_view.h +++ b/chrome/browser/ui/views/location_bar/location_bar_view.h @@ -14,7 +14,7 @@ #include "chrome/browser/ui/location_bar/location_bar.h" #include "chrome/browser/ui/omnibox/omnibox_edit_controller.h" #include "chrome/browser/ui/search/search_model_observer.h" -#include "chrome/browser/ui/toolbar/toolbar_model.h" +#include "chrome/browser/ui/toolbar/chrome_toolbar_model.h" #include "chrome/browser/ui/views/dropdown_bar_host.h" #include "chrome/browser/ui/views/dropdown_bar_host_delegate.h" #include "chrome/browser/ui/views/extensions/extension_popup.h" diff --git a/chrome/browser/ui/views/omnibox/omnibox_view_views.cc b/chrome/browser/ui/views/omnibox/omnibox_view_views.cc index adc7bd44d96f..44ba4d9bcf8c 100644 --- a/chrome/browser/ui/views/omnibox/omnibox_view_views.cc +++ b/chrome/browser/ui/views/omnibox/omnibox_view_views.cc @@ -204,8 +204,7 @@ void OmniboxViewViews::SaveStateToTab(content::WebContents* tab) { } void OmniboxViewViews::OnTabChanged(const content::WebContents* web_contents) { - security_level_ = controller()->GetToolbarModel()->GetSecurityLevel(false); - + UpdateSecurityLevel(); const OmniboxState* state = static_cast( web_contents->GetUserData(&OmniboxState::kKey)); model()->RestoreState(state ? &state->model_state : NULL); @@ -228,7 +227,7 @@ void OmniboxViewViews::ResetTabState(content::WebContents* web_contents) { void OmniboxViewViews::Update() { const connection_security::SecurityLevel old_security_level = security_level_; - security_level_ = controller()->GetToolbarModel()->GetSecurityLevel(false); + UpdateSecurityLevel(); if (model()->UpdatePermanentText()) { // Something visibly changed. Re-enable URL replacement. controller()->GetToolbarModel()->set_url_replacement_enabled(true); @@ -433,6 +432,12 @@ void OmniboxViewViews::AccessibilitySetValue(const base::string16& new_value) { SetUserText(new_value, new_value, true); } +void OmniboxViewViews::UpdateSecurityLevel() { + ChromeToolbarModel* chrome_toolbar_model = + static_cast(controller()->GetToolbarModel()); + security_level_ = chrome_toolbar_model->GetSecurityLevel(false); +} + void OmniboxViewViews::SetWindowTextAndCaretPos(const base::string16& text, size_t caret_pos, bool update_popup, diff --git a/chrome/browser/ui/views/omnibox/omnibox_view_views.h b/chrome/browser/ui/views/omnibox/omnibox_view_views.h index 00f68f54d8f2..8bd94d280189 100644 --- a/chrome/browser/ui/views/omnibox/omnibox_view_views.h +++ b/chrome/browser/ui/views/omnibox/omnibox_view_views.h @@ -12,7 +12,7 @@ #include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" #include "chrome/browser/ui/omnibox/omnibox_view.h" -#include "chrome/browser/ui/toolbar/toolbar_model.h" +#include "chrome/browser/ui/toolbar/chrome_toolbar_model.h" #include "ui/base/window_open_disposition.h" #include "ui/gfx/range/range.h" #include "ui/views/controls/textfield/textfield.h" @@ -122,6 +122,9 @@ class OmniboxViewViews // don't normally use this). Sets the value and clears the selection. void AccessibilitySetValue(const base::string16& new_value); + // Updates |security_level_| based on the toolbar model's current value. + void UpdateSecurityLevel(); + // OmniboxView: void SetWindowTextAndCaretPos(const base::string16& text, size_t caret_pos, diff --git a/chrome/chrome_browser_ui.gypi b/chrome/chrome_browser_ui.gypi index 7bd9e4beddc7..7cc7a8b5bf13 100644 --- a/chrome/chrome_browser_ui.gypi +++ b/chrome/chrome_browser_ui.gypi @@ -2712,6 +2712,8 @@ 'chrome_browser_ui_toolbar_model_sources': [ 'browser/ui/android/toolbar/toolbar_model_android.cc', 'browser/ui/android/toolbar/toolbar_model_android.h', + 'browser/ui/toolbar/chrome_toolbar_model.cc', + 'browser/ui/toolbar/chrome_toolbar_model.h', 'browser/ui/toolbar/toolbar_model.cc', 'browser/ui/toolbar/toolbar_model.h', 'browser/ui/toolbar/toolbar_model_delegate.h', -- 2.11.4.GIT