Make Carbon plugin idle event source robust against changes during iteration

webkit/glue/plugins/webplugin_delegate_impl_mac.mm

 // Copyright (c) 2006-2008 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.
 
 #import <Cocoa/Cocoa.h>
 
 #include "webkit/glue/plugins/webplugin_delegate_impl.h"
 
 #include <string>
 #include <unistd.h>
@@ skipping 72 matching lines @@
 
 class CarbonIdleEventSource {
  public:
   // Returns the shared Carbon idle event source.
   static CarbonIdleEventSource* SharedInstance() {
     DCHECK(MessageLoop::current()->type() == MessageLoop::TYPE_UI);
     static CarbonIdleEventSource* event_source = new CarbonIdleEventSource();
     return event_source;
   }
 
-  // Registers the plugin delegate as interested in receiving idle events
-  // suitable for a visible plugin.
-  // Registering a delegate as visible automatically unregisters it from the
-  // hidden event source.
-  void RegisterVisibleDelegate(WebPluginDelegateImpl* delegate) {
-    UnregisterDelegate(delegate);
-    if (visible_delegates_.empty()) {
-      visible_timer_.Start(
-          base::TimeDelta::FromMilliseconds(kVisibleIdlePeriodMs), this,
-          &CarbonIdleEventSource::SendVisiblePluginEvents);
+  // Registers the plugin delegate as interested in receiving idle events at
+  // a rate appropriate for the given visibility. A delegate can safely be
+  // re-registered any number of times, with the latest registration winning.
+  void RegisterDelegate(WebPluginDelegateImpl* delegate, bool visible) {
+    if (visible) {
+      visible_delegates_->RegisterDelegate(delegate);
+      hidden_delegates_->UnregisterDelegate(delegate);
+    } else {
+      hidden_delegates_->RegisterDelegate(delegate);
+      visible_delegates_->UnregisterDelegate(delegate);
     }
-    visible_delegates_.insert(delegate);
-  }
-
-  // Registers the plugin delegate as interested in receiving idle events
-  // suitable for a plugin that isn't visible.
-  // Registering a delegate as hidden automatically unregisters it from the
-  // visible event source.
-  void RegisterHiddenDelegate(WebPluginDelegateImpl* delegate) {
-    UnregisterDelegate(delegate);
-    if (hidden_delegates_.empty()) {
-      hidden_timer_.Start(
-          base::TimeDelta::FromMilliseconds(kHiddenIdlePeriodMs), this,
-          &CarbonIdleEventSource::SendHiddenPluginEvents);
-    }
-    hidden_delegates_.insert(delegate);
   }
 
   // Removes the plugin delegate from the list of plugins receiving idle events.
   void UnregisterDelegate(WebPluginDelegateImpl* delegate) {
-    size_t removed = visible_delegates_.erase(delegate);
-    if (removed > 0 && visible_delegates_.empty())
-      visible_timer_.Stop();
-    removed = hidden_delegates_.erase(delegate);
-    if (removed > 0 && hidden_delegates_.empty())
-      hidden_timer_.Stop();
+    visible_delegates_->UnregisterDelegate(delegate);
+    hidden_delegates_->UnregisterDelegate(delegate);
   }
 
  private:
-  CarbonIdleEventSource() {}
+  class VisibilityGroup {
+   public:
+    explicit VisibilityGroup(int timer_period)
+        : timer_period_(timer_period), iterator_(delegates_.end()) {}
 
-  void SendVisiblePluginEvents() {
-    SendIdleEventsToDelegates(visible_delegates_);
-  }
+    // Adds |delegate| to this visibility group.
+    void RegisterDelegate(WebPluginDelegateImpl* delegate) {
+      if (delegates_.empty()) {
+        timer_.Start(base::TimeDelta::FromMilliseconds(timer_period_),
+                     this, &VisibilityGroup::SendIdleEvents);
+      }
+      delegates_.insert(delegate);
+    }
 
-  void SendHiddenPluginEvents() {
-    SendIdleEventsToDelegates(hidden_delegates_);
-  }
+    // Removes |delegate| from this visibility group.
+    void UnregisterDelegate(WebPluginDelegateImpl* delegate) {
+      // If a plugin changes visibility during idle event handling, it
+      // may be removed from this set while SendIdleEvents is still iterating;
+      // if that happens and it's next on the list, increment the iterator

[pink (ping after 24hrs), 2010/02/05 17:31:32]

doesn't any change to the list while an iterator is in progress make the
iterator invalid and undefined, not just "if it's the next one"? What's the
documented behavior for stl? I fear this might just happen to work but isn't
guaranteed to.

+      // before erasing so that the iteration won't be corrupted.
+      if ((iterator_ != delegates_.end()) && (*iterator_ == delegate))
+        ++iterator_;
+      size_t removed = delegates_.erase(delegate);
+      if (removed > 0 && delegates_.empty())
+        timer_.Stop();
+    }
 
-  void SendIdleEventsToDelegates(
-      const std::set<WebPluginDelegateImpl*>& delegates) const {
-    for (std::set<WebPluginDelegateImpl*>::iterator i = delegates.begin();
-         i != delegates.end();) {
-      // If the plugin changes size or position during idle event handling, it
-      // may be removed from this set; increment the iterator before calling
-      // into the delegate to ensure that the iteration won't be corrupted.
-      WebPluginDelegateImpl* delegate = *(i++);
-      delegate->FireIdleEvent();
+   private:
+    // Fires off idle events for each delegate in the group.
+    void SendIdleEvents() {
+      for (iterator_ = delegates_.begin(); iterator_ != delegates_.end();) {
+        // Pre-increment so that the skip logic in UnregisterDelegates works.
+        WebPluginDelegateImpl* delegate = *(iterator_++);
+        delegate->FireIdleEvent();
+      }
     }
-  }
 
-  base::RepeatingTimer<CarbonIdleEventSource> visible_timer_;
-  base::RepeatingTimer<CarbonIdleEventSource> hidden_timer_;
-  std::set<WebPluginDelegateImpl*> visible_delegates_;
-  std::set<WebPluginDelegateImpl*> hidden_delegates_;
+    int timer_period_;
+    base::RepeatingTimer<VisibilityGroup> timer_;
+    std::set<WebPluginDelegateImpl*> delegates_;
+    std::set<WebPluginDelegateImpl*>::iterator iterator_;
+  };
+
+  CarbonIdleEventSource()
+      : visible_delegates_(new VisibilityGroup(kVisibleIdlePeriodMs)),
+        hidden_delegates_(new VisibilityGroup(kHiddenIdlePeriodMs)) {}
+
+  scoped_ptr<VisibilityGroup> visible_delegates_;
+  scoped_ptr<VisibilityGroup> hidden_delegates_;
+
+  DISALLOW_COPY_AND_ASSIGN(CarbonIdleEventSource);
 };
 #endif  // !NP_NO_CARBON
 
 }  // namespace
 
 WebPluginDelegateImpl::WebPluginDelegateImpl(
     gfx::PluginWindowHandle containing_view,
     NPAPI::PluginInstance *instance)
     : windowless_needs_set_window_(true),
       // all Mac plugins are "windowless" in the Windows/X11 sense
@@ skipping 463 matching lines @@
     int width = new_width ? new_width : old_width;
     window_bounds.left = target_x;
     window_bounds.top = target_y;
     window_bounds.right = window_bounds.left + width;
     window_bounds.bottom = window_bounds.top + height;
     SetWindowBounds(window, kWindowContentRgn, &window_bounds);
   }
 }
 
 void WebPluginDelegateImpl::UpdateIdleEventRate() {
-  if (container_is_visible_ && !clip_rect_.IsEmpty())
-    CarbonIdleEventSource::SharedInstance()->RegisterVisibleDelegate(this);
-  else
-    CarbonIdleEventSource::SharedInstance()->RegisterHiddenDelegate(this);
+  bool plugin_visible = container_is_visible_ && !clip_rect_.IsEmpty();
+  CarbonIdleEventSource::SharedInstance()->RegisterDelegate(this,
+                                                            plugin_visible);
 }
 #endif  // !NP_NO_CARBON
 
 static bool WebInputEventIsWebMouseEvent(const WebInputEvent& event) {
   switch (event.type) {
     case WebInputEvent::MouseMove:
     case WebInputEvent::MouseLeave:
     case WebInputEvent::MouseEnter:
     case WebInputEvent::MouseDown:
     case WebInputEvent::MouseUp:
@@ skipping 405 matching lines @@
 #ifndef NP_NO_QUICKDRAW
   // Quickdraw-based plugins can draw at any time, so tell the renderer to
   // repaint.
   // TODO: only do this if the contents of the offscreen window has changed,
   // so as not to spam the renderer with an unchanging image.
   if (instance()->drawing_model() == NPDrawingModelQuickDraw)
     instance()->webplugin()->Invalidate();
 #endif
 }
 #endif  // !NP_NO_CARBON