Clean up the page state saving mechanism.

content/renderer/render_view_impl.cc

 // Copyright (c) 2012 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 "content/renderer/render_view_impl.h"
 
 #include <algorithm>
 #include <cmath>
 
 #include "base/auto_reset.h"
@@ skipping 307 matching lines @@
     LAZY_INSTANCE_INITIALIZER;
 
 // 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.
-const int kDelaySecondsForContentStateSyncHidden = 5;
-const int kDelaySecondsForContentStateSync = 1;
+const int kHiddenPageStateSendingDelaySeconds = 5;
+const int kPageStateSendingDelaySeconds = 1;
 
 #if defined(OS_ANDROID)
 // Delay between tapping in content and launching the associated android intent.
 // Used to allow users see what has been recognized as content.
 const size_t kContentIntentDelayMilliseconds = 700;
 #endif
 
 static RenderViewImpl* (*g_create_render_view_impl)(RenderViewImplParams*) =
     NULL;
 
@@ skipping 302 matching lines @@
 
 }  // namespace
 
 RenderViewImpl::RenderViewImpl(RenderViewImplParams* params)
     : RenderWidget(blink::WebPopupTypeNone,
                    params->screen_info,
                    params->swapped_out,
                    params->hidden,
                    params->never_visible),
       webkit_preferences_(params->webkit_prefs),
-      send_content_state_immediately_(false),
       enabled_bindings_(0),
       send_preferred_size_changes_(false),
       navigation_gesture_(NavigationGestureUnknown),
       opened_by_user_gesture_(true),
       opener_suppressed_(false),
       suppress_dialogs_until_swap_out_(false),
+      page_state_sent_immediately_(false),
+      page_state_dirty_(false),
       page_id_(-1),
       last_page_id_sent_to_browser_(-1),
       next_page_id_(params->next_page_id),
       history_list_offset_(-1),
       history_list_length_(0),
       frames_in_progress_(0),
       target_url_status_(TARGET_NONE),
       uses_temporary_zoom_level_(false),
 #if defined(OS_ANDROID)
       top_controls_constraints_(cc::BOTH),
@@ skipping 851 matching lines @@
     return;
   if (in_live_resize)
     webview()->willStartLiveResize();
   else
     webview()->willEndLiveResize();
 }
 #endif
 
 ///////////////////////////////////////////////////////////////////////////////
 
-// Sends the current history state to the browser so it will be saved before we
-// navigate to a new page.
-void RenderViewImpl::UpdateSessionHistory(WebFrame* frame) {
-  // If we have a valid page ID at this point, then it corresponds to the page
-  // we are navigating away from.  Otherwise, this is the first navigation, so
-  // there is no past session history to record.
-  if (page_id_ == -1)
-    return;
-  SendUpdateState(history_controller_->GetCurrentEntry());
-}
-
-void RenderViewImpl::SendUpdateState(HistoryEntry* entry) {
-  if (!entry)
-    return;
-
-  // Don't send state updates for kSwappedOutURL.
-  if (entry->root().urlString() == WebString::fromUTF8(kSwappedOutURL))
-    return;
-
-  Send(new ViewHostMsg_UpdateState(
-      routing_id_, page_id_, HistoryEntryToPageState(entry)));
-}
-
 bool RenderViewImpl::SendAndRunNestedMessageLoop(IPC::SyncMessage* message) {
   // Before WebKit asks us to show an alert (etc.), it takes care of doing the
   // equivalent of WebView::willEnterModalLoop.  In the case of showModalDialog
   // it is particularly important that we do not call willEnterModalLoop as
   // that would defer resource loads for the dialog itself.
   if (RenderThreadImpl::current())  // Will be NULL during unit tests.
     RenderThreadImpl::current()->DoNotNotifyWebKitOfModalLoop();
 
   message->EnableMessagePumping();  // Runs a nested message loop.
   return Send(message);
@@ skipping 329 matching lines @@
   }
 }
 
 gfx::RectF RenderViewImpl::ClientRectToPhysicalWindowRect(
     const gfx::RectF& rect) const {
   gfx::RectF window_rect = rect;
   window_rect.Scale(device_scale_factor_ * webview()->pageScaleFactor());
   return window_rect;
 }
 
-void RenderViewImpl::StartNavStateSyncTimerIfNecessary() {
-  // No need to update state if no page has committed yet.
+void RenderViewImpl::MarkPageStateAsDirty() {
+  // No need to send state if no page has committed yet.
   if (page_id_ == -1)
     return;
 
+  page_state_dirty_ = true;
+
   int delay;
-  if (send_content_state_immediately_)
+  if (page_state_sent_immediately_)
     delay = 0;
   else if (is_hidden())
-    delay = kDelaySecondsForContentStateSyncHidden;
+    delay = kHiddenPageStateSendingDelaySeconds;
   else
-    delay = kDelaySecondsForContentStateSync;
+    delay = kPageStateSendingDelaySeconds;
 
-  if (nav_state_sync_timer_.IsRunning()) {
+  if (page_state_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)
+    if (page_state_timer_.GetCurrentDelay().InSeconds() == delay)
       return;
-    nav_state_sync_timer_.Stop();
+    page_state_timer_.Stop();
   }
 
-  nav_state_sync_timer_.Start(FROM_HERE, TimeDelta::FromSeconds(delay), this,
-                              &RenderViewImpl::SyncNavigationState);
+  page_state_timer_.Start(FROM_HERE, TimeDelta::FromSeconds(delay), this,
+                          &RenderViewImpl::FlushPageState);
+}
+
+void RenderViewImpl::FlushPageState() {
+  if (!page_state_dirty_ || page_id_ == -1 || !webview())

[Charlie Reis, 2014/10/31 21:38:49]

Just want to verify that I understand the implications of page_state_dirty_.  I
agree that it makes sense to avoid sending the IPC if we don't need to, and that
we'll set it to true when state changes happen.

It looks like we currently don't send an initial UpdateState right after commit,
so there's no change in that case.

We do currently send UpdateState for pages that don't need it right before we
leave (either in DidCommit or OnSwapOut), but I can't imagine that's required. 
If there's any downside to this change, it's that it might make our browser-side
UpdateState DCHECK harder to reproduce, since it'll only happen if scrolling or
other state changes are occurring.  But that's not a good reason to skip the
dirty bit.  So seems ok to me.

[Avi (use Gerrit), 2014/10/31 22:11:12]

Exactly. This is an intentional change, where we don't send updates if there
wasn't a reason to send one.

+    return;
+
+  HistoryEntry* entry = history_controller_->GetCurrentEntry();
+  if (!entry)
+    return;
+
+  // Don't send state updates for kSwappedOutURL.
+  if (entry->root().urlString() == WebString::fromUTF8(kSwappedOutURL))
+    return;
+
+  Send(new ViewHostMsg_UpdateState(
+      routing_id_, page_id_, HistoryEntryToPageState(entry)));
+
+  page_state_dirty_ = false;
 }
 
 void RenderViewImpl::setMouseOverURL(const WebURL& url) {
   mouse_over_url_ = GURL(url);
   UpdateTargetURL(mouse_over_url_, focus_url_);
 }
 
 void RenderViewImpl::setKeyboardFocusURL(const WebURL& url) {
   focus_url_ = GURL(url);
   UpdateTargetURL(focus_url_, mouse_over_url_);
@@ skipping 465 matching lines @@
   for (size_t i = 0; i < icon_urls.size(); i++) {
     std::vector<gfx::Size> sizes;
     ConvertToFaviconSizes(icon_urls[i].sizes(), &sizes);
     urls.push_back(FaviconURL(
         icon_urls[i].iconURL(), ToFaviconType(icon_urls[i].iconType()), sizes));
   }
   SendUpdateFaviconURL(urls);
 }
 
 void RenderViewImpl::didUpdateCurrentHistoryItem(WebLocalFrame* frame) {
-  StartNavStateSyncTimerIfNecessary();
+  MarkPageStateAsDirty();
 }
 
 void RenderViewImpl::CheckPreferredSize() {
   // We don't always want to send the change messages over IPC, only if we've
   // been put in that mode by getting a |ViewMsg_EnablePreferredSizeChangedMode|
   // message.
   if (!send_preferred_size_changes_ || !webview())
     return;
 
   gfx::Size size = webview()->contentsPreferredMinimumSize();
@@ skipping 12 matching lines @@
                                                       preferred_size_));
 }
 
 BrowserPluginManager* RenderViewImpl::GetBrowserPluginManager() {
   if (!browser_plugin_manager_.get())
     browser_plugin_manager_ = BrowserPluginManager::Create(this);
   return browser_plugin_manager_.get();
 }
 
 void RenderViewImpl::didChangeScrollOffset(WebLocalFrame* frame) {
-  StartNavStateSyncTimerIfNecessary();
+  MarkPageStateAsDirty();
 
   FOR_EACH_OBSERVER(
       RenderViewObserver, observers_, DidChangeScrollOffset(frame));
 }
 
 void RenderViewImpl::SendFindReply(int request_id,
                                    int match_count,
                                    int ordinal,
                                    const WebRect& selection_rect,
                                    bool final_status_update) {
@@ skipping 103 matching lines @@
 bool RenderViewImpl::ShouldDisplayScrollbars(int width, int height) const {
   return (!send_preferred_size_changes_ ||
           (disable_scrollbars_size_limit_.width() <= width ||
            disable_scrollbars_size_limit_.height() <= height));
 }
 
 int RenderViewImpl::GetEnabledBindings() const {
   return enabled_bindings_;
 }
 
-bool RenderViewImpl::GetContentStateImmediately() const {
-  return send_content_state_immediately_;
+bool RenderViewImpl::GetPageStateSentImmediately() const {
+  return page_state_sent_immediately_;
 }
 
 blink::WebPageVisibilityState RenderViewImpl::GetVisibilityState() const {
   return visibilityState();
 }
 
 void RenderViewImpl::DidStartLoading() {
   main_render_frame_->didStartLoading(true);
 }
 
 void RenderViewImpl::DidStopLoading() {
   main_render_frame_->didStopLoading();
 }
 
-void RenderViewImpl::SyncNavigationState() {
-  if (!webview())
-    return;
-  SendUpdateState(history_controller_->GetCurrentEntry());
+void RenderViewImpl::ForcePageStateFlushForTesting() {
+  FlushPageState();
 }
 
 blink::WebPlugin* RenderViewImpl::GetWebPluginForFind() {
   if (!webview())
     return NULL;
 
   WebFrame* main_frame = webview()->mainFrame();
   if (main_frame->isWebLocalFrame() &&
       main_frame->document().isPluginDocument())
     return webview()->mainFrame()->document().to<WebPluginDocument>().plugin();
@@ skipping 1616 matching lines @@
     std::vector<gfx::Size> sizes;
     ConvertToFaviconSizes(icon_urls[i].sizes(), &sizes);
     if (!url.isEmpty())
       urls.push_back(
           FaviconURL(url, ToFaviconType(icon_urls[i].iconType()), sizes));
   }
   SendUpdateFaviconURL(urls);
 }
 
 }  // namespace content