diff options
| -rw-r--r-- | ppapi/ppapi_shared.gypi | 4 | ||||
| -rw-r--r-- | ppapi/ppapi_tests.gypi | 1 | ||||
| -rw-r--r-- | ppapi/proxy/plugin_globals.cc | 10 | ||||
| -rw-r--r-- | ppapi/proxy/plugin_globals.h | 4 | ||||
| -rw-r--r-- | ppapi/shared_impl/callback_tracker.cc | 75 | ||||
| -rw-r--r-- | ppapi/shared_impl/callback_tracker.h | 101 | ||||
| -rw-r--r-- | ppapi/shared_impl/ppapi_globals.h | 3 | ||||
| -rw-r--r-- | ppapi/shared_impl/resource_tracker.cc | 4 | ||||
| -rw-r--r-- | ppapi/shared_impl/test_globals.cc | 9 | ||||
| -rw-r--r-- | ppapi/shared_impl/test_globals.h | 4 | ||||
| -rw-r--r-- | ppapi/shared_impl/tracked_callback.cc | 87 | ||||
| -rw-r--r-- | ppapi/shared_impl/tracked_callback.h | 106 | ||||
| -rw-r--r-- | ppapi/shared_impl/tracked_callback_unittest.cc | 266 | ||||
| -rw-r--r-- | webkit/plugins/ppapi/host_globals.cc | 10 | ||||
| -rw-r--r-- | webkit/plugins/ppapi/host_globals.h | 3 | ||||
| -rw-r--r-- | webkit/plugins/ppapi/plugin_module.cc | 9 | ||||
| -rw-r--r-- | webkit/plugins/ppapi/plugin_module.h | 8 |
17 files changed, 700 insertions, 4 deletions
diff --git a/ppapi/ppapi_shared.gypi b/ppapi/ppapi_shared.gypi index f7a01d9..a497a63 100644 --- a/ppapi/ppapi_shared.gypi +++ b/ppapi/ppapi_shared.gypi @@ -30,6 +30,8 @@ '../base/base.gyp:base', ], 'sources': [ + 'shared_impl/callback_tracker.cc', + 'shared_impl/callback_tracker.h', 'shared_impl/file_type_conversion.cc', 'shared_impl/file_type_conversion.h', 'shared_impl/function_group_base.cc', @@ -89,6 +91,8 @@ 'shared_impl/scoped_pp_resource.h', 'shared_impl/time_conversion.cc', 'shared_impl/time_conversion.h', + 'shared_impl/tracked_callback.cc', + 'shared_impl/tracked_callback.h', 'shared_impl/var.cc', 'shared_impl/var.h', 'shared_impl/var_tracker.cc', diff --git a/ppapi/ppapi_tests.gypi b/ppapi/ppapi_tests.gypi index 14e3d3c..26d2527 100644 --- a/ppapi/ppapi_tests.gypi +++ b/ppapi/ppapi_tests.gypi @@ -152,6 +152,7 @@ 'shared_impl/resource_tracker_unittest.cc', 'shared_impl/test_globals.cc', 'shared_impl/test_globals.h', + 'shared_impl/tracked_callback_unittest.cc', 'shared_impl/var_tracker_unittest.cc', ], }, diff --git a/ppapi/proxy/plugin_globals.cc b/ppapi/proxy/plugin_globals.cc index e5acc4c0..664fbeb 100644 --- a/ppapi/proxy/plugin_globals.cc +++ b/ppapi/proxy/plugin_globals.cc @@ -13,7 +13,8 @@ PluginGlobals* PluginGlobals::plugin_globals_ = NULL; PluginGlobals::PluginGlobals() : ppapi::PpapiGlobals(), - plugin_proxy_delegate_(NULL) { + plugin_proxy_delegate_(NULL), + callback_tracker_(new CallbackTracker) { DCHECK(!plugin_globals_); plugin_globals_ = this; } @@ -31,6 +32,13 @@ VarTracker* PluginGlobals::GetVarTracker() { return &plugin_var_tracker_; } +CallbackTracker* PluginGlobals::GetCallbackTrackerForInstance( + PP_Instance instance) { + // In the plugin process, the callback tracker is always the same, regardless + // of the instance. + return callback_tracker_.get(); +} + FunctionGroupBase* PluginGlobals::GetFunctionAPI(PP_Instance inst, ApiID id) { PluginDispatcher* dispatcher = PluginDispatcher::GetForInstance(inst); if (dispatcher) diff --git a/ppapi/proxy/plugin_globals.h b/ppapi/proxy/plugin_globals.h index a76b7f0..ffcf01e 100644 --- a/ppapi/proxy/plugin_globals.h +++ b/ppapi/proxy/plugin_globals.h @@ -9,6 +9,7 @@ #include "ppapi/proxy/plugin_resource_tracker.h" #include "ppapi/proxy/plugin_var_tracker.h" #include "ppapi/proxy/ppapi_proxy_export.h" +#include "ppapi/shared_impl/callback_tracker.h" #include "ppapi/shared_impl/ppapi_globals.h" namespace ppapi { @@ -29,6 +30,8 @@ class PPAPI_PROXY_EXPORT PluginGlobals : public PpapiGlobals { // PpapiGlobals implementation. virtual ResourceTracker* GetResourceTracker() OVERRIDE; virtual VarTracker* GetVarTracker() OVERRIDE; + virtual CallbackTracker* GetCallbackTrackerForInstance( + PP_Instance instance) OVERRIDE; virtual FunctionGroupBase* GetFunctionAPI(PP_Instance inst, ApiID id) OVERRIDE; virtual PP_Module GetModuleForInstance(PP_Instance instance) OVERRIDE; @@ -55,6 +58,7 @@ class PPAPI_PROXY_EXPORT PluginGlobals : public PpapiGlobals { PluginProxyDelegate* plugin_proxy_delegate_; PluginResourceTracker plugin_resource_tracker_; PluginVarTracker plugin_var_tracker_; + scoped_refptr<CallbackTracker> callback_tracker_; DISALLOW_COPY_AND_ASSIGN(PluginGlobals); }; diff --git a/ppapi/shared_impl/callback_tracker.cc b/ppapi/shared_impl/callback_tracker.cc new file mode 100644 index 0000000..1f22119 --- /dev/null +++ b/ppapi/shared_impl/callback_tracker.cc @@ -0,0 +1,75 @@ +// Copyright (c) 2010 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 "ppapi/shared_impl/callback_tracker.h" + +#include <algorithm> + +#include "base/bind.h" +#include "base/compiler_specific.h" +#include "base/logging.h" +#include "base/message_loop.h" +#include "ppapi/c/pp_completion_callback.h" +#include "ppapi/shared_impl/tracked_callback.h" + +namespace ppapi { + +// CallbackTracker ------------------------------------------------------------- + +CallbackTracker::CallbackTracker() { +} + +void CallbackTracker::AbortAll() { + // Iterate over a copy since |Abort()| calls |Remove()| (indirectly). + // TODO(viettrungluu): This obviously isn't so efficient. + CallbackSetMap pending_callbacks_copy = pending_callbacks_; + for (CallbackSetMap::iterator it1 = pending_callbacks_copy.begin(); + it1 != pending_callbacks_copy.end(); ++it1) { + for (CallbackSet::iterator it2 = it1->second.begin(); + it2 != it1->second.end(); ++it2) { + (*it2)->Abort(); + } + } +} + +void CallbackTracker::PostAbortForResource(PP_Resource resource_id) { + CHECK(resource_id != 0); + CallbackSetMap::iterator it1 = pending_callbacks_.find(resource_id); + if (it1 == pending_callbacks_.end()) + return; + for (CallbackSet::iterator it2 = it1->second.begin(); + it2 != it1->second.end(); ++it2) { + // Post the abort. + (*it2)->PostAbort(); + } +} + +CallbackTracker::~CallbackTracker() { + // All callbacks must be aborted before destruction. + CHECK_EQ(0u, pending_callbacks_.size()); +} + +void CallbackTracker::Add( + const scoped_refptr<TrackedCallback>& tracked_callback) { + PP_Resource resource_id = tracked_callback->resource_id(); + DCHECK(pending_callbacks_[resource_id].find(tracked_callback) == + pending_callbacks_[resource_id].end()); + pending_callbacks_[resource_id].insert(tracked_callback); +} + +void CallbackTracker::Remove( + const scoped_refptr<TrackedCallback>& tracked_callback) { + CallbackSetMap::iterator map_it = + pending_callbacks_.find(tracked_callback->resource_id()); + DCHECK(map_it != pending_callbacks_.end()); + CallbackSet::iterator it = map_it->second.find(tracked_callback); + DCHECK(it != map_it->second.end()); + map_it->second.erase(it); + + // If there are no pending callbacks left for this ID, get rid of the entry. + if (map_it->second.empty()) + pending_callbacks_.erase(map_it); +} + +} // namespace ppapi diff --git a/ppapi/shared_impl/callback_tracker.h b/ppapi/shared_impl/callback_tracker.h new file mode 100644 index 0000000..5e354fb --- /dev/null +++ b/ppapi/shared_impl/callback_tracker.h @@ -0,0 +1,101 @@ +// Copyright (c) 2011 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. + +#ifndef PPAPI_SHARED_IMPL_CALLBACK_TRACKER_H_ +#define PPAPI_SHARED_IMPL_CALLBACK_TRACKER_H_ + +#include <map> +#include <set> + +#include "base/basictypes.h" +#include "base/memory/ref_counted.h" +#include "base/task.h" +#include "ppapi/c/pp_resource.h" +#include "ppapi/shared_impl/ppapi_shared_export.h" + +namespace ppapi { + +class TrackedCallback; + +// Pepper callbacks have the following semantics (unless otherwise specified; +// in particular, the below apply to all completion callbacks): +// - Callbacks are always run on the main thread. +// - Callbacks are always called from the main message loop. In particular, +// calling into Pepper will not result in the plugin being re-entered via a +// synchronously-run callback. +// - Each callback will be executed (a.k.a. completed) exactly once. +// - Each callback may be *aborted*, which means that it will be executed with +// result |PP_ERROR_ABORTED| (in the case of completion callbacks). +// - Before |PPP_ShutdownModule()| is called, every pending callback (for that +// module) will be aborted. +// - Callbacks are usually associated to a resource, whose "deletion" provides +// a "cancellation" (or abort) mechanism -- see below. +// - When a plugin releases its last reference to resource, all callbacks +// associated to that resource are aborted. Even if a non-abortive completion +// of such a callback had previously been scheduled (i.e., posted), that +// callback must now be aborted. The aborts should be scheduled immediately +// (upon the last reference being given up) and should not rely on anything +// else (e.g., a background task to complete or further action from the +// plugin). +// - Abortive completion gives no information about the status of the +// asynchronous operation: The operation may have not yet begun, may be in +// progress, or may be completed (successfully or not). In fact, the +// operation may still proceed after the callback has been aborted. +// - Any output data buffers provided to Pepper are associated with a resource. +// Once that resource is released, no subsequent writes to those buffers. (If +// background threads are set up to write into such buffers, the final +// release operation should not return into the plugin until it can +// guaranteed that those threads will no longer write into the buffers.) +// +// Thread-safety notes: +// Currently, everything should happen on the main thread. The objects are +// thread-safe ref-counted, so objects which live on different threads may keep +// references. Releasing a reference to |TrackedCallback| on a different thread +// (possibly causing destruction) is also okay. Otherwise, all methods should be +// called only from the main thread. + +// |CallbackTracker| tracks pending Pepper callbacks for a single module. It +// also tracks, for each resource ID, which callbacks are pending. When a +// callback is (just about to be) completed, it is removed from the tracker. We +// use |CallbackTracker| for two things: (1) to ensure that all callbacks are +// properly aborted before module shutdown, and (2) to ensure that all callbacks +// associated to a given resource are aborted when a plugin (module) releases +// its last reference to that resource. +class PPAPI_SHARED_EXPORT CallbackTracker + : public base::RefCountedThreadSafe<CallbackTracker> { + public: + CallbackTracker(); + + // Abort all callbacks (synchronously). + void AbortAll(); + + // Abort all callbacks associated to the given resource ID (which must be + // valid, i.e., nonzero) by posting a task (or tasks). + void PostAbortForResource(PP_Resource resource_id); + + private: + friend class base::RefCountedThreadSafe<CallbackTracker>; + ~CallbackTracker(); + + // |TrackedCallback| are expected to automatically add and + // remove themselves from their provided |CallbackTracker|. + friend class TrackedCallback; + void Add(const scoped_refptr<TrackedCallback>& tracked_callback); + void Remove(const scoped_refptr<TrackedCallback>& tracked_callback); + + // For each resource ID with a pending callback, store a set with its pending + // callbacks. (Resource ID 0 is used for callbacks not associated to a valid + // resource.) If a resource ID is re-used for another resource, there may be + // aborted callbacks corresponding to the original resource in that set; these + // will be removed when they are completed (abortively). + typedef std::set<scoped_refptr<TrackedCallback> > CallbackSet; + typedef std::map<PP_Resource, CallbackSet> CallbackSetMap; + CallbackSetMap pending_callbacks_; + + DISALLOW_COPY_AND_ASSIGN(CallbackTracker); +}; + +} // namespace ppapi + +#endif // PPAPI_SHARED_IMPL_CALLBACK_TRACKER_H_ diff --git a/ppapi/shared_impl/ppapi_globals.h b/ppapi/shared_impl/ppapi_globals.h index 56ec6bf..b3d7d55 100644 --- a/ppapi/shared_impl/ppapi_globals.h +++ b/ppapi/shared_impl/ppapi_globals.h @@ -13,6 +13,7 @@ namespace ppapi { +class CallbackTracker; class FunctionGroupBase; class ResourceTracker; class VarTracker; @@ -29,6 +30,8 @@ class PPAPI_SHARED_EXPORT PpapiGlobals { // Retrieves the corresponding tracker. virtual ResourceTracker* GetResourceTracker() = 0; virtual VarTracker* GetVarTracker() = 0; + virtual CallbackTracker* GetCallbackTrackerForInstance( + PP_Instance instance) = 0; // Returns the function object corresponding to the given ID, or NULL if // there isn't one. diff --git a/ppapi/shared_impl/resource_tracker.cc b/ppapi/shared_impl/resource_tracker.cc index 2b3cb9f..3b66db8 100644 --- a/ppapi/shared_impl/resource_tracker.cc +++ b/ppapi/shared_impl/resource_tracker.cc @@ -4,7 +4,9 @@ #include "ppapi/shared_impl/resource_tracker.h" +#include "ppapi/shared_impl/callback_tracker.h" #include "ppapi/shared_impl/id_assignment.h" +#include "ppapi/shared_impl/ppapi_globals.h" #include "ppapi/shared_impl/resource.h" namespace ppapi { @@ -168,6 +170,8 @@ void ResourceTracker::RemoveResource(Resource* object) { } void ResourceTracker::LastPluginRefWasDeleted(Resource* object) { + PpapiGlobals::Get()->GetCallbackTrackerForInstance(object->pp_instance())-> + PostAbortForResource(object->pp_resource()); object->LastPluginRefWasDeleted(); } diff --git a/ppapi/shared_impl/test_globals.cc b/ppapi/shared_impl/test_globals.cc index 624b85b..e870e5c 100644 --- a/ppapi/shared_impl/test_globals.cc +++ b/ppapi/shared_impl/test_globals.cc @@ -6,7 +6,9 @@ namespace ppapi { -TestGlobals::TestGlobals() : ppapi::PpapiGlobals() { +TestGlobals::TestGlobals() + : ppapi::PpapiGlobals(), + callback_tracker_(new CallbackTracker) { } TestGlobals::~TestGlobals() { @@ -20,6 +22,11 @@ VarTracker* TestGlobals::GetVarTracker() { return &var_tracker_; } +CallbackTracker* TestGlobals::GetCallbackTrackerForInstance( + PP_Instance instance) { + return callback_tracker_.get(); +} + FunctionGroupBase* TestGlobals::GetFunctionAPI(PP_Instance inst, ApiID id) { return NULL; } diff --git a/ppapi/shared_impl/test_globals.h b/ppapi/shared_impl/test_globals.h index 49f189f..9c58fd3 100644 --- a/ppapi/shared_impl/test_globals.h +++ b/ppapi/shared_impl/test_globals.h @@ -6,6 +6,7 @@ #define PPAPI_SHARED_IMPL_TEST_GLOBALS_H_ #include "base/compiler_specific.h" +#include "ppapi/shared_impl/callback_tracker.h" #include "ppapi/shared_impl/ppapi_globals.h" #include "ppapi/shared_impl/resource_tracker.h" #include "ppapi/shared_impl/var_tracker.h" @@ -31,6 +32,8 @@ class TestGlobals : public PpapiGlobals { // PpapiGlobals implementation. virtual ResourceTracker* GetResourceTracker() OVERRIDE; virtual VarTracker* GetVarTracker() OVERRIDE; + virtual CallbackTracker* GetCallbackTrackerForInstance( + PP_Instance instance) OVERRIDE; virtual FunctionGroupBase* GetFunctionAPI(PP_Instance inst, ApiID id) OVERRIDE; virtual PP_Module GetModuleForInstance(PP_Instance instance) OVERRIDE; @@ -38,6 +41,7 @@ class TestGlobals : public PpapiGlobals { private: ResourceTracker resource_tracker_; TestVarTracker var_tracker_; + scoped_refptr<CallbackTracker> callback_tracker_; DISALLOW_COPY_AND_ASSIGN(TestGlobals); }; diff --git a/ppapi/shared_impl/tracked_callback.cc b/ppapi/shared_impl/tracked_callback.cc new file mode 100644 index 0000000..b36569d --- /dev/null +++ b/ppapi/shared_impl/tracked_callback.cc @@ -0,0 +1,87 @@ +// Copyright (c) 2010 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 "ppapi/shared_impl/tracked_callback.h" + +#include "base/bind.h" +#include "base/compiler_specific.h" +#include "base/logging.h" +#include "base/message_loop.h" +#include "ppapi/c/pp_completion_callback.h" +#include "ppapi/c/pp_errors.h" +#include "ppapi/shared_impl/callback_tracker.h" +#include "ppapi/shared_impl/ppapi_globals.h" +#include "ppapi/shared_impl/resource.h" + +namespace ppapi { + +// TrackedCallback ------------------------------------------------------------- + +// Note: don't keep a Resource* since it may go out of scope before us. +TrackedCallback::TrackedCallback( + Resource* resource, + const PP_CompletionCallback& callback) + : ALLOW_THIS_IN_INITIALIZER_LIST(abort_impl_factory_(this)), + resource_id_(resource->pp_resource()), + completed_(false), + aborted_(false), + callback_(callback) { + tracker_ = PpapiGlobals::Get()->GetCallbackTrackerForInstance( + resource->pp_instance()), + tracker_->Add(make_scoped_refptr(this)); +} + +TrackedCallback::~TrackedCallback() { +} + +void TrackedCallback::Abort() { + if (!completed()) { + aborted_ = true; + Run(PP_ERROR_ABORTED); + } +} + +void TrackedCallback::PostAbort() { + if (!completed()) { + aborted_ = true; + // Post a task for the abort (only if necessary). + if (!abort_impl_factory_.HasWeakPtrs()) { + MessageLoop::current()->PostTask( + FROM_HERE, + base::Bind(&TrackedCallback::Abort, + abort_impl_factory_.GetWeakPtr())); + } + } +} + +void TrackedCallback::Run(int32_t result) { + if (!completed()) { + // Cancel any pending calls. + abort_impl_factory_.InvalidateWeakPtrs(); + + // Copy |callback_| and look at |aborted()| now, since |MarkAsCompleted()| + // may delete us. + PP_CompletionCallback callback = callback_; + if (aborted()) + result = PP_ERROR_ABORTED; + + // Do this before running the callback in case of reentrancy (which + // shouldn't happen, but avoid strange failures). + MarkAsCompleted(); + PP_RunCompletionCallback(&callback, result); + } +} + +void TrackedCallback::MarkAsCompleted() { + DCHECK(!completed()); + + // We will be removed; maintain a reference to ensure we won't be deleted + // until we're done. + scoped_refptr<TrackedCallback> thiz = this; + completed_ = true; + tracker_->Remove(thiz); + tracker_ = NULL; +} + +} // namespace ppapi diff --git a/ppapi/shared_impl/tracked_callback.h b/ppapi/shared_impl/tracked_callback.h new file mode 100644 index 0000000..ed42a59 --- /dev/null +++ b/ppapi/shared_impl/tracked_callback.h @@ -0,0 +1,106 @@ +// Copyright (c) 2011 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. + +#ifndef PPAPI_SHARED_IMPL_TRACKED_CALLBACK_H_ +#define PPAPI_SHARED_IMPL_TRACKED_CALLBACK_H_ + +#include <map> +#include <set> + +#include "base/basictypes.h" +#include "base/memory/ref_counted.h" +#include "base/task.h" +#include "ppapi/c/pp_completion_callback.h" +#include "ppapi/c/pp_resource.h" +#include "ppapi/shared_impl/ppapi_shared_export.h" + +namespace ppapi { + +class CallbackTracker; +class Resource; + +// |TrackedCallback| represents a tracked Pepper callback (from the browser to +// the plugin), typically still pending. Such callbacks have the standard Pepper +// callback semantics. Execution (i.e., completion) of callbacks happens through +// objects of subclasses of |TrackedCallback|. Two things are ensured: (1) that +// the callback is executed at most once, and (2) once a callback is marked to +// be aborted, any subsequent completion is abortive (even if a non-abortive +// completion had previously been scheduled). +// +// The details of non-abortive completion depend on the type of callback (e.g., +// different parameters may be required), but basic abort functionality is core. +// The ability to post aborts is needed in many situations to ensure that the +// plugin is not re-entered into. (Note that posting a task to just run +// |Abort()| is different and not correct; calling |PostAbort()| additionally +// guarantees that all subsequent completions will be abortive.) +// +// This class is reference counted so that different things can hang on to it, +// and not worry too much about ensuring Pepper callback semantics. Note that +// the "owning" |CallbackTracker| will keep a reference until the callback is +// completed. +// +// Subclasses must do several things: +// - They must ensure that the callback is executed at most once (by looking at +// |completed()| before running the callback). +// - They must ensure that the callback is run abortively if it is marked as to +// be aborted (by looking at |aborted()| before running the callback). +// - They must call |MarkAsCompleted()| immediately before actually running the +// callback; see the comment for |MarkAsCompleted()| for a caveat. +class PPAPI_SHARED_EXPORT TrackedCallback + : public base::RefCountedThreadSafe<TrackedCallback> { + public: + // Create a tracked completion callback and register it with the tracker. The + // resource pointer is not stored. + TrackedCallback(Resource* resource, + const PP_CompletionCallback& callback); + + // These run the callback in an abortive manner, or post a task to do so (but + // immediately marking the callback as to be aborted). + void Abort(); + void PostAbort(); + + // Run the callback with the given result. If the callback had previously been + // marked as to be aborted (by |PostAbort()|), |result| will be ignored and + // the callback will be run with result |PP_ERROR_ABORTED|. + void Run(int32_t result); + + // Returns the ID of the resource which "owns" the callback, or 0 if the + // callback is not associated with any resource. + PP_Resource resource_id() const { return resource_id_; } + + // Returns true if the callback was completed (possibly aborted). + bool completed() const { return completed_; } + + // Returns true if the callback was or should be aborted; this will be the + // case whenever |Abort()| or |PostAbort()| is called before a non-abortive + // completion. + bool aborted() const { return aborted_; } + + private: + // This class is ref counted. + friend class base::RefCountedThreadSafe<TrackedCallback>; + virtual ~TrackedCallback(); + + // Mark this object as complete and remove it from the tracker. This must only + // be called once. Note that running this may result in this object being + // deleted (so keep a reference if it'll still be needed). + void MarkAsCompleted(); + + // Factory used by |PostAbort()|. Note that it's safe to cancel any pending + // posted aborts on destruction -- before it's destroyed, the "owning" + // |CallbackTracker| must have gone through and done (synchronous) |Abort()|s. + base::WeakPtrFactory<TrackedCallback> abort_impl_factory_; + + scoped_refptr<CallbackTracker> tracker_; + PP_Resource resource_id_; + bool completed_; + bool aborted_; + PP_CompletionCallback callback_; + + DISALLOW_IMPLICIT_CONSTRUCTORS(TrackedCallback); +}; + +} // namespace ppapi + +#endif // PPAPI_SHARED_IMPL_TRACKED_CALLBACK_H_ diff --git a/ppapi/shared_impl/tracked_callback_unittest.cc b/ppapi/shared_impl/tracked_callback_unittest.cc new file mode 100644 index 0000000..62da396 --- /dev/null +++ b/ppapi/shared_impl/tracked_callback_unittest.cc @@ -0,0 +1,266 @@ +// Copyright (c) 2011 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 "base/memory/ref_counted.h" +#include "base/message_loop.h" +#include "ppapi/c/pp_completion_callback.h" +#include "ppapi/c/pp_errors.h" +#include "ppapi/shared_impl/callback_tracker.h" +#include "ppapi/shared_impl/resource.h" +#include "ppapi/shared_impl/resource_tracker.h" +#include "ppapi/shared_impl/test_globals.h" +#include "ppapi/shared_impl/tracked_callback.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace ppapi { + +namespace { + +class TrackedCallbackTest : public testing::Test { + public: + TrackedCallbackTest() + : message_loop_(MessageLoop::TYPE_DEFAULT), + pp_instance_(1234) {} + + PP_Instance pp_instance() const { return pp_instance_; } + + virtual void SetUp() OVERRIDE { + globals_.GetResourceTracker()->DidCreateInstance(pp_instance_); + } + virtual void TearDown() OVERRIDE { + globals_.GetResourceTracker()->DidDeleteInstance(pp_instance_); + } + + private: + MessageLoop message_loop_; + TestGlobals globals_; + PP_Instance pp_instance_; +}; + +struct CallbackRunInfo { + // All valid results (PP_OK, PP_ERROR_...) are nonpositive. + CallbackRunInfo() : run_count(0), result(1) {} + unsigned run_count; + int32_t result; +}; + +void TestCallback(void* user_data, int32_t result) { + CallbackRunInfo* info = reinterpret_cast<CallbackRunInfo*>(user_data); + info->run_count++; + if (info->run_count == 1) + info->result = result; +} + +} // namespace + +// CallbackShutdownTest -------------------------------------------------------- + +namespace { + +class CallbackShutdownTest : public TrackedCallbackTest { + public: + CallbackShutdownTest() {} + + // Cases: + // (1) A callback which is run (so shouldn't be aborted on shutdown). + // (2) A callback which is aborted (so shouldn't be aborted on shutdown). + // (3) A callback which isn't run (so should be aborted on shutdown). + CallbackRunInfo& info_did_run() { return info_did_run_; } // (1) + CallbackRunInfo& info_did_abort() { return info_did_abort_; } // (2) + CallbackRunInfo& info_didnt_run() { return info_didnt_run_; } // (3) + + private: + CallbackRunInfo info_did_run_; + CallbackRunInfo info_did_abort_; + CallbackRunInfo info_didnt_run_; +}; + +} // namespace + +// Tests that callbacks are properly aborted on module shutdown. +TEST_F(CallbackShutdownTest, AbortOnShutdown) { + scoped_refptr<Resource> resource(new Resource(pp_instance())); + + // Set up case (1) (see above). + EXPECT_EQ(0U, info_did_run().run_count); + scoped_refptr<TrackedCallback> callback_did_run = new TrackedCallback( + resource.get(), + PP_MakeCompletionCallback(&TestCallback, &info_did_run())); + EXPECT_EQ(0U, info_did_run().run_count); + callback_did_run->Run(PP_OK); + EXPECT_EQ(1U, info_did_run().run_count); + EXPECT_EQ(PP_OK, info_did_run().result); + + // Set up case (2). + EXPECT_EQ(0U, info_did_abort().run_count); + scoped_refptr<TrackedCallback> callback_did_abort = new TrackedCallback( + resource.get(), + PP_MakeCompletionCallback(&TestCallback, &info_did_abort())); + EXPECT_EQ(0U, info_did_abort().run_count); + callback_did_abort->Abort(); + EXPECT_EQ(1U, info_did_abort().run_count); + EXPECT_EQ(PP_ERROR_ABORTED, info_did_abort().result); + + // Set up case (3). + EXPECT_EQ(0U, info_didnt_run().run_count); + scoped_refptr<TrackedCallback> callback_didnt_run = new TrackedCallback( + resource.get(), + PP_MakeCompletionCallback(&TestCallback, &info_didnt_run())); + EXPECT_EQ(0U, info_didnt_run().run_count); + + PpapiGlobals::Get()->GetCallbackTrackerForInstance(pp_instance())->AbortAll(); + + // Check case (1). + EXPECT_EQ(1U, info_did_run().run_count); + + // Check case (2). + EXPECT_EQ(1U, info_did_abort().run_count); + + // Check case (3). + EXPECT_EQ(1U, info_didnt_run().run_count); + EXPECT_EQ(PP_ERROR_ABORTED, info_didnt_run().result); +} + +// CallbackResourceTest -------------------------------------------------------- + +namespace { + +class CallbackResourceTest : public TrackedCallbackTest { + public: + CallbackResourceTest() {} +}; + +class CallbackMockResource : public Resource { + public: + CallbackMockResource(PP_Instance instance) : Resource(instance) {} + ~CallbackMockResource() {} + + PP_Resource SetupForTest() { + PP_Resource resource_id = GetReference(); + EXPECT_NE(0, resource_id); + + callback_did_run_ = new TrackedCallback( + this, + PP_MakeCompletionCallback(&TestCallback, &info_did_run_)); + EXPECT_EQ(0U, info_did_run_.run_count); + + callback_did_abort_ = new TrackedCallback( + this, + PP_MakeCompletionCallback(&TestCallback, &info_did_abort_)); + EXPECT_EQ(0U, info_did_abort_.run_count); + + callback_didnt_run_ = new TrackedCallback( + this, + PP_MakeCompletionCallback(&TestCallback, &info_didnt_run_)); + EXPECT_EQ(0U, info_didnt_run_.run_count); + + callback_did_run_->Run(PP_OK); + callback_did_abort_->Abort(); + + CheckIntermediateState(); + + return resource_id; + } + + void CheckIntermediateState() { + EXPECT_EQ(1U, info_did_run_.run_count); + EXPECT_EQ(PP_OK, info_did_run_.result); + + EXPECT_EQ(1U, info_did_abort_.run_count); + EXPECT_EQ(PP_ERROR_ABORTED, info_did_abort_.result); + + EXPECT_EQ(0U, info_didnt_run_.run_count); + } + + void CheckFinalState() { + EXPECT_EQ(1U, info_did_run_.run_count); + EXPECT_EQ(PP_OK, info_did_run_.result); + EXPECT_EQ(1U, info_did_abort_.run_count); + EXPECT_EQ(PP_ERROR_ABORTED, info_did_abort_.result); + EXPECT_EQ(1U, info_didnt_run_.run_count); + EXPECT_EQ(PP_ERROR_ABORTED, info_didnt_run_.result); + } + + scoped_refptr<TrackedCallback> callback_did_run_; + CallbackRunInfo info_did_run_; + + scoped_refptr<TrackedCallback> callback_did_abort_; + CallbackRunInfo info_did_abort_; + + scoped_refptr<TrackedCallback> callback_didnt_run_; + CallbackRunInfo info_didnt_run_; +}; + +} // namespace + +// Test that callbacks get aborted on the last resource unref. +TEST_F(CallbackResourceTest, AbortOnNoRef) { + ResourceTracker* resource_tracker = + PpapiGlobals::Get()->GetResourceTracker(); + + // Test several things: Unref-ing a resource (to zero refs) with callbacks + // which (1) have been run, (2) have been aborted, (3) haven't been completed. + // Check that the uncompleted one gets aborted, and that the others don't get + // called again. + scoped_refptr<CallbackMockResource> resource_1( + new CallbackMockResource(pp_instance())); + PP_Resource resource_1_id = resource_1->SetupForTest(); + + // Also do the same for a second resource, and make sure that unref-ing the + // first resource doesn't much up the second resource. + scoped_refptr<CallbackMockResource> resource_2( + new CallbackMockResource(pp_instance())); + PP_Resource resource_2_id = resource_2->SetupForTest(); + + // Double-check that resource #1 is still okay. + resource_1->CheckIntermediateState(); + + // Kill resource #1, spin the message loop to run posted calls, and check that + // things are in the expected states. + resource_tracker->ReleaseResource(resource_1_id); + MessageLoop::current()->RunAllPending(); + resource_1->CheckFinalState(); + resource_2->CheckIntermediateState(); + + // Kill resource #2. + resource_tracker->ReleaseResource(resource_2_id); + MessageLoop::current()->RunAllPending(); + resource_1->CheckFinalState(); + resource_2->CheckFinalState(); + + // This shouldn't be needed, but make sure there are no stranded tasks. + MessageLoop::current()->RunAllPending(); +} + +// Test that "resurrecting" a resource (getting a new ID for a |Resource|) +// doesn't resurrect callbacks. +TEST_F(CallbackResourceTest, Resurrection) { + ResourceTracker* resource_tracker = + PpapiGlobals::Get()->GetResourceTracker(); + + scoped_refptr<CallbackMockResource> resource( + new CallbackMockResource(pp_instance())); + PP_Resource resource_id = resource->SetupForTest(); + + // Unref it, spin the message loop to run posted calls, and check that things + // are in the expected states. + resource_tracker->ReleaseResource(resource_id); + MessageLoop::current()->RunAllPending(); + resource->CheckFinalState(); + + // "Resurrect" it and check that the callbacks are still dead. + PP_Resource new_resource_id = resource->GetReference(); + MessageLoop::current()->RunAllPending(); + resource->CheckFinalState(); + + // Unref it again and do the same. + resource_tracker->ReleaseResource(new_resource_id); + MessageLoop::current()->RunAllPending(); + resource->CheckFinalState(); + + // This shouldn't be needed, but make sure there are no stranded tasks. + MessageLoop::current()->RunAllPending(); +} + +} // namespace ppapi diff --git a/webkit/plugins/ppapi/host_globals.cc b/webkit/plugins/ppapi/host_globals.cc index 80f538c..56eb90a 100644 --- a/webkit/plugins/ppapi/host_globals.cc +++ b/webkit/plugins/ppapi/host_globals.cc @@ -58,6 +58,16 @@ HostGlobals::~HostGlobals() { return &host_var_tracker_; } +::ppapi::CallbackTracker* HostGlobals::GetCallbackTrackerForInstance( + PP_Instance instance) { + std::map<PP_Instance, linked_ptr<InstanceData> >::iterator found = + instance_map_.find(instance); + if (found == instance_map_.end()) + return NULL; + + return found->second->instance->module()->GetNewCallbackTracker(); +} + ::ppapi::FunctionGroupBase* HostGlobals::GetFunctionAPI(PP_Instance pp_instance, ::ppapi::ApiID id) { // Get the instance object. This also ensures that the instance data is in diff --git a/webkit/plugins/ppapi/host_globals.h b/webkit/plugins/ppapi/host_globals.h index 51b7827..914be3a 100644 --- a/webkit/plugins/ppapi/host_globals.h +++ b/webkit/plugins/ppapi/host_globals.h @@ -6,6 +6,7 @@ #define WEBKIT_PLUGINS_PPAPI_HOST_GLOBALS_H_ #include "base/compiler_specific.h" +#include "ppapi/shared_impl/callback_tracker.h" #include "ppapi/shared_impl/ppapi_globals.h" #include "ppapi/shared_impl/var_tracker.h" #include "webkit/plugins/ppapi/host_resource_tracker.h" @@ -31,6 +32,8 @@ class HostGlobals : public ::ppapi::PpapiGlobals { // PpapiGlobals implementation. virtual ::ppapi::ResourceTracker* GetResourceTracker() OVERRIDE; virtual ::ppapi::VarTracker* GetVarTracker() OVERRIDE; + virtual ::ppapi::CallbackTracker* GetCallbackTrackerForInstance( + PP_Instance instance) OVERRIDE; virtual ::ppapi::FunctionGroupBase* GetFunctionAPI( PP_Instance inst, ::ppapi::ApiID id) OVERRIDE; diff --git a/webkit/plugins/ppapi/plugin_module.cc b/webkit/plugins/ppapi/plugin_module.cc index 9934666..3e08385d 100644 --- a/webkit/plugins/ppapi/plugin_module.cc +++ b/webkit/plugins/ppapi/plugin_module.cc @@ -84,6 +84,7 @@ #include "ppapi/c/trusted/ppb_graphics_3d_trusted.h" #include "ppapi/c/trusted/ppb_image_data_trusted.h" #include "ppapi/c/trusted/ppb_url_loader_trusted.h" +#include "ppapi/shared_impl/callback_tracker.h" #include "ppapi/shared_impl/ppb_input_event_shared.h" #include "ppapi/shared_impl/ppb_opengles2_shared.h" #include "ppapi/shared_impl/ppb_var_shared.h" @@ -415,7 +416,8 @@ PluginModule::PluginModule(const std::string& name, const FilePath& path, PluginDelegate::ModuleLifetime* lifetime_delegate) : lifetime_delegate_(lifetime_delegate), - callback_tracker_(new CallbackTracker), + old_callback_tracker_(new CallbackTracker), + callback_tracker_(new ::ppapi::CallbackTracker), is_in_destructor_(false), is_crashed_(false), broker_(NULL), @@ -445,6 +447,7 @@ PluginModule::~PluginModule() { GetLivePluginSet()->erase(this); + old_callback_tracker_->AbortAll(); callback_tracker_->AbortAll(); if (entry_points_.shutdown_module) @@ -550,6 +553,10 @@ void PluginModule::InstanceDeleted(PluginInstance* instance) { } scoped_refptr<CallbackTracker> PluginModule::GetCallbackTracker() { + return old_callback_tracker_; +} + +scoped_refptr< ::ppapi::CallbackTracker> PluginModule::GetNewCallbackTracker() { return callback_tracker_; } diff --git a/webkit/plugins/ppapi/plugin_module.h b/webkit/plugins/ppapi/plugin_module.h index dc2eaff..708c8ac 100644 --- a/webkit/plugins/ppapi/plugin_module.h +++ b/webkit/plugins/ppapi/plugin_module.h @@ -28,6 +28,7 @@ struct PPB_Core; typedef void* NPIdentifier; namespace ppapi { +class CallbackTracker; class WebKitForwarding; } // namespace ppapi @@ -117,6 +118,7 @@ class WEBKIT_PLUGINS_EXPORT PluginModule : void InstanceDeleted(PluginInstance* instance); scoped_refptr<CallbackTracker> GetCallbackTracker(); + scoped_refptr< ::ppapi::CallbackTracker> GetNewCallbackTracker(); // Called when running out of process and the plugin crashed. This will // release relevant resources and update all affected instances. @@ -154,7 +156,11 @@ class WEBKIT_PLUGINS_EXPORT PluginModule : // Tracker for completion callbacks, used mainly to ensure that all callbacks // are properly aborted on module shutdown. - scoped_refptr<CallbackTracker> callback_tracker_; + // + // TODO(brettw) remove this in favor of the ::ppapi:: one below. + scoped_refptr<CallbackTracker> old_callback_tracker_; + + scoped_refptr< ::ppapi::CallbackTracker> callback_tracker_; PP_Module pp_module_; |