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

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<AudioMessageFilter> 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("<input type=\"text\" id=\"elt_text\"></input>");
 
@@ -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("<html>"
            "<head>"
            "</head>"
@@ -211,7 +211,7 @@ TEST_F(RenderViewTest, ImeComposition) {
         // and move the input focus to the <div> 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("<html>"
                 "<head>"
                 "</head>"
@@ -266,7 +266,7 @@ TEST_F(RenderViewTest, OnSetTextDirection) {
   // This test changes the text direction of the <textarea> element, and
   // writes the values of its 'dir' attribute and its 'direction' property to
   // verify that the text direction is changed.
-  view_->set_delay_seconds_for_form_state_sync(0);
+  view_->set_send_content_state_immediately(true);
   LoadHTML("<html>"
            "<head>"
            "</head>"
@@ -559,7 +559,7 @@ TEST_F(RenderViewTest, OnHandleKeyboardEvent) {
   // TODO(hbono): <http://crbug.com/2215> Our WebKit port set |ev.metaKey| to
   // true when pressing an alt key, i.e. the |ev.metaKey| value is not
   // trustworthy. We will check the |ev.metaKey| value when this issue is fixed.
-  view_->set_delay_seconds_for_form_state_sync(0);
+  view_->set_send_content_state_immediately(true);
   LoadHTML("<html>"
            "<head>"
            "<title></title>"
@@ -818,7 +818,7 @@ TEST_F(RenderViewTest, InsertCharacters) {
     // This <div> element is used by the EditorClientImpl class to insert
     // characters received through the RenderWidget::OnHandleInputEvent()
     // function.
-    view_->set_delay_seconds_for_form_state_sync(0);
+    view_->set_send_content_state_immediately(true);
     LoadHTML("<html>"
              "<head>"
              "<title></title>"
diff --git a/chrome/renderer/render_view_unittest_mac.mm b/chrome/renderer/render_view_unittest_mac.mm
index c2c766f..a83af57 100644
--- a/chrome/renderer/render_view_unittest_mac.mm
+++ b/chrome/renderer/render_view_unittest_mac.mm
@@ -89,7 +89,7 @@ TEST_F(RenderViewTest, MacTestCmdUp) {
 
   // First test when javascript does not eat keypresses -- should scroll.
   sprintf(htmlBuffer, kRawHtml, "true");
-  view_->set_delay_seconds_for_form_state_sync(0);
+  view_->set_send_content_state_immediately(true);
   LoadHTML(htmlBuffer);
   render_thread_.sink().ClearMessages();
 
@@ -112,7 +112,7 @@ TEST_F(RenderViewTest, MacTestCmdUp) {
 
   // Now let javascript eat the key events -- no scrolling should happen
   sprintf(htmlBuffer, kRawHtml, "false");
-  view_->set_delay_seconds_for_form_state_sync(0);
+  view_->set_send_content_state_immediately(true);
   LoadHTML(htmlBuffer);
   render_thread_.sink().ClearMessages();
 
diff --git a/chrome/renderer/render_widget.h b/chrome/renderer/render_widget.h
index f4270cc..7428ec3 100644
--- a/chrome/renderer/render_widget.h
+++ b/chrome/renderer/render_widget.h
@@ -162,6 +162,8 @@ class RenderWidget : public IPC::Channel::Listener,
   // state.
   void SetHidden(bool hidden);
 
+  bool is_hidden() const { return is_hidden_; }
+
   // True if a PaintRect_ACK message is pending.
   bool paint_reply_pending() const {
     return paint_reply_pending_;