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

Side by Side Diff: third_party/WebKit/Source/platform/LifecycleNotifier.h

Issue 2094143002: Avoid snapshotting ContextLifecycleObservers when iterating. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: remove IterationScope, indirection not needed Created 4 years, 5 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
« no previous file with comments | « third_party/WebKit/Source/core/page/PageLifecycleNotifier.cpp ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 /* 1 /*
2 * Copyright (C) 2008 Apple Inc. All Rights Reserved. 2 * Copyright (C) 2008 Apple Inc. All Rights Reserved.
3 * Copyright (C) 2013 Google Inc. All Rights Reserved. 3 * Copyright (C) 2013 Google Inc. All Rights Reserved.
4 * 4 *
5 * Redistribution and use in source and binary forms, with or without 5 * Redistribution and use in source and binary forms, with or without
6 * modification, are permitted provided that the following conditions 6 * modification, are permitted provided that the following conditions
7 * are met: 7 * are met:
8 * 1. Redistributions of source code must retain the above copyright 8 * 1. Redistributions of source code must retain the above copyright
9 * notice, this list of conditions and the following disclaimer. 9 * notice, this list of conditions and the following disclaimer.
10 * 2. Redistributions in binary form must reproduce the above copyright 10 * 2. Redistributions in binary form must reproduce the above copyright
(...skipping 35 matching lines...) Expand 10 before | Expand all | Expand 10 after
46 // 46 //
47 // When contextDestroyed() is called, the observer's lifecycleContext() 47 // When contextDestroyed() is called, the observer's lifecycleContext()
48 // is still valid and safe to use during the notification. 48 // is still valid and safe to use during the notification.
49 virtual void notifyContextDestroyed(); 49 virtual void notifyContextDestroyed();
50 50
51 DEFINE_INLINE_VIRTUAL_TRACE() 51 DEFINE_INLINE_VIRTUAL_TRACE()
52 { 52 {
53 visitor->trace(m_observers); 53 visitor->trace(m_observers);
54 } 54 }
55 55
56 bool isIteratingOverObservers() const { return m_iterating != IteratingNone; } 56 bool isIteratingOverObservers() const { return m_iterationState != NotIterat ing; }
57 57
58 protected: 58 protected:
59 LifecycleNotifier() 59 LifecycleNotifier()
60 : m_iterating(IteratingNone) 60 : m_iterationState(NotIterating)
61 { 61 {
62 } 62 }
63 63
64 enum IterationType {
65 IteratingNone,
66 IteratingOverAll,
67 };
68
69 IterationType m_iterating;
70
71 protected:
72 using ObserverSet = HeapHashSet<WeakMember<Observer>>;
73
74 ObserverSet m_observers;
75
76 #if DCHECK_IS_ON() 64 #if DCHECK_IS_ON()
77 T* context() { return static_cast<T*>(this); } 65 T* context() { return static_cast<T*>(this); }
78 #endif 66 #endif
67
68 using ObserverSet = HeapHashSet<WeakMember<Observer>>;
69
70 enum IterationState {
71 AllowingNone = 0,
72 AllowingAddition,
73 AllowingRemoval,
74 NotIterating = AllowingAddition | AllowingRemoval,
75 };
76
77 // Iteration state is recorded while iterating the observer set,
78 // optionally barring add or remove mutations.
79 IterationState m_iterationState;
80 ObserverSet m_observers;
79 }; 81 };
80 82
81 template<typename T, typename Observer> 83 template<typename T, typename Observer>
82 inline LifecycleNotifier<T, Observer>::~LifecycleNotifier() 84 inline LifecycleNotifier<T, Observer>::~LifecycleNotifier()
83 { 85 {
84 // FIXME: Enable the following ASSERT. Also see a FIXME in Document::detach( ). 86 // FIXME: Enable the following ASSERT. Also see a FIXME in Document::detach( ).
85 // ASSERT(!m_observers.size()); 87 // ASSERT(!m_observers.size());
86 } 88 }
87 89
88 template<typename T, typename Observer> 90 template<typename T, typename Observer>
89 inline void LifecycleNotifier<T, Observer>::notifyContextDestroyed() 91 inline void LifecycleNotifier<T, Observer>::notifyContextDestroyed()
90 { 92 {
91 TemporaryChange<IterationType> scope(m_iterating, IteratingOverAll); 93 // Observer unregistration is allowed, but effectively a no-op.
94 TemporaryChange<IterationState> scope(m_iterationState, AllowingRemoval);
92 ObserverSet observers; 95 ObserverSet observers;
93 m_observers.swap(observers); 96 m_observers.swap(observers);
94 for (Observer* observer : observers) { 97 for (Observer* observer : observers) {
95 ASSERT(observer->lifecycleContext() == context()); 98 ASSERT(observer->lifecycleContext() == context());
96 observer->contextDestroyed(); 99 observer->contextDestroyed();
97 } 100 }
98 } 101 }
99 102
100 template<typename T, typename Observer> 103 template<typename T, typename Observer>
101 inline void LifecycleNotifier<T, Observer>::addObserver(Observer* observer) 104 inline void LifecycleNotifier<T, Observer>::addObserver(Observer* observer)
102 { 105 {
103 RELEASE_ASSERT(m_iterating != IteratingOverAll); 106 RELEASE_ASSERT(m_iterationState & AllowingAddition);
104 m_observers.add(observer); 107 m_observers.add(observer);
105 } 108 }
106 109
107 template<typename T, typename Observer> 110 template<typename T, typename Observer>
108 inline void LifecycleNotifier<T, Observer>::removeObserver(Observer* observer) 111 inline void LifecycleNotifier<T, Observer>::removeObserver(Observer* observer)
109 { 112 {
113 RELEASE_ASSERT(m_iterationState & AllowingRemoval);
110 m_observers.remove(observer); 114 m_observers.remove(observer);
111 } 115 }
112 116
113 } // namespace blink 117 } // namespace blink
114 118
115 #endif // LifecycleNotifier_h 119 #endif // LifecycleNotifier_h
OLDNEW
« no previous file with comments | « third_party/WebKit/Source/core/page/PageLifecycleNotifier.cpp ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698