diff options
| -rw-r--r-- | chrome/browser/ui/search/instant_controller.cc | 47 | ||||
| -rw-r--r-- | chrome/browser/ui/search/instant_controller.h | 30 | ||||
| -rw-r--r-- | chrome/browser/ui/search/instant_controller_unittest.cc | 254 | ||||
| -rw-r--r-- | chrome/browser/ui/search/instant_page.cc | 4 | ||||
| -rw-r--r-- | chrome/browser/ui/search/instant_page.h | 2 | ||||
| -rw-r--r-- | tools/metrics/histograms/histograms.xml | 1 |
6 files changed, 313 insertions, 25 deletions
diff --git a/chrome/browser/ui/search/instant_controller.cc b/chrome/browser/ui/search/instant_controller.cc index d01af5d..c861a1c 100644 --- a/chrome/browser/ui/search/instant_controller.cc +++ b/chrome/browser/ui/search/instant_controller.cc @@ -7,12 +7,15 @@ #include <iterator> #include "base/metrics/histogram.h" +#include "base/prefs/pref_service.h" #include "base/strings/string_util.h" #include "base/strings/stringprintf.h" #include "base/strings/utf_string_conversions.h" #include "chrome/browser/autocomplete/autocomplete_provider.h" #include "chrome/browser/autocomplete/autocomplete_result.h" #include "chrome/browser/autocomplete/search_provider.h" +#include "chrome/browser/content_settings/content_settings_provider.h" +#include "chrome/browser/content_settings/host_content_settings_map.h" #include "chrome/browser/history/history_service.h" #include "chrome/browser/history/history_service_factory.h" #include "chrome/browser/history/history_tab_helper.h" @@ -30,6 +33,8 @@ #include "chrome/browser/ui/search/search_tab_helper.h" #include "chrome/common/chrome_notification_types.h" #include "chrome/common/chrome_switches.h" +#include "chrome/common/content_settings_types.h" +#include "chrome/common/pref_names.h" #include "chrome/common/url_constants.h" #include "components/sessions/serialized_navigation_entry.h" #include "content/public/browser/navigation_entry.h" @@ -1588,25 +1593,25 @@ void InstantController::ReloadStaleNTP() { } bool InstantController::ShouldSwitchToLocalNTP() const { - if (!ntp_) + if (!ntp()) + return true; + + // Assume users with Javascript disabled do not want the online experience. + if (!IsJavascriptEnabled()) return true; // Already a local page. Not calling IsLocal() because we want to distinguish // between the Google-specific and generic local NTP. - if (extended_enabled() && ntp_->instant_url() == GetLocalInstantURL()) + if (extended_enabled() && ntp()->instant_url() == GetLocalInstantURL()) return false; if (PageIsCurrent(ntp())) return false; - // TODO(shishir): This is not completely reliable. Find a better way to detect - // startup time. - const bool in_startup = !browser_->GetActiveWebContents(); - // The preloaded NTP does not support instant yet. If we're not in startup, // always fall back to the local NTP. If we are in startup, use the local NTP // (unless the finch flag to use the remote NTP is set). - return !(in_startup && chrome::ShouldPreferRemoteNTPOnStartup()); + return !(InStartup() && chrome::ShouldPreferRemoteNTPOnStartup()); } void InstantController::ResetOverlay(const std::string& instant_url) { @@ -1631,6 +1636,10 @@ InstantController::ShouldSwitchToLocalOverlay() const { if (!overlay()) return DetermineFallbackReason(NULL, std::string()); + // Assume users with Javascript disabled do not want the online experience. + if (!IsJavascriptEnabled()) + return INSTANT_FALLBACK_JAVASCRIPT_DISABLED; + if (overlay()->IsLocal()) return INSTANT_FALLBACK_NONE; @@ -1874,3 +1883,27 @@ void InstantController::PopulateInstantAutocompleteResultFromMatch( << result->description << "' '" << result->search_query << "' " << result->transition << " " << result->autocomplete_match_index; } + +bool InstantController::IsJavascriptEnabled() const { + GURL instant_url(GetInstantURL()); + GURL origin(instant_url.GetOrigin()); + ContentSetting js_setting = profile()->GetHostContentSettingsMap()-> + GetContentSetting(origin, origin, CONTENT_SETTINGS_TYPE_JAVASCRIPT, + NO_RESOURCE_IDENTIFIER); + // Javascript can be disabled either in content settings or via a WebKit + // preference, so check both. Disabling it through the Settings page affects + // content settings. I'm not sure how to disable the WebKit preference, but + // it's theoretically possible some users have it off. + bool js_content_enabled = + js_setting == CONTENT_SETTING_DEFAULT || + js_setting == CONTENT_SETTING_ALLOW; + bool js_webkit_enabled = profile()->GetPrefs()->GetBoolean( + prefs::kWebKitJavascriptEnabled); + return js_content_enabled && js_webkit_enabled; +} + +bool InstantController::InStartup() const { + // TODO(shishir): This is not completely reliable. Find a better way to detect + // startup time. + return !browser_->GetActiveWebContents(); +} diff --git a/chrome/browser/ui/search/instant_controller.h b/chrome/browser/ui/search/instant_controller.h index cdb170b..0a3d42e 100644 --- a/chrome/browser/ui/search/instant_controller.h +++ b/chrome/browser/ui/search/instant_controller.h @@ -76,7 +76,8 @@ class InstantController : public InstantPage::Delegate { INSTANT_FALLBACK_ORIGIN_PATH_MISMATCH = 3, INSTANT_FALLBACK_INSTANT_NOT_SUPPORTED = 4, INSTANT_FALLBACK_NO_OVERLAY = 5, - INSTANT_FALLBACK_MAX = 6, + INSTANT_FALLBACK_JAVASCRIPT_DISABLED = 6, + INSTANT_FALLBACK_MAX = 7, }; InstantController(BrowserInstantController* browser, @@ -251,11 +252,30 @@ class InstantController : public InstantPage::Delegate { virtual InstantTab* instant_tab() const; virtual InstantNTP* ntp() const; + virtual Profile* profile() const; + + // Returns true if Javascript is enabled and false otherwise. + virtual bool IsJavascriptEnabled() const; + + // Returns true if the browser is in startup. + virtual bool InStartup() const; + private: friend class InstantExtendedManualTest; friend class InstantTestBase; - FRIEND_TEST_ALL_PREFIXES(InstantControllerTest, - ShouldSwitchToLocalOverlayReturn); +#define UNIT_F(test) FRIEND_TEST_ALL_PREFIXES(InstantControllerTest, test) + UNIT_F(DoesNotSwitchToLocalNTPIfOnCurrentNTP); + UNIT_F(DoesNotSwitchToLocalNTPIfOnLocalNTP); + UNIT_F(IsJavascriptEnabled); + UNIT_F(IsJavascriptEnabledChecksContentSettings); + UNIT_F(IsJavascriptEnabledChecksPrefs); + UNIT_F(PrefersRemoteNTPOnStartup); + UNIT_F(ShouldSwitchToLocalOverlay); + UNIT_F(SwitchesToLocalNTPIfJSDisabled); + UNIT_F(SwitchesToLocalNTPIfNoInstantSupport); + UNIT_F(SwitchesToLocalNTPIfNoNTPReady); + UNIT_F(SwitchesToLocalNTPIfPathBad); +#undef UNIT_F FRIEND_TEST_ALL_PREFIXES(InstantTest, OmniboxFocusLoadsInstant); FRIEND_TEST_ALL_PREFIXES(InstantExtendedTest, UsesOverlayIfTabNotReady); FRIEND_TEST_ALL_PREFIXES(InstantExtendedTest, @@ -319,8 +339,6 @@ class InstantController : public InstantPage::Delegate { TypedSearchURLDoesntReuseInstantTab); #endif - Profile* profile() const; - // Overridden from InstantPage::Delegate: // TODO(shishir): We assume that the WebContent's current RenderViewHost is // the RenderViewHost being created which is not always true. Fix this. @@ -373,7 +391,7 @@ class InstantController : public InstantPage::Delegate { // Returns the local Instant URL. (Just a convenience wrapper around // chrome::GetLocalInstantURL.) - std::string GetLocalInstantURL() const; + virtual std::string GetLocalInstantURL() const; // Returns true if |page| has an up-to-date Instant URL and supports Instant. // Note that local URLs will not pass this check. diff --git a/chrome/browser/ui/search/instant_controller_unittest.cc b/chrome/browser/ui/search/instant_controller_unittest.cc index d449236..3ed131e 100644 --- a/chrome/browser/ui/search/instant_controller_unittest.cc +++ b/chrome/browser/ui/search/instant_controller_unittest.cc @@ -3,12 +3,19 @@ // found in the LICENSE file. #include "base/memory/scoped_ptr.h" -#include "base/message_loop.h" #include "base/metrics/histogram.h" #include "base/metrics/histogram_samples.h" #include "base/metrics/statistics_recorder.h" +#include "base/prefs/pref_service.h" +#include "chrome/browser/content_settings/host_content_settings_map.h" +#include "chrome/browser/search/search.h" #include "chrome/browser/ui/search/instant_controller.h" +#include "chrome/browser/ui/search/instant_ntp.h" #include "chrome/browser/ui/search/instant_overlay.h" +#include "chrome/common/content_settings.h" +#include "chrome/common/pref_names.h" +#include "chrome/test/base/testing_profile.h" +#include "content/public/test/test_browser_thread.h" #include "testing/gtest/include/gtest/gtest.h" using base::HistogramBase; @@ -45,24 +52,75 @@ class TestableInstantOverlay : public InstantOverlay { bool test_is_local_; }; +class TestableInstantNTP : public InstantNTP { + public: + TestableInstantNTP(InstantController* controller, + const std::string& instant_url) + : InstantNTP(controller, instant_url) { + } + + // Overrides from InstantPage + virtual bool supports_instant() const OVERRIDE { + return test_supports_instant_; + } + + virtual bool IsLocal() const OVERRIDE { + return test_is_local_; + }; + + virtual const std::string& instant_url() const OVERRIDE { + return test_instant_url_; + } + + void set_instant_url(const std::string& instant_url) { + test_instant_url_ = instant_url; + } + + void set_supports_instant(bool supports_instant) { + test_supports_instant_ = supports_instant; + } + + void set_is_local(bool is_local) { + test_is_local_ = is_local; + } + + private: + std::string test_instant_url_; + bool test_supports_instant_; + bool test_is_local_; +}; + class TestableInstantController : public InstantController { public: TestableInstantController() : InstantController(NULL, true), test_instant_url_("http://test_url"), - test_extended_enabled_(true) {} + test_extended_enabled_(true), + override_javascript_enabled_(true), + test_javascript_enabled_(true), + test_in_startup_(false), + test_overlay_(NULL), + test_ntp_(NULL) {} // Overrides from InstantController virtual std::string GetInstantURL() const OVERRIDE { return test_instant_url_; } + virtual std::string GetLocalInstantURL() const OVERRIDE { + return "http://local_instant_url"; + } + virtual bool extended_enabled() const OVERRIDE { return test_extended_enabled_; } virtual InstantOverlay* overlay() const OVERRIDE { - return test_overlay_.get(); + return test_overlay_; + } + + virtual InstantNTP* ntp() const OVERRIDE { + return test_ntp_; } void set_instant_url(std::string instant_url) { @@ -74,19 +132,56 @@ class TestableInstantController : public InstantController { } void set_overlay(InstantOverlay* overlay) { - test_overlay_.reset(overlay); + test_overlay_ = overlay; + } + + void set_ntp(InstantNTP* ntp) { + test_ntp_ = ntp; + } + + void set_javascript_enabled(bool javascript_enabled) { + override_javascript_enabled_ = true; + test_javascript_enabled_ = javascript_enabled; + } + + void set_override_javascript_enabled(bool override_javascript_enabled) { + override_javascript_enabled_ = override_javascript_enabled; + } + + void set_in_startup(bool in_startup) { + test_in_startup_ = in_startup; + } + + virtual bool IsJavascriptEnabled() const OVERRIDE { + if (override_javascript_enabled_) + return test_javascript_enabled_; + else + return InstantController::IsJavascriptEnabled(); + } + + virtual bool InStartup() const OVERRIDE { + return test_in_startup_; + } + + virtual Profile* profile() const OVERRIDE { + return &profile_; } private: std::string test_instant_url_; bool test_extended_enabled_; - scoped_ptr<InstantOverlay> test_overlay_; + bool override_javascript_enabled_; + bool test_javascript_enabled_; + bool test_in_startup_; + InstantOverlay* test_overlay_; + InstantNTP* test_ntp_; + mutable TestingProfile profile_; }; class InstantControllerTest : public testing::Test { public: InstantControllerTest() - : loop_(base::MessageLoop::TYPE_DEFAULT), + : ui_thread_(content::BrowserThread::UI), instant_controller_(new TestableInstantController()) { } @@ -99,11 +194,11 @@ class InstantControllerTest : public testing::Test { } private: - base::MessageLoop loop_; + content::TestBrowserThread ui_thread_; scoped_ptr<TestableInstantController> instant_controller_; }; -TEST_F(InstantControllerTest, ShouldSwitchToLocalOverlayReturn) { +TEST_F(InstantControllerTest, ShouldSwitchToLocalOverlay) { InstantController::InstantFallbackReason fallback_reason; instant_controller()->set_extended_enabled(false); @@ -115,13 +210,19 @@ TEST_F(InstantControllerTest, ShouldSwitchToLocalOverlayReturn) { ASSERT_EQ(fallback_reason, InstantController::INSTANT_FALLBACK_NO_OVERLAY); std::string instant_url("http://test_url"); - TestableInstantOverlay* test_overlay = - new TestableInstantOverlay(instant_controller(), instant_url); + scoped_ptr<TestableInstantOverlay> test_overlay( + new TestableInstantOverlay(instant_controller(), instant_url)); test_overlay->set_is_local(true); - instant_controller()->set_overlay(test_overlay); + instant_controller()->set_overlay(test_overlay.get()); fallback_reason = instant_controller()->ShouldSwitchToLocalOverlay(); ASSERT_EQ(fallback_reason, InstantController::INSTANT_FALLBACK_NONE); + instant_controller()->set_javascript_enabled(false); + fallback_reason = instant_controller()->ShouldSwitchToLocalOverlay(); + ASSERT_EQ(fallback_reason, + InstantController::INSTANT_FALLBACK_JAVASCRIPT_DISABLED); + instant_controller()->set_javascript_enabled(true); + test_overlay->set_is_local(false); instant_controller()->set_instant_url(""); fallback_reason = instant_controller()->ShouldSwitchToLocalOverlay(); @@ -143,3 +244,134 @@ TEST_F(InstantControllerTest, ShouldSwitchToLocalOverlayReturn) { fallback_reason = instant_controller()->ShouldSwitchToLocalOverlay(); ASSERT_EQ(fallback_reason, InstantController::INSTANT_FALLBACK_NONE); } + +TEST_F(InstantControllerTest, PrefersRemoteNTPOnStartup) { + std::string instant_url("http://instant_url"); + scoped_ptr<TestableInstantNTP> ntp(new TestableInstantNTP( + instant_controller(), instant_url)); + ntp->set_is_local(false); + instant_controller()->set_ntp(ntp.get()); + instant_controller()->set_javascript_enabled(true); + instant_controller()->set_extended_enabled(true); + instant_controller()->set_instant_url(instant_url); + ntp->set_instant_url(instant_url); + ntp->set_supports_instant(false); + instant_controller()->set_in_startup(true); + EXPECT_EQ(!chrome::ShouldPreferRemoteNTPOnStartup(), + instant_controller()->ShouldSwitchToLocalNTP()); +} + +TEST_F(InstantControllerTest, SwitchesToLocalNTPIfNoInstantSupport) { + std::string instant_url("http://instant_url"); + scoped_ptr<TestableInstantNTP> ntp(new TestableInstantNTP( + instant_controller(), instant_url)); + ntp.reset(new TestableInstantNTP(instant_controller(), instant_url)); + ntp->set_is_local(false); + instant_controller()->set_ntp(ntp.get()); + instant_controller()->set_javascript_enabled(true); + instant_controller()->set_extended_enabled(true); + instant_controller()->set_instant_url(instant_url); + ntp->set_instant_url(instant_url); + ntp->set_supports_instant(false); + instant_controller()->set_in_startup(false); + EXPECT_TRUE(instant_controller()->ShouldSwitchToLocalNTP()); +} + +TEST_F(InstantControllerTest, SwitchesToLocalNTPIfPathBad) { + std::string instant_url("http://instant_url"); + scoped_ptr<TestableInstantNTP> ntp(new TestableInstantNTP( + instant_controller(), instant_url)); + ntp.reset(new TestableInstantNTP(instant_controller(), instant_url)); + ntp->set_is_local(false); + instant_controller()->set_ntp(ntp.get()); + instant_controller()->set_javascript_enabled(true); + instant_controller()->set_extended_enabled(true); + instant_controller()->set_instant_url("http://bogus_url"); + ntp->set_instant_url(instant_url); + ntp->set_supports_instant(true); + instant_controller()->set_in_startup(false); + EXPECT_TRUE(instant_controller()->ShouldSwitchToLocalNTP()); +} + +TEST_F(InstantControllerTest, DoesNotSwitchToLocalNTPIfOnCurrentNTP) { + std::string instant_url("http://instant_url"); + scoped_ptr<TestableInstantNTP> ntp(new TestableInstantNTP( + instant_controller(), instant_url)); + ntp.reset(new TestableInstantNTP(instant_controller(), instant_url)); + ntp->set_is_local(false); + instant_controller()->set_ntp(ntp.get()); + instant_controller()->set_javascript_enabled(true); + instant_controller()->set_extended_enabled(true); + instant_controller()->set_instant_url(instant_url); + ntp->set_instant_url(instant_url); + ntp->set_supports_instant(true); + instant_controller()->set_in_startup(false); + EXPECT_FALSE(instant_controller()->ShouldSwitchToLocalNTP()); +} + +TEST_F(InstantControllerTest, DoesNotSwitchToLocalNTPIfOnLocalNTP) { + std::string instant_url("http://instant_url"); + scoped_ptr<TestableInstantNTP> ntp(new TestableInstantNTP( + instant_controller(), instant_url)); + ntp.reset(new TestableInstantNTP(instant_controller(), instant_url)); + ntp->set_is_local(false); + instant_controller()->set_ntp(ntp.get()); + instant_controller()->set_javascript_enabled(true); + instant_controller()->set_extended_enabled(true); + instant_controller()->set_instant_url(instant_url); + ntp->set_instant_url("http://local_instant_url"); + ntp->set_supports_instant(true); + instant_controller()->set_in_startup(false); + EXPECT_FALSE(instant_controller()->ShouldSwitchToLocalNTP()); +} + +TEST_F(InstantControllerTest, SwitchesToLocalNTPIfJSDisabled) { + std::string instant_url("http://instant_url"); + scoped_ptr<TestableInstantNTP> ntp(new TestableInstantNTP( + instant_controller(), instant_url)); + ntp.reset(new TestableInstantNTP(instant_controller(), instant_url)); + ntp->set_is_local(false); + instant_controller()->set_ntp(ntp.get()); + instant_controller()->set_javascript_enabled(false); + instant_controller()->set_extended_enabled(true); + instant_controller()->set_instant_url(instant_url); + ntp->set_instant_url("http://local_instant_url"); + ntp->set_supports_instant(true); + instant_controller()->set_in_startup(false); + EXPECT_TRUE(instant_controller()->ShouldSwitchToLocalNTP()); +} + +TEST_F(InstantControllerTest, SwitchesToLocalNTPIfNoNTPReady) { + EXPECT_TRUE(instant_controller()->ShouldSwitchToLocalNTP()); +} + +TEST_F(InstantControllerTest, IsJavascriptEnabled) { + instant_controller()->set_override_javascript_enabled(false); + EXPECT_TRUE(instant_controller()->IsJavascriptEnabled()); +} + +TEST_F(InstantControllerTest, IsJavascriptEnabledChecksContentSettings) { + instant_controller()->set_override_javascript_enabled(false); + instant_controller()->profile()->GetHostContentSettingsMap() + ->SetDefaultContentSetting(CONTENT_SETTINGS_TYPE_JAVASCRIPT, + CONTENT_SETTING_DEFAULT); + EXPECT_TRUE(instant_controller()->IsJavascriptEnabled()); + instant_controller()->profile()->GetHostContentSettingsMap() + ->SetDefaultContentSetting(CONTENT_SETTINGS_TYPE_JAVASCRIPT, + CONTENT_SETTING_ALLOW); + EXPECT_TRUE(instant_controller()->IsJavascriptEnabled()); + instant_controller()->profile()->GetHostContentSettingsMap() + ->SetDefaultContentSetting(CONTENT_SETTINGS_TYPE_JAVASCRIPT, + CONTENT_SETTING_BLOCK); + EXPECT_FALSE(instant_controller()->IsJavascriptEnabled()); +} + +TEST_F(InstantControllerTest, IsJavascriptEnabledChecksPrefs) { + instant_controller()->set_override_javascript_enabled(false); + instant_controller()->profile()->GetPrefs()->SetBoolean( + prefs::kWebKitJavascriptEnabled, true); + EXPECT_TRUE(instant_controller()->IsJavascriptEnabled()); + instant_controller()->profile()->GetPrefs()->SetBoolean( + prefs::kWebKitJavascriptEnabled, false); + EXPECT_FALSE(instant_controller()->IsJavascriptEnabled()); +} diff --git a/chrome/browser/ui/search/instant_page.cc b/chrome/browser/ui/search/instant_page.cc index 8568d2e..356ecd0 100644 --- a/chrome/browser/ui/search/instant_page.cc +++ b/chrome/browser/ui/search/instant_page.cc @@ -25,6 +25,10 @@ bool InstantPage::supports_instant() const { return supports_instant_; } +const std::string& InstantPage::instant_url() const { + return instant_url_; +} + bool InstantPage::IsLocal() const { return contents() && (contents()->GetURL() == GURL(chrome::kChromeSearchLocalNtpUrl) || diff --git a/chrome/browser/ui/search/instant_page.h b/chrome/browser/ui/search/instant_page.h index 5bb3045..341defc 100644 --- a/chrome/browser/ui/search/instant_page.h +++ b/chrome/browser/ui/search/instant_page.h @@ -111,7 +111,7 @@ class InstantPage : public content::WebContentsObserver { // Returns the Instant URL that was loaded for this page. Returns the empty // string if no URL was explicitly loaded as is the case for InstantTab. - const std::string& instant_url() const { return instant_url_; } + virtual const std::string& instant_url() const; // Returns true if the page is known to support the Instant API. This starts // out false, and is set to true whenever we get any message from the page. diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index d6c2d91..1658339 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -15001,6 +15001,7 @@ other types of suffix sets. <int value="3" label="Page not current: origin/path mismatch"/> <int value="4" label="Page not current: instant not supported"/> <int value="5" label="No overlay"/> + <int value="6" label="Javascript disabled"/> </enum> <enum name="InstantExtended_InstantNavigation" type="int"> |