Deal with frame removal by content scripts

extensions/renderer/dispatcher.cc

 // Copyright 2014 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 "extensions/renderer/dispatcher.h"
 
 #include <stddef.h>
 
 #include <utility>
 
@@ skipping 482 matching lines @@
   }
 
   // In testing, the document lifetime events can happen after the render
   // process shutdown event.
   // See: http://crbug.com/21508 and http://crbug.com/500851
   if (content_watcher_) {
     content_watcher_->DidCreateDocumentElement(frame);
   }
 }
 
+void Dispatcher::AfterDidCreateDocumentElement(
+    const base::WeakPtr<content::RenderFrame>& render_frame) {
+  DCHECK(render_frame.get());
+  ExtensionFrameHelper* frame_helper =
+      ExtensionFrameHelper::Get(render_frame.get());

[ncarter (slow), 2016/02/11 22:47:32]

Nasko pointed me at this CL to see if there might be an alternative to minting
the WeakPtr on the content side and passing it through here. 

One idea would be to add a WeakPtrFactory<> to the ExtensionFrameHelper (whose
lifetime, I think, is bound to the renderframe). This would let the whole
WeakPtr dance be contained entirely inside the //extensions layer.

[robwu, 2016/02/11 23:36:50]

Done. I don't see any issue with passing a weakptr down from content/ (except
that the creation of the weak ptr is unnecessary if extensions are disabled),
but it's your call.

[ncarter (slow), 2016/02/12 19:32:57]

I'm not opposed to it per se either, but I think it's cleaner if we handle the
deletion-detection at the same layer that the deletion-triggering happens.

My reasoning is that passing a weakptr as an in-param fairly uncommon idiom
outside of ctors and functions used as callbacks (where time might pass between
the bind and the invocation). Uncommon idioms result in code that's harder to
understand: in the case of the content client calls, when I first saw them, I
wondered "Why would content need to pass a WeakPtr here? Is it possible the
frame has already been deleted? If so why would content be reporting an event on
the deleted frame?"

If RenderFrameHost had already exposed a public GetWeakPtr<> method I would have
had no trouble with you using that. But it wasn't there already, and since there
is a way to get the behavior you need without adding a method to
RenderFrameHost, that's what I suggested.

[robwu, 2016/02/12 19:51:43]

That makes sense. Thanks for explaining your line of reasoning and review!

+  if (!frame_helper)
+    return;  // The frame is invisible to extensions.
+
+  script_injection_manager_->AfterDidCreateDocumentElement(render_frame.get());
+  if (!render_frame.get())
+    return;  // Frame was destroyed by injected user script.
+
+  frame_helper->AfterDidCreateDocumentElement(render_frame);
+}
+
+void Dispatcher::AfterDidFinishDocumentLoad(
+    const base::WeakPtr<content::RenderFrame>& render_frame) {
+  if (!ExtensionFrameHelper::Get(render_frame.get()))
+    return;  // The frame is invisible to extensions.
+
+  script_injection_manager_->AfterDidFinishDocumentLoad(render_frame.get());
+  // The frame might be destroyed by the injected user script at this point.
+}
+
 void Dispatcher::OnExtensionResponse(int request_id,
                                      bool success,
                                      const base::ListValue& response,
                                      const std::string& error) {
   request_sender_->HandleResponse(request_id, success, response, error);
 }
 
 void Dispatcher::DispatchEvent(const std::string& extension_id,
                                const std::string& event_name) const {
   base::ListValue args;
@@ skipping 1099 matching lines @@
   // The "guestViewDeny" module must always be loaded last. It registers
   // error-providing custom elements for the GuestView types that are not
   // available, and thus all of those types must have been checked and loaded
   // (or not loaded) beforehand.
   if (context_type == Feature::BLESSED_EXTENSION_CONTEXT) {
     module_system->Require("guestViewDeny");
   }
 }
 
 }  // namespace extensions