Fix UaF in RenderFrameImpl::OnBeforeUnload.

content/renderer/render_view_browsertest.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 <stddef.h>
 #include <stdint.h>
 #include <tuple>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 2167 matching lines @@
   request_params.pending_history_list_offset = 2;
   frame()->Navigate(CommonNavigationParams(), StartNavigationParams(),
                     request_params);
 
   // The history list in RenderView should have been updated.
   EXPECT_EQ(1, view()->historyBackListCount());
   EXPECT_EQ(2, view()->historyBackListCount() +
       view()->historyForwardListCount() + 1);
 }
 
+class ConsoleCallbackFilter : public IPC::Listener {
+ public:
+  explicit ConsoleCallbackFilter(
+      base::Callback<void(const base::string16&)> callback)
+      : callback_(callback) {}
+
+  bool OnMessageReceived(const IPC::Message& msg) override {
+    bool handled = true;
+    IPC_BEGIN_MESSAGE_MAP(ConsoleCallbackFilter, msg)
+      IPC_MESSAGE_HANDLER(FrameHostMsg_DidAddMessageToConsole,
+                          OnDidAddMessageToConsole)
+      IPC_MESSAGE_UNHANDLED(handled = false)
+    IPC_END_MESSAGE_MAP()
+    return handled;
+  }
+
+  void OnDidAddMessageToConsole(int32_t,
+                                const base::string16& message,
+                                int32_t,
+                                const base::string16&) {
+    callback_.Run(message);
+  }
+
+ private:
+  base::Callback<void(const base::string16&)> callback_;
+};
+
+// Tests that there's no UaF after dispatchBeforeUnloadEvent.
+// See https://crbug.com/666714.
+TEST_F(RenderViewImplTest, DispatchBeforeUnloadCanDetachFrame) {
+  LoadHTML(
+      "<script>window.onbeforeunload = function() { "
+      "window.console.log('OnBeforeUnload called'); }</script>");
+  std::unique_ptr<ConsoleCallbackFilter> callback_filter(

[Charlie Reis, 2016/11/23 07:07:00]

Nice.  Let's add a comment explaining what this is doing.

[lfg, 2016/11/23 16:48:02]

Done. Can you do a quick sanity check to make sure the comments make sense?

[Charlie Reis, 2016/11/23 17:47:11]

Thanks-- looks good.

+      new ConsoleCallbackFilter(base::Bind(
+          [](RenderFrameImpl* frame, const base::string16& msg) {
+            EXPECT_EQ(base::UTF8ToUTF16("OnBeforeUnload called"), msg);
+            frame->OnMessageReceived(FrameMsg_SwapOut(
+                frame->GetRoutingID(), 1, false, FrameReplicationState()));
+          },
+          base::Unretained(frame()))));
+  render_thread_->sink().AddFilter(callback_filter.get());
+  frame()->OnMessageReceived(
+      FrameMsg_BeforeUnload(frame()->GetRoutingID(), false));
+  render_thread_->sink().RemoveFilter(callback_filter.get());
+}
+
 TEST_F(RenderViewImplBlinkSettingsTest, Default) {
   DoSetUp();
   EXPECT_FALSE(settings()->viewportEnabled());
 }
 
 TEST_F(RenderViewImplBlinkSettingsTest, CommandLine) {
   base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
       switches::kBlinkSettings,
       "multiTargetTapNotificationEnabled=true,viewportEnabled=true");
   DoSetUp();
@@ skipping 294 matching lines @@
   ExpectPauseAndResume(3);
   blink::WebScriptSource source2(
       WebString::fromUTF8("function func2() { func1(); }; func2();"));
   frame()->GetWebFrame()->executeScriptInIsolatedWorld(17, &source2, 1, 1);
 
   EXPECT_FALSE(IsPaused());
   Detach();
 }
 
 }  // namespace content