Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(25)

Side by Side Diff: ppapi/shared_impl/callback_tracker.cc

Issue 869883003: Never lock the Pepper proxy lock on the IO thread (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: fix GN build, fix tests Created 5 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright (c) 2010 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "ppapi/shared_impl/callback_tracker.h" 5 #include "ppapi/shared_impl/callback_tracker.h"
6 6
7 #include <algorithm> 7 #include <algorithm>
8 8
9 #include "base/bind.h" 9 #include "base/bind.h"
10 #include "base/compiler_specific.h" 10 #include "base/compiler_specific.h"
11 #include "base/logging.h" 11 #include "base/logging.h"
12 #include "base/message_loop/message_loop.h" 12 #include "base/message_loop/message_loop.h"
13 #include "ppapi/c/pp_completion_callback.h" 13 #include "ppapi/c/pp_completion_callback.h"
14 #include "ppapi/shared_impl/proxy_lock.h"
14 #include "ppapi/shared_impl/tracked_callback.h" 15 #include "ppapi/shared_impl/tracked_callback.h"
15 16
16 namespace ppapi { 17 namespace ppapi {
17 18
18 // CallbackTracker ------------------------------------------------------------- 19 // CallbackTracker -------------------------------------------------------------
19 20
20 CallbackTracker::CallbackTracker() {} 21 CallbackTracker::CallbackTracker() : abort_all_called_(false) {
22 }
21 23
22 void CallbackTracker::AbortAll() { 24 void CallbackTracker::AbortAll() {
23 // Iterate over a copy since |Abort()| calls |Remove()| (indirectly). 25 // Iterate over a copy:
24 // TODO(viettrungluu): This obviously isn't so efficient. 26 // 1) because |Abort()| calls |Remove()| (indirectly).
25 CallbackSetMap pending_callbacks_copy = pending_callbacks_; 27 // 2) So we can drop the lock before calling in to TrackedCallback.
28 CallbackSetMap pending_callbacks_copy;
29 {
30 base::AutoLock acquire(lock_);
31 pending_callbacks_copy = pending_callbacks_;
32 abort_all_called_ = true;
33 }
26 for (CallbackSetMap::iterator it1 = pending_callbacks_copy.begin(); 34 for (CallbackSetMap::iterator it1 = pending_callbacks_copy.begin();
27 it1 != pending_callbacks_copy.end(); 35 it1 != pending_callbacks_copy.end();
28 ++it1) { 36 ++it1) {
29 for (CallbackSet::iterator it2 = it1->second.begin(); 37 for (CallbackSet::iterator it2 = it1->second.begin();
30 it2 != it1->second.end(); 38 it2 != it1->second.end();
31 ++it2) { 39 ++it2) {
32 (*it2)->Abort(); 40 (*it2)->Abort();
33 } 41 }
34 } 42 }
35 } 43 }
36 44
37 void CallbackTracker::PostAbortForResource(PP_Resource resource_id) { 45 void CallbackTracker::PostAbortForResource(PP_Resource resource_id) {
38 CHECK(resource_id != 0); 46 CHECK_NE(resource_id, 0);
39 CallbackSetMap::iterator it1 = pending_callbacks_.find(resource_id); 47 CallbackSet callbacks_for_resource;
40 if (it1 == pending_callbacks_.end()) 48 {
41 return; 49 base::AutoLock acquire(lock_);
42 for (CallbackSet::iterator it2 = it1->second.begin(); 50 CallbackSetMap::iterator iter = pending_callbacks_.find(resource_id);
43 it2 != it1->second.end(); 51 // The resource may have no callbacks, so it won't be found, and we're done.
44 ++it2) { 52 if (iter == pending_callbacks_.end())
45 // Post the abort. 53 return;
46 (*it2)->PostAbort(); 54 // Copy the set so we can drop the lock before calling in to
55 // TrackedCallback.
56 callbacks_for_resource = iter->second;
57 }
58 for (const auto& iter : callbacks_for_resource) {
59 iter->PostAbort();
47 } 60 }
48 } 61 }
49 62
50 CallbackTracker::~CallbackTracker() { 63 CallbackTracker::~CallbackTracker() {
51 // All callbacks must be aborted before destruction. 64 // All callbacks must be aborted before destruction.
52 CHECK_EQ(0u, pending_callbacks_.size()); 65 CHECK_EQ(0u, pending_callbacks_.size());
53 } 66 }
54 67
55 void CallbackTracker::Add( 68 void CallbackTracker::Add(
56 const scoped_refptr<TrackedCallback>& tracked_callback) { 69 const scoped_refptr<TrackedCallback>& tracked_callback) {
70 base::AutoLock acquire(lock_);
71 DCHECK(!abort_all_called_);
57 PP_Resource resource_id = tracked_callback->resource_id(); 72 PP_Resource resource_id = tracked_callback->resource_id();
58 DCHECK(pending_callbacks_[resource_id].find(tracked_callback) == 73 DCHECK(pending_callbacks_[resource_id].find(tracked_callback) ==
59 pending_callbacks_[resource_id].end()); 74 pending_callbacks_[resource_id].end());
60 pending_callbacks_[resource_id].insert(tracked_callback); 75 pending_callbacks_[resource_id].insert(tracked_callback);
61 } 76 }
62 77
63 void CallbackTracker::Remove( 78 void CallbackTracker::Remove(
64 const scoped_refptr<TrackedCallback>& tracked_callback) { 79 const scoped_refptr<TrackedCallback>& tracked_callback) {
80 base::AutoLock acquire(lock_);
65 CallbackSetMap::iterator map_it = 81 CallbackSetMap::iterator map_it =
66 pending_callbacks_.find(tracked_callback->resource_id()); 82 pending_callbacks_.find(tracked_callback->resource_id());
67 DCHECK(map_it != pending_callbacks_.end()); 83 DCHECK(map_it != pending_callbacks_.end());
68 CallbackSet::iterator it = map_it->second.find(tracked_callback); 84 CallbackSet::iterator it = map_it->second.find(tracked_callback);
69 DCHECK(it != map_it->second.end()); 85 DCHECK(it != map_it->second.end());
70 map_it->second.erase(it); 86 map_it->second.erase(it);
71 87
72 // If there are no pending callbacks left for this ID, get rid of the entry. 88 // If there are no pending callbacks left for this ID, get rid of the entry.
73 if (map_it->second.empty()) 89 if (map_it->second.empty())
74 pending_callbacks_.erase(map_it); 90 pending_callbacks_.erase(map_it);
75 } 91 }
76 92
77 } // namespace ppapi 93 } // namespace ppapi
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698