Close URL requests associated with an instance when that instance goes away.

Not doing this causes a bug in shutdown (wehn we can't do fast shutdown and just kill the renderer).

The problem is that if a URLLoader is open (like maybe it's leaked by the plugin), the ResourceTracker will be the one to delete it. The ResourceTracker is a singleton which is deleted in the AtExit manager. If a URLLoader is deleted from there we'll crash trying to delete the corresponding WebKit loader which no longer exists (because the page associated with it went away).

BUG=61795
TEST=none
Review URL: http://codereview.chromium.org/4690003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@65501 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 110 insertions, 15 deletions

diff --git a/webkit/glue/plugins/pepper_plugin_instance.cc b/webkit/glue/plugins/pepper_plugin_instance.cc
index 8a20e74..47ea900 100644
--- a/webkit/glue/plugins/pepper_plugin_instance.cc
+++ b/webkit/glue/plugins/pepper_plugin_instance.cc
@@ -311,6 +311,8 @@ PluginInstance::PluginInstance(PluginDelegate* delegate,
 }
 
 PluginInstance::~PluginInstance() {
+  FOR_EACH_OBSERVER(Observer, observers_, InstanceDestroyed(this));
+
   delegate_->InstanceDeleted(this);
   module_->InstanceDeleted(this);
 
@@ -337,6 +339,14 @@ const PPB_Zoom_Dev* PluginInstance::GetZoomInterface() {
   return &ppb_zoom;
 }
 
+void PluginInstance::AddObserver(Observer* observer) {
+  observers_.AddObserver(observer);
+}
+
+void PluginInstance::RemoveObserver(Observer* observer) {
+  observers_.RemoveObserver(observer);
+}
+
 void PluginInstance::Paint(WebCanvas* canvas,
                            const gfx::Rect& plugin_rect,
                            const gfx::Rect& paint_rect) {
diff --git a/webkit/glue/plugins/pepper_plugin_instance.h b/webkit/glue/plugins/pepper_plugin_instance.h
index 4ee92b7..bd99843 100644
--- a/webkit/glue/plugins/pepper_plugin_instance.h
+++ b/webkit/glue/plugins/pepper_plugin_instance.h
@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/observer_list.h"
 #include "base/ref_counted.h"
 #include "base/scoped_ptr.h"
 #include "base/string16.h"
@@ -62,6 +63,14 @@ class FullscreenContainer;
 // ResourceTracker.
 class PluginInstance : public base::RefCounted<PluginInstance> {
  public:
+  class Observer {
+   public:
+    // Indicates that the instance is being destroyed. This will be called from
+    // the instance's destructor so don't do anything in this callback that
+    // uses the instance.
+    virtual void InstanceDestroyed(PluginInstance* instance) = 0;
+  };
+
   PluginInstance(PluginDelegate* delegate,
                  PluginModule* module,
                  const PPP_Instance* instance_interface);
@@ -91,6 +100,12 @@ class PluginInstance : public base::RefCounted<PluginInstance> {
   // nonzero.
   PP_Instance pp_instance() const { return pp_instance_; }
 
+  // Other classes can register an observer for instance events. These pointers
+  // are NOT owned by the Instance. If the object implementing the observer
+  // goes away, it must take care to unregister itself.
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
   // Paints the current backing store to the web page.
   void Paint(WebKit::WebCanvas* canvas,
              const gfx::Rect& plugin_rect,
@@ -292,6 +307,9 @@ class PluginInstance : public base::RefCounted<PluginInstance> {
   // Plugin container for fullscreen mode. NULL if not in fullscreen mode.
   FullscreenContainer* fullscreen_container_;
 
+  // Non-owning pointers to all active observers.
+  ObserverList<Observer, false> observers_;
+
   DISALLOW_COPY_AND_ASSIGN(PluginInstance);
 };
 
diff --git a/webkit/glue/plugins/pepper_resource.h b/webkit/glue/plugins/pepper_resource.h
index 2436ebd..3c4ebca 100644
--- a/webkit/glue/plugins/pepper_resource.h
+++ b/webkit/glue/plugins/pepper_resource.h
@@ -70,6 +70,18 @@ class Resource : public base::RefCountedThreadSafe<Resource> {
   // reference to the plugin.
   PP_Resource GetReference();
 
+  // Returns the resource ID of this object OR NULL IF THERE IS NONE ASSIGNED.
+  // This will happen if the plugin doesn't have a reference to the given
+  // resource. The resource will not be addref'ed.
+  //
+  // This should only be used as an input parameter to the plugin for status
+  // updates in the proxy layer, where if the plugin has no reference, it will
+  // just give up since nothing needs to be updated.
+  //
+  // Generally you should use GetReference instead. This is why it has this
+  // obscure name rather than pp_resource().
+  PP_Resource GetReferenceNoAddRef() const;
+
   // When you need to ensure that a resource has a reference, but you do not
   // want to increase the refcount (for example, if you need to call a plugin
   // callback function with a reference), you can use this class. For example:
@@ -85,19 +97,6 @@ class Resource : public base::RefCountedThreadSafe<Resource> {
     const PP_Resource id;
   };
 
- protected:
-  // Returns the resource ID of this object OR NULL IF THERE IS NONE ASSIGNED.
-  // This will happen if the plugin doesn't have a reference to the given
-  // resource. The resource will not be addref'ed.
-  //
-  // This should only be used as an input parameter to the plugin for status
-  // updates in the proxy layer, where if the plugin has no reference, it will
-  // just give up since nothing needs to be updated.
-  //
-  // Generally you should use GetReference instead. This is why it has this
-  // obscure name rather than pp_resource().
-  PP_Resource GetReferenceNoAddRef() const;
-
  private:
   // Type-specific getters for individual resource types. These will return
   // NULL if the resource does not match the specified type. Used by the Cast()
diff --git a/webkit/glue/plugins/pepper_resource_tracker.cc b/webkit/glue/plugins/pepper_resource_tracker.cc
index 90a5ece..ba6f8f0 100644
--- a/webkit/glue/plugins/pepper_resource_tracker.cc
+++ b/webkit/glue/plugins/pepper_resource_tracker.cc
@@ -66,6 +66,16 @@ bool ResourceTracker::UnrefResource(PP_Resource res) {
   }
 }
 
+void ResourceTracker::ForceDeletePluginResourceRefs(PP_Resource res) {
+  ResourceMap::iterator i = live_resources_.find(res);
+  if (i != live_resources_.end())
+    return;  // Nothing to do.
+
+  i->second.second = 0;
+  i->second.first->StoppedTracking();
+  live_resources_.erase(i);
+}
+
 uint32 ResourceTracker::GetLiveObjectsForModule(PluginModule* module) const {
   // Since this is for testing only, we'll just go through all of them and
   // count.
diff --git a/webkit/glue/plugins/pepper_resource_tracker.h b/webkit/glue/plugins/pepper_resource_tracker.h
index e2aec73..ad25d1a 100644
--- a/webkit/glue/plugins/pepper_resource_tracker.h
+++ b/webkit/glue/plugins/pepper_resource_tracker.h
@@ -47,6 +47,16 @@ class ResourceTracker {
   bool AddRefResource(PP_Resource res);
   bool UnrefResource(PP_Resource res);
 
+  // Forces the plugin refcount of the given resource to 0. This can be used to
+  // delete an object the plugin has leaked or whose lifetime is otherwise
+  // exceeded.
+  //
+  // Note that this may not necessarily delete the resource object since the
+  // regular refcount is maintained separately from the plugin refcount and
+  // random components in the Pepper implementation could still have
+  // references to it.
+  void ForceDeletePluginResourceRefs(PP_Resource res);
+
   // Returns the number of resources associated with this module.
   //
   // This is slow, use only for testing.
diff --git a/webkit/glue/plugins/pepper_url_loader.cc b/webkit/glue/plugins/pepper_url_loader.cc
index cb7cb5f..c9a580a 100644
--- a/webkit/glue/plugins/pepper_url_loader.cc
+++ b/webkit/glue/plugins/pepper_url_loader.cc
@@ -198,9 +198,12 @@ URLLoader::URLLoader(PluginInstance* instance, bool main_document_loader)
       record_upload_progress_(false),
       has_universal_access_(false),
       status_callback_(NULL) {
+  instance->AddObserver(this);
 }
 
 URLLoader::~URLLoader() {
+  if (instance_)
+    instance_->RemoveObserver(this);
 }
 
 // static
@@ -425,6 +428,40 @@ void URLLoader::didFail(WebURLLoader* loader, const WebURLError& error) {
   RunCallback(done_status_);
 }
 
+void URLLoader::InstanceDestroyed(PluginInstance* instance) {
+  // When the instance is destroyed, we force delete any associated loads.
+  DCHECK(instance == instance_);
+  instance_ = NULL;
+
+  // Normally the only ref to this class will be from the plugin which
+  // ForceDeletePluginResourceRefs will free. We don't want our object to be
+  // deleted out from under us until the function completes.
+  scoped_refptr<URLLoader> death_grip(this);
+
+  // Force delete any plugin refs to us. If the instance is being deleted, we
+  // don't want to allow the requests to continue to use bandwidth and send us
+  // callbacks (for which we might have no plugin).
+  ResourceTracker *tracker = ResourceTracker::Get();
+  PP_Resource loader_resource = GetReferenceNoAddRef();
+  if (loader_resource)
+    tracker->ForceDeletePluginResourceRefs(loader_resource);
+
+  // Also force free the response from the plugin, both the plugin's ref(s)
+  // and ours.
+  if (response_info_.get()) {
+    PP_Resource response_info_resource = response_info_->GetReferenceNoAddRef();
+    if (response_info_resource)
+      tracker->ForceDeletePluginResourceRefs(response_info_resource);
+    response_info_ = NULL;
+  }
+
+  // Free the WebKit request.
+  loader_.reset();
+
+  // Often, |this| will be deleted at the end of this function when death_grip
+  // goes out of scope.
+}
+
 void URLLoader::RunCallback(int32_t result) {
   if (!pending_callback_.func)
     return;
diff --git a/webkit/glue/plugins/pepper_url_loader.h b/webkit/glue/plugins/pepper_url_loader.h
index fbeb163..dd91708 100644
--- a/webkit/glue/plugins/pepper_url_loader.h
+++ b/webkit/glue/plugins/pepper_url_loader.h
@@ -11,6 +11,7 @@
 #include "ppapi/c/pp_completion_callback.h"
 #include "ppapi/c/dev/ppb_url_loader_trusted_dev.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebURLLoaderClient.h"
+#include "webkit/glue/plugins/pepper_plugin_instance.h"
 #include "webkit/glue/plugins/pepper_resource.h"
 
 struct PPB_URLLoader_Dev;
@@ -27,7 +28,9 @@ class PluginInstance;
 class URLRequestInfo;
 class URLResponseInfo;
 
-class URLLoader : public Resource, public WebKit::WebURLLoaderClient {
+class URLLoader : public Resource,
+                  public WebKit::WebURLLoaderClient,
+                  public PluginInstance::Observer {
  public:
   URLLoader(PluginInstance* instance, bool main_document_loader);
   virtual ~URLLoader();
@@ -78,6 +81,9 @@ class URLLoader : public Resource, public WebKit::WebURLLoaderClient {
   virtual void didFail(WebKit::WebURLLoader* loader,
                        const WebKit::WebURLError& error);
 
+  // PluginInstance::Observer implementation.
+  void InstanceDestroyed(PluginInstance* instance);
+
   URLResponseInfo* response_info() const { return response_info_; }
 
  private:
@@ -94,7 +100,12 @@ class URLLoader : public Resource, public WebKit::WebURLLoaderClient {
   // synchronize an out-of-process plugin's state.
   void UpdateStatus();
 
-  scoped_refptr<PluginInstance> instance_;
+  // This will be NULL if the instance has been deleted but this URLLoader was
+  // somehow leaked. In general, you should not need to check this for NULL.
+  // However, if you see a NULL pointer crash, that means somebody is holding
+  // a reference to this object longer than the PluginInstance's lifetime.
+  PluginInstance* instance_;
+
   // If true, then the plugin instance is a full-frame plugin and we're just
   // wrapping the main document's loader (i.e. loader_ is null).
   bool main_document_loader_;