Refactor sudden termination

content/renderer/render_frame_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/renderer/render_frame_impl.h"
 
 #include <map>
 #include <string>
 
 #include "base/auto_reset.h"
@@ skipping 459 matching lines @@
     extra_data = &kEmptyData;
   CommonNavigationParams params;
   params.url = request->url();
   params.referrer = Referrer(
       GURL(request->httpHeaderField(WebString::fromUTF8("Referer")).latin1()),
       request->referrerPolicy());
   params.transition = extra_data->transition_type();
   return params;
 }
 
+// Returns true if the disablers tracked by counter go from being absent to

[Charlie Reis, 2015/01/23 06:01:32]

nit: |counter|

[clamy, 2015/01/26 16:38:40]

Removed this function which was no longer needed.

+// present or vice-versa.
+bool HandleSuddenTerminationDisablerChange(int* counter, int variation) {
+  bool presence_before = *counter;
+  *counter += variation;
+  DCHECK(*counter >= 0);
+  bool presence_after = *counter;
+  return presence_before == presence_after;
+}
+
 #if !defined(OS_ANDROID)
 media::Context3D GetSharedMainThreadContext3D() {
   cc::ContextProvider* provider =
       RenderThreadImpl::current()->SharedMainThreadContextProvider().get();
   if (!provider)
     return media::Context3D();
   return media::Context3D(provider->ContextGL(), provider->GrContext());
 }
 #endif
 
@@ skipping 156 matching lines @@
 #endif
 #if defined(VIDEO_HOLE)
       contains_media_player_(false),
 #endif
       geolocation_dispatcher_(NULL),
       push_messaging_dispatcher_(NULL),
       screen_orientation_dispatcher_(NULL),
       manifest_manager_(NULL),
       accessibility_mode_(AccessibilityModeOff),
       renderer_accessibility_(NULL),
+      beforeunload_handlers_(0),
+      unload_handlers_(0),
       weak_factory_(this) {
   std::pair<RoutingIDFrameMap::iterator, bool> result =
       g_routing_id_frame_map.Get().insert(std::make_pair(routing_id_, this));
   CHECK(result.second) << "Inserting a duplicate item.";
 
   RenderThread::Get()->AddRoute(routing_id_, this);
 
   render_view_->RegisterRenderFrame(this);
 
   // Everything below subclasses RenderFrameObserver and is automatically
@@ skipping 2867 matching lines @@
 bool RenderFrameImpl::enterFullscreen() {
   Send(new FrameHostMsg_ToggleFullscreen(routing_id_, true));
   return true;
 }
 
 bool RenderFrameImpl::exitFullscreen() {
   Send(new FrameHostMsg_ToggleFullscreen(routing_id_, false));
   return true;
 }
 
+void RenderFrameImpl::suddenTerminationDisablerChanged(

[Charlie Reis, 2015/01/23 06:01:32]

This seems unnecessarily complex.  Why do we need to track counters in
RenderFrameImpl?  We could avoid storing any state here (and the complex
HandleSuddenTerminationDisablerChange method) if Blink did that work for us.

In other words, just have Blink tell WebFrameClient if it goes between zero and
non-zero handlers for beforeunload or unload.  Should be easy, since it's just
checking set.isEmpty() before and after.

Then RenderFrameImpl just passes that message on to RFH without any extra logic
needed.

[clamy, 2015/01/26 16:38:40]

Done.

+    int variation,
+    blink::WebFrameClient::SuddenTerminationDisablerType type) {
+  int* relevant_counter;
+  switch (type) {
+    case blink::WebFrameClient::BeforeUnloadHandler:
+      relevant_counter = &beforeunload_handlers_;
+      break;
+    case blink::WebFrameClient::UnloadHandler:
+      relevant_counter = &unload_handlers_;
+      break;
+    default:
+      NOTREACHED();
+  }
+  DCHECK(relevant_counter);
+
+  bool presence_changed = HandleSuddenTerminationDisablerChange(
+      relevant_counter, variation);
+  if (!presence_changed)
+    return;
+
+  // The presence status of one of the sudden termination disablers changed,
+  // inform the browser.
+  switch (type) {
+    case blink::WebFrameClient::BeforeUnloadHandler:
+      Send(new FrameHostMsg_BeforeUnloadHandlersPresenceChanged(
+          routing_id_, beforeunload_handlers_));

[Charlie Reis, 2015/01/23 06:01:32]

This is a count, not a bool.  Prefix with !! to make the intention clear (and it
might be necessary for compiling on Windows anyway).

[clamy, 2015/01/26 16:38:40]

Switched to booleans.

+      break;
+    case blink::WebFrameClient::UnloadHandler:
+      Send(new FrameHostMsg_UnloadHandlersPresenceChanged(
+          routing_id_, unload_handlers_));
+      break;
+    default:
+      NOTREACHED();
+  }
+}
+
 void RenderFrameImpl::DidPlay(blink::WebMediaPlayer* player) {
   Send(new FrameHostMsg_MediaPlayingNotification(
       routing_id_, reinterpret_cast<int64>(player), player->hasVideo(),
       player->hasAudio(), player->isRemote()));
 }
 
 void RenderFrameImpl::DidPause(blink::WebMediaPlayer* player) {
   Send(new FrameHostMsg_MediaPausedNotification(
       routing_id_, reinterpret_cast<int64>(player)));
 }
@@ skipping 850 matching lines @@
 
 #if defined(ENABLE_BROWSER_CDMS)
 RendererCdmManager* RenderFrameImpl::GetCdmManager() {
   if (!cdm_manager_)
     cdm_manager_ = new RendererCdmManager(this);
   return cdm_manager_;
 }
 #endif  // defined(ENABLE_BROWSER_CDMS)
 
 }  // namespace content