From 882daa9e07369d55b9e7647e0dd3b3316d3a74a4 Mon Sep 17 00:00:00 2001 From: "sky@chromium.org" Date: Thu, 5 Nov 2009 16:31:31 +0000 Subject: Makes it so that we sync changes in content state more often for selected tabs. This is necesitated by the fact that when closing a tab we don't grab the most recent content state, leaving session restore with slightly stale data. To really fix this requires sending a message on tab close and waiting for it, but that induces tab close jank, something we're not going to do. BUG=17220 TEST=none Review URL: http://codereview.chromium.org/342100 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@31097 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/renderer/render_view.cc | 43 +++++++++++++++++++++++------ chrome/renderer/render_view.h | 20 +++++++------- chrome/renderer/render_view_unittest.cc | 12 ++++---- chrome/renderer/render_view_unittest_mac.mm | 4 +-- chrome/renderer/render_widget.h | 2 ++ 5 files changed, 54 insertions(+), 27 deletions(-) diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc index 2391ab6..c4af4c9 100644 --- a/chrome/renderer/render_view.cc +++ b/chrome/renderer/render_view.cc @@ -187,9 +187,16 @@ static const int kDelayForCaptureMs = 500; // delay. static const int kDelayForForcedCaptureMs = 6000; -// The default value for RenderView.delay_seconds_for_form_state_sync_, see -// that variable for more. -const int kDefaultDelaySecondsForFormStateSync = 5; +// Time, in seconds, we delay before sending content state changes (such as form +// state and scroll position) to the browser. We delay sending changes to avoid +// spamming the browser. +// To avoid having tab/session restore require sending a message to get the +// current content state during tab closing we use a shorter timeout for the +// foreground renderer. This means there is a small window of time from which +// content state is modified and not sent to session restore, but this is +// better than having to wake up all renderers during shutdown. +static const int kDelaySecondsForContentStateSyncHidden = 5; +static const int kDelaySecondsForContentStateSync = 1; // The maximum number of popups that can be spawned from one page. static const int kMaximumNumberOfUnacknowledgedPopups = 25; @@ -234,7 +241,7 @@ RenderView::RenderView(RenderThreadBase* render_thread, autofill_query_id_(0), popup_notification_visible_(false), spelling_panel_visible_(false), - delay_seconds_for_form_state_sync_(kDefaultDelaySecondsForFormStateSync), + send_content_state_immediately_(false), send_preferred_size_changes_(false), ALLOW_THIS_IN_INITIALIZER_LIST( notification_provider_(new NotificationProvider(this))), @@ -1610,6 +1617,28 @@ void RenderView::UpdateTargetURL(const GURL& url, const GURL& fallback_url) { } } +void RenderView::StartNavStateSyncTimerIfNecessary() { + int delay; + if (send_content_state_immediately_) + delay = 0; + else if (is_hidden()) + delay = kDelaySecondsForContentStateSyncHidden; + else + delay = kDelaySecondsForContentStateSync; + + if (nav_state_sync_timer_.IsRunning()) { + // The timer is already running. If the delay of the timer maches the amount + // we want to delay by, then return. Otherwise stop the timer so that it + // gets started with the right delay. + if (nav_state_sync_timer_.GetCurrentDelay().InSeconds() == delay) + return; + nav_state_sync_timer_.Stop(); + } + + nav_state_sync_timer_.Start( + TimeDelta::FromSeconds(delay), this, &RenderView::SyncNavigationState); +} + void RenderView::setMouseOverURL(const WebURL& url) { mouse_over_url_ = GURL(url); UpdateTargetURL(mouse_over_url_, focus_url_); @@ -2316,11 +2345,7 @@ void RenderView::didChangeLocationWithinPage( } void RenderView::didUpdateCurrentHistoryItem(WebFrame* frame) { - if (!nav_state_sync_timer_.IsRunning()) { - nav_state_sync_timer_.Start( - TimeDelta::FromSeconds(delay_seconds_for_form_state_sync_), this, - &RenderView::SyncNavigationState); - } + StartNavStateSyncTimerIfNecessary(); } void RenderView::assignIdentifierToRequest( diff --git a/chrome/renderer/render_view.h b/chrome/renderer/render_view.h index c6fb295..f162b99 100644 --- a/chrome/renderer/render_view.h +++ b/chrome/renderer/render_view.h @@ -392,11 +392,11 @@ class RenderView : public RenderWidget, const std::string& css, const std::string& id); - int delay_seconds_for_form_state_sync() const { - return delay_seconds_for_form_state_sync_; - } - void set_delay_seconds_for_form_state_sync(int delay_in_seconds) { - delay_seconds_for_form_state_sync_ = delay_in_seconds; + // Whether content state (such as form state and scroll position) should be + // sent to the browser immediately. This is normally false, but set to true + // by some tests. + void set_send_content_state_immediately(bool value) { + send_content_state_immediately_ = value; } AudioMessageFilter* audio_message_filter() { return audio_message_filter_; } @@ -735,6 +735,9 @@ class RenderView : public RenderWidget, // If |url| is empty, show |fallback_url|. void UpdateTargetURL(const GURL& url, const GURL& fallback_url); + // Starts nav_state_sync_timer_ if it isn't already running. + void StartNavStateSyncTimerIfNecessary(); + // Bitwise-ORed set of extra bindings that have been enabled. See // BindingsPolicy for details. int enabled_bindings_; @@ -897,11 +900,8 @@ class RenderView : public RenderWidget, // True if the browser is showing the spelling panel for us. bool spelling_panel_visible_; - // Time in seconds of the delay between syncing page state such as form - // elements and scroll position. This timeout allows us to avoid spamming the - // browser process with every little thing that changes. This normally doesn't - // change but is overridden by tests. - int delay_seconds_for_form_state_sync_; + // See description above setter. + bool send_content_state_immediately_; scoped_refptr audio_message_filter_; diff --git a/chrome/renderer/render_view_unittest.cc b/chrome/renderer/render_view_unittest.cc index 2626096..8128c59 100644 --- a/chrome/renderer/render_view_unittest.cc +++ b/chrome/renderer/render_view_unittest.cc @@ -59,7 +59,7 @@ TEST_F(RenderViewTest, OnNavStateChanged) { // Don't want any delay for form state sync changes. This will still post a // message so updates will get coalesced, but as soon as we spin the message // loop, it will generate an update. - view_->set_delay_seconds_for_form_state_sync(0); + view_->set_send_content_state_immediately(true); LoadHTML(""); @@ -83,7 +83,7 @@ TEST_F(RenderViewTest, OnImeStateChanged) { view_->OnImeSetInputMode(true); // Load an HTML page consisting of two input fields. - view_->set_delay_seconds_for_form_state_sync(0); + view_->set_send_content_state_immediately(true); LoadHTML("" "" "" @@ -211,7 +211,7 @@ TEST_F(RenderViewTest, ImeComposition) { // and move the input focus to the
element, where we can use // IMEs. view_->OnImeSetInputMode(ime_message->enable); - view_->set_delay_seconds_for_form_state_sync(0); + view_->set_send_content_state_immediately(true); LoadHTML("" "" "" @@ -266,7 +266,7 @@ TEST_F(RenderViewTest, OnSetTextDirection) { // This test changes the text direction of the