Remove RenderFrameHostImplState and convert it to boolean.

content/browser/frame_host/render_frame_host_impl.cc

 // Copyright 2013 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/browser/frame_host/render_frame_host_impl.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 118 matching lines @@
       return base::i18n::RIGHT_TO_LEFT;
     default:
       NOTREACHED();
       return base::i18n::UNKNOWN_DIRECTION;
   }
 }
 
 }  // namespace
 
 // static
-bool RenderFrameHostImpl::IsRFHStateActive(RenderFrameHostImplState rfh_state) {
-  return rfh_state == STATE_DEFAULT;
-}
-
-// static
 RenderFrameHost* RenderFrameHost::FromID(int render_process_id,
                                          int render_frame_id) {
   return RenderFrameHostImpl::FromID(render_process_id, render_frame_id);
 }
 
 #if defined(OS_ANDROID)
 // static
 void RenderFrameHost::AllowInjectingJavaScriptForAndroidWebView() {
   g_allow_injecting_javascript = true;
 }
@@ skipping 36 matching lines @@
     : render_view_host_(render_view_host),
       delegate_(delegate),
       site_instance_(static_cast<SiteInstanceImpl*>(site_instance)),
       process_(site_instance->GetProcess()),
       cross_process_frame_connector_(NULL),
       render_frame_proxy_host_(NULL),
       frame_tree_(frame_tree),
       frame_tree_node_(frame_tree_node),
       render_widget_host_(nullptr),
       routing_id_(routing_id),
-      rfh_state_(STATE_DEFAULT),
+      is_waiting_for_swapout_ack_(false),
       render_frame_created_(false),
       navigations_suspended_(false),
       is_waiting_for_beforeunload_ack_(false),
       unload_ack_is_for_navigation_(false),
       is_loading_(false),
       pending_commit_(false),
       nav_entry_id_(0),
       accessibility_reset_token_(0),
       accessibility_reset_count_(0),
       browser_plugin_embedder_ax_tree_id_(AXTreeIDRegistry::kNoAXTreeID),
@@ skipping 49 matching lines @@
 
   GetProcess()->RemoveRoute(routing_id_);
   g_routing_id_frame_map.Get().erase(
       RenderFrameHostID(GetProcess()->GetID(), routing_id_));
 
   site_instance_->RemoveObserver(this);
 
   if (delegate_ && render_frame_created_)
     delegate_->RenderFrameDeleted(this);
 
-  bool is_active = IsRFHStateActive(rfh_state_);
-
   // If this RenderFrameHost is swapped out, it already decremented the active
   // frame count of the SiteInstance it belongs to.
-  if (is_active)
+  if (is_active())
     GetSiteInstance()->DecrementActiveFrameCount();
 
   // If this RenderFrameHost is swapping with a RenderFrameProxyHost, the
   // RenderFrame will already be deleted in the renderer process. Main frame
   // RenderFrames will be cleaned up as part of deleting its RenderView. In all
   // other cases, the RenderFrame should be cleaned up (if it exists).
-  if (is_active && !frame_tree_node_->IsMainFrame() && render_frame_created_)
+  if (is_active() && !frame_tree_node_->IsMainFrame() && render_frame_created_)
     Send(new FrameMsg_Delete(routing_id_));
 
   // NULL out the swapout timer; in crash dumps this member will be null only if
   // the dtor has run.
   swapout_event_monitor_timeout_.reset();
 
   for (const auto& iter: visual_state_callbacks_) {
     iter.second.Run(false);
   }
 
@@ skipping 545 matching lines @@
     const std::string& frame_unique_name,
     blink::WebSandboxFlags sandbox_flags,
     const blink::WebFrameOwnerProperties& frame_owner_properties) {
   // TODO(lukasza): Call ReceivedBadMessage when |frame_unique_name| is empty.
   DCHECK(!frame_unique_name.empty());
 
   // It is possible that while a new RenderFrameHost was committed, the
   // RenderFrame corresponding to this host sent an IPC message to create a
   // frame and it is delivered after this host is swapped out.
   // Ignore such messages, as we know this RenderFrameHost is going away.
-  if (rfh_state_ != RenderFrameHostImpl::STATE_DEFAULT ||
-      frame_tree_node_->current_frame_host() != this)
+  if (!is_active() || frame_tree_node_->current_frame_host() != this)
     return;
 
   frame_tree_->AddFrame(frame_tree_node_, GetProcess()->GetID(), new_routing_id,
                         scope, frame_name, frame_unique_name, sandbox_flags,
                         frame_owner_properties);
 }
 
 void RenderFrameHostImpl::OnDetach() {
   frame_tree_->RemoveFrame(frame_tree_node_);
 }
@@ skipping 314 matching lines @@
 
 void RenderFrameHostImpl::SwapOut(
     RenderFrameProxyHost* proxy,
     bool is_loading) {
   // The end of this event is in OnSwapOutACK when the RenderFrame has completed
   // the operation and sends back an IPC message.
   // The trace event may not end properly if the ACK times out.  We expect this
   // to be fixed when RenderViewHostImpl::OnSwapOut moves to RenderFrameHost.
   TRACE_EVENT_ASYNC_BEGIN0("navigation", "RenderFrameHostImpl::SwapOut", this);
 
-  // If this RenderFrameHost is not in the default state, it must have already
+  // If this RenderFrameHost is already pending deletion, it must have already
   // gone through this, therefore just return.
-  if (rfh_state_ != RenderFrameHostImpl::STATE_DEFAULT) {
+  if (!is_active()) {
     NOTREACHED() << "RFH should be in default state when calling SwapOut.";
     return;
   }
 
   swapout_event_monitor_timeout_->Start(
       base::TimeDelta::FromMilliseconds(RenderViewHostImpl::kUnloadTimeoutMS));
 
   // There may be no proxy if there are no active views in the process.
   int proxy_routing_id = MSG_ROUTING_NONE;
   FrameReplicationState replication_state;
   if (proxy) {
     set_render_frame_proxy_host(proxy);
     proxy_routing_id = proxy->GetRoutingID();
     replication_state = proxy->frame_tree_node()->current_replication_state();
   }
 
   if (IsRenderFrameLive()) {
     Send(new FrameMsg_SwapOut(routing_id_, proxy_routing_id, is_loading,
                               replication_state));
   }
 
-  // If this is the last active frame in the SiteInstance, the SetState call
-  // below will trigger the deletion of the SiteInstance's proxies.
-  SetState(RenderFrameHostImpl::STATE_PENDING_SWAP_OUT);
+  // TODO(nasko): If the frame is not live, shouldn't we just delete the RFH by
+  // simulating the receipt of swap out ack?

[Charlie Reis, 2016/04/08 20:25:37]

Since we know we want to this, let's rephrase into a statement:

s/shouldn't we/we should/

[nasko, 2016/04/08 21:25:32]

Done.

+  is_waiting_for_swapout_ack_ = true;
+  if (frame_tree_node_->IsMainFrame())
+    render_view_host_->set_is_active(false);
+
+  // If this is the last active frame in the SiteInstance, the
+  // DecrementActiveFrameCount call will trigger the deletion of the
+  // SiteInstance's proxies.
+  GetSiteInstance()->DecrementActiveFrameCount();
 
   if (!GetParent())
     delegate_->SwappedOut(this);
 }
 
 void RenderFrameHostImpl::OnBeforeUnloadACK(
     bool proceed,
     const base::TimeTicks& renderer_before_unload_start_time,
     const base::TimeTicks& renderer_before_unload_end_time) {
   TRACE_EVENT_ASYNC_END1("navigation", "RenderFrameHostImpl BeforeUnload", this,
@@ skipping 79 matching lines @@
         before_unload_end_time);
   }
 
   // If canceled, notify the delegate to cancel its pending navigation entry.
   if (!proceed)
     render_view_host_->GetDelegate()->DidCancelLoading();
 }
 
 bool RenderFrameHostImpl::IsWaitingForUnloadACK() const {
   return render_view_host_->is_waiting_for_close_ack_ ||
-      rfh_state_ == STATE_PENDING_SWAP_OUT;
+         is_waiting_for_swapout_ack_;
 }
 
 void RenderFrameHostImpl::OnSwapOutACK() {
   OnSwappedOut();
 }
 
 void RenderFrameHostImpl::OnRenderProcessGone(int status, int exit_code) {
   if (frame_tree_node_->IsMainFrame()) {
     // Keep the termination status so we can get at it later when we
     // need to know why it died.
@@ skipping 13 matching lines @@
 
   // Execute any pending AX tree snapshot callbacks with an empty response,
   // since we're never going to get a response from this renderer.
   for (const auto& iter : ax_tree_snapshot_callbacks_)
     iter.second.Run(ui::AXTreeUpdate());
   ax_tree_snapshot_callbacks_.clear();
 
   // If the process has died, we don't need to wait for the swap out ack from
   // this RenderFrame if it is pending deletion.  Complete the swap out to
   // destroy it.
-  if (!IsRFHStateActive(rfh_state()))
+  if (!is_active())
     OnSwappedOut();
 
   // Note: don't add any more code at this point in the function because
   // |this| may be deleted. Any additional cleanup should happen before
   // the last block of code here.
 }
 
 void RenderFrameHostImpl::OnSwappedOut() {
   // Ignore spurious swap out ack.
-  if (rfh_state_ != STATE_PENDING_SWAP_OUT)
+  if (!is_waiting_for_swapout_ack_)
     return;
 
   TRACE_EVENT_ASYNC_END0("navigation", "RenderFrameHostImpl::SwapOut", this);
   swapout_event_monitor_timeout_->Stop();
 
   ClearAllWebUI();
 
   // If this is a main frame RFH that's about to be deleted, update its RVH's
-  // swapped-out state here, since SetState won't be called once this RFH is
-  // deleted below. https://crbug.com/505887
+  // swapped-out state here. https://crbug.com/505887
   if (frame_tree_node_->IsMainFrame()) {
     render_view_host_->set_is_active(false);
     render_view_host_->set_is_swapped_out(true);
   }
 
   bool deleted =
       frame_tree_node_->render_manager()->DeleteFromPendingList(this);
   CHECK(deleted);
 }
 
@@ skipping 256 matching lines @@
   // reset but this message includes a reset token.
   if (accessibility_reset_token_ != reset_token) {
     Send(new AccessibilityMsg_Events_ACK(routing_id_));
     return;
   }
   accessibility_reset_token_ = 0;
 
   RenderWidgetHostViewBase* view = GetViewForAccessibility();
 
   AccessibilityMode accessibility_mode = delegate_->GetAccessibilityMode();
-  if ((accessibility_mode != AccessibilityModeOff) && view &&
-      RenderFrameHostImpl::IsRFHStateActive(rfh_state())) {
+  if ((accessibility_mode != AccessibilityModeOff) && view && is_active()) {
     if (accessibility_mode & AccessibilityModeFlagPlatform)
       GetOrCreateBrowserAccessibilityManager();
 
     std::vector<AXEventNotificationDetails> details;
     details.reserve(params.size());
     for (size_t i = 0; i < params.size(); ++i) {
       const AccessibilityHostMsg_EventParams& param = params[i];
       AXEventNotificationDetails detail;
       detail.event_type = param.event_type;
       detail.id = param.id;
@@ skipping 49 matching lines @@
   Send(new AccessibilityMsg_Events_ACK(routing_id_));
 }
 
 void RenderFrameHostImpl::OnAccessibilityLocationChanges(
     const std::vector<AccessibilityHostMsg_LocationChangeParams>& params) {
   if (accessibility_reset_token_)
     return;
 
   RenderWidgetHostViewBase* view = static_cast<RenderWidgetHostViewBase*>(
       render_view_host_->GetWidget()->GetView());
-  if (view && RenderFrameHostImpl::IsRFHStateActive(rfh_state())) {
+  if (view && is_active()) {
     AccessibilityMode accessibility_mode = delegate_->GetAccessibilityMode();
     if (accessibility_mode & AccessibilityModeFlagPlatform) {
       BrowserAccessibilityManager* manager =
           GetOrCreateBrowserAccessibilityManager();
       if (manager)
         manager->OnLocationChanges(params);
     }
     // TODO(aboxhall): send location change events to web contents observers too
   }
 }
@@ skipping 59 matching lines @@
   if (IsBrowserSideNavigationEnabled() && to_different_document) {
     bad_message::ReceivedBadMessage(GetProcess(),
                                     bad_message::RFH_UNEXPECTED_LOAD_START);
     return;
   }
   bool was_previously_loading = frame_tree_node_->frame_tree()->IsLoading();
   is_loading_ = true;
 
   // Only inform the FrameTreeNode of a change in load state if the load state
   // of this RenderFrameHost is being tracked.
-  if (rfh_state_ == STATE_DEFAULT) {
+  if (is_active()) {
     frame_tree_node_->DidStartLoading(to_different_document,
                                       was_previously_loading);
   }
 }
 
 void RenderFrameHostImpl::OnDidStopLoading() {
   // This method should never be called when the frame is not loading.
   // Unfortunately, it can happen if a history navigation happens during a
   // BeforeUnload or Unload event.
   // TODO(fdegans): Change this to a DCHECK after LoadEventProgress has been
   // refactored in Blink. See crbug.com/466089
   if (!is_loading_) {
     LOG(WARNING) << "OnDidStopLoading was called twice.";
     return;
   }
 
   is_loading_ = false;
   navigation_handle_.reset();
 
   // Only inform the FrameTreeNode of a change in load state if the load state
   // of this RenderFrameHost is being tracked.
-  if (rfh_state_ == STATE_DEFAULT)
+  if (is_active())
     frame_tree_node_->DidStopLoading();
 }
 
 void RenderFrameHostImpl::OnDidChangeLoadProgress(double load_progress) {
   frame_tree_node_->DidChangeLoadProgress(load_progress);
 }
 
 void RenderFrameHostImpl::OnSerializeAsMHTMLResponse(
     int job_id,
     bool success,
@@ skipping 90 matching lines @@
 
   if (browser_command_line.HasSwitch(switches::kEnableWebVR)) {
     GetServiceRegistry()->AddService(base::Bind(&VRDeviceManager::BindRequest));
   }
 #endif
 
   GetContentClient()->browser()->RegisterRenderFrameMojoServices(
       GetServiceRegistry(), this);
 }
 
-void RenderFrameHostImpl::SetState(RenderFrameHostImplState rfh_state) {
-  // We decrement the number of RenderFrameHosts in a SiteInstance when the
-  // status of a RenderFrameHost gets flipped from active.
-  if (IsRFHStateActive(rfh_state_) && !IsRFHStateActive(rfh_state))
-    GetSiteInstance()->DecrementActiveFrameCount();
-
+void RenderFrameHostImpl::ResetWaitingState() {

[Charlie Reis, 2016/04/08 20:25:37]

Let's put a DCHECK(is_active()) just to catch anyone who adds a bad call to
this.

[nasko, 2016/04/08 21:25:32]

Done.

   // The active state of the RVH is determined by its main frame, since
   // subframes should have their own widgets.
   if (frame_tree_node_->IsMainFrame()) {
-    render_view_host_->set_is_active(IsRFHStateActive(rfh_state));
+    render_view_host_->set_is_active(true);
     render_view_host_->set_is_swapped_out(false);
   }
 
-  // Whenever we change the RFH state to and from active state, we should not be
-  // waiting for beforeunload or close acks.  We clear them here to be safe,
-  // since they can cause navigations to be ignored in
-  // OnDidCommitProvisionalLoad.
-  // TODO(creis): Move is_waiting_for_beforeunload_ack_ into the state machine.
-  if (rfh_state == STATE_DEFAULT || rfh_state_ == STATE_DEFAULT) {
-    if (is_waiting_for_beforeunload_ack_) {
-      is_waiting_for_beforeunload_ack_ = false;
-      render_view_host_->GetWidget()->decrement_in_flight_event_count();
-      render_view_host_->GetWidget()->StopHangMonitorTimeout();
-    }
-    send_before_unload_start_time_ = base::TimeTicks();
-    render_view_host_->is_waiting_for_close_ack_ = false;
+  // Whenever we reset the RFH state, we should not be waiting for beforeunload
+  // or close acks.  We clear them here to be safe, since they can cause
+  // navigations to be ignored in OnDidCommitProvisionalLoad.
+  if (is_waiting_for_beforeunload_ack_) {
+    is_waiting_for_beforeunload_ack_ = false;
+    render_view_host_->GetWidget()->decrement_in_flight_event_count();
+    render_view_host_->GetWidget()->StopHangMonitorTimeout();
   }
-  rfh_state_ = rfh_state;
+  send_before_unload_start_time_ = base::TimeTicks();
+  render_view_host_->is_waiting_for_close_ack_ = false;
 }
 
 bool RenderFrameHostImpl::CanCommitURL(const GURL& url) {
   // TODO(creis): We should also check for WebUI pages here.  Also, when the
   // out-of-process iframes implementation is ready, we should check for
   // cross-site URLs that are not allowed to commit in this process.
 
   // Give the client a chance to disallow URLs from committing.
   return GetContentClient()->browser()->CanCommitURL(GetProcess(), url);
 }
@@ skipping 45 matching lines @@
 
   // Only send the message if we aren't suspended at the start of a cross-site
   // request.
   if (navigations_suspended_) {
     // This may replace an existing set of params, if this is a pending RFH that
     // is navigated twice consecutively.
     suspended_nav_params_.reset(
         new NavigationParams(common_params, start_params, request_params));
   } else {
     // Get back to a clean state, in case we start a new navigation without
     // completing a RFH swap or unload handler.

[Charlie Reis, 2016/04/08 20:25:37]

nit: without completing an unload handler.

(I do wonder whether this case is dead code now, but we can leave it for now.)

Same in the comments below.

[nasko, 2016/04/08 21:25:32]

Done.

-    SetState(RenderFrameHostImpl::STATE_DEFAULT);
+    ResetWaitingState();
     SendNavigateMessage(common_params, start_params, request_params);
   }
 
   // Force the throbber to start. This is done because Blink's "started loading"
   // message will be received asynchronously from the UI of the browser. But the
   // throbber needs to be kept in sync with what's happening in the UI. For
   // example, the throbber will start immediately when the user navigates even
   // if the renderer is delayed. There is also an issue with the throbber
   // starting because the WebUI (which controls whether the favicon is
   // displayed) happens synchronously. If the start loading messages was
@@ skipping 145 matching lines @@
     scoped_ptr<StreamHandle> body,
     const CommonNavigationParams& common_params,
     const RequestNavigationParams& request_params,
     bool is_view_source) {
   DCHECK((response && body.get()) ||
           !ShouldMakeNetworkRequestForURL(common_params.url));
   UpdatePermissionsForNavigation(common_params, request_params);
 
   // Get back to a clean state, in case we start a new navigation without
   // completing a RFH swap or unload handler.
-  SetState(RenderFrameHostImpl::STATE_DEFAULT);
+  ResetWaitingState();
 
   // The renderer can exit view source mode when any error or cancellation
   // happen. When reusing the same renderer, overwrite to recover the mode.
   if (is_view_source &&
       this == frame_tree_node_->render_manager()->current_frame_host()) {
     DCHECK(!GetParent());
     render_view_host()->Send(new FrameMsg_EnableViewSourceMode(routing_id_));
   }
 
   const GURL body_url = body.get() ? body->GetURL() : GURL();
@@ skipping 20 matching lines @@
   }
 }
 
 void RenderFrameHostImpl::FailedNavigation(
     const CommonNavigationParams& common_params,
     const RequestNavigationParams& request_params,
     bool has_stale_copy_in_cache,
     int error_code) {
   // Get back to a clean state, in case a new navigation started without
   // completing a RFH swap or unload handler.
-  SetState(RenderFrameHostImpl::STATE_DEFAULT);
+  ResetWaitingState();
 
   Send(new FrameMsg_FailedNavigation(routing_id_, common_params, request_params,
                                      has_stale_copy_in_cache, error_code));
 
   // An error page is expected to commit, hence why is_loading_ is set to true.
   is_loading_ = true;
   frame_tree_node_->ResetNavigationRequest(true);
 }
 
 void RenderFrameHostImpl::SetUpMojoIfNeeded() {
@@ skipping 146 matching lines @@
         mojo::GetProxy(&mojo_image_downloader_));
   }
   return mojo_image_downloader_;
 }
 
 void RenderFrameHostImpl::ResetLoadingState() {
   if (is_loading()) {
     // When pending deletion, just set the loading state to not loading.
     // Otherwise, OnDidStopLoading will take care of that, as well as sending
     // notification to the FrameTreeNode about the change in loading state.
-    if (rfh_state_ != STATE_DEFAULT)
+    if (!is_active())
       is_loading_ = false;
     else
       OnDidStopLoading();
   }
 }
 
 bool RenderFrameHostImpl::IsSameSiteInstance(
     RenderFrameHostImpl* other_render_frame_host) {
   // As a sanity check, make sure the frame belongs to the same BrowserContext.
   CHECK_EQ(GetSiteInstance()->GetBrowserContext(),
@@ skipping 14 matching lines @@
 }
 
 void RenderFrameHostImpl::SetAccessibilityCallbackForTesting(
     const base::Callback<void(RenderFrameHostImpl*, ui::AXEvent, int)>&
         callback) {
   accessibility_testing_callback_ = callback;
 }
 
 void RenderFrameHostImpl::UpdateAXTreeData() {
   AccessibilityMode accessibility_mode = delegate_->GetAccessibilityMode();
-  if (accessibility_mode == AccessibilityModeOff ||
-      !RenderFrameHostImpl::IsRFHStateActive(rfh_state())) {
+  if (accessibility_mode == AccessibilityModeOff || !is_active()) {
     return;
   }
 
   std::vector<AXEventNotificationDetails> details;
   details.reserve(1U);
   AXEventNotificationDetails detail;
   detail.ax_tree_id = GetAXTreeID();
   detail.update.has_tree_data = true;
   AXContentTreeDataToAXTreeData(&detail.update.tree_data);
   details.push_back(detail);
@@ skipping 110 matching lines @@
   if (navigations_suspended_) {
     TRACE_EVENT_ASYNC_BEGIN0("navigation",
                              "RenderFrameHostImpl navigation suspended", this);
   } else {
     TRACE_EVENT_ASYNC_END0("navigation",
                            "RenderFrameHostImpl navigation suspended", this);
   }
 
   if (!suspend && suspended_nav_params_) {
     // There's navigation message params waiting to be sent. Now that we're not
-    // suspended anymore, resume navigation by sending them. If we were swapped
-    // out, we should also stop filtering out the IPC messages now.
-    SetState(RenderFrameHostImpl::STATE_DEFAULT);
+    // suspended anymore, resume navigation by sending them.
+    ResetWaitingState();
 
     DCHECK(!proceed_time.is_null());
     // TODO(csharrison): Make sure that PlzNavigate and the current architecture
     // measure navigation start in the same way in the presence of the
     // BeforeUnload event.
     suspended_nav_params_->common_params.navigation_start = proceed_time;
     SendNavigateMessage(suspended_nav_params_->common_params,
                         suspended_nav_params_->start_params,
                         suspended_nav_params_->request_params);
     suspended_nav_params_.reset();
@@ skipping 171 matching lines @@
 }
 
 void RenderFrameHostImpl::CreateWebBluetoothService(
     blink::mojom::WebBluetoothServiceRequest request) {
   DCHECK(!web_bluetooth_service_);
   web_bluetooth_service_.reset(
       new WebBluetoothServiceImpl(this, std::move(request)));
 }
 
 }  // namespace content