Clean up WebDOMEvent ownership of WebCore::Event.

public/platform/WebPrivatePtr.h

 /*
  * Copyright (C) 2010 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 33 matching lines @@
 // wrap a reference counted WebCore class.
 template <typename T>
 class WebPrivatePtr {
 public:
     WebPrivatePtr() : m_ptr(0) { }
     ~WebPrivatePtr() { WEBKIT_ASSERT(!m_ptr); }
 
     bool isNull() const { return !m_ptr; }
 
 #if WEBKIT_IMPLEMENTATION
+    WebPrivatePtr(const WebPrivatePtr& other)
+        : m_ptr(other.m_ptr)
+    {
+        if (m_ptr)
+            m_ptr->ref();
+    }

[abarth-chromium, 2013/05/22 17:16:11]

We shouldn't need to modify this class.

[dmichael (off chromium), 2013/05/22 17:37:33]

See the updated CL description. WebPrivatePtr did not define a copy constructor,
so the compiler-provided version was being used, and it didn't handle the
reference count. We can certainly avoid using it in this CL (using assign() as
you suggested in WebDOMEvent), but if we don't fix this, it will bite somebody
else. We *at least* need to disable the copy ctor. I think it's better to just
make it work; I don't see a downside.

Side note that WebSpeechInputRequest, when used from chromium or DumpRenderTree,
was using the compiler-provided assignment operator (since the real
implementation is WEBKIT_IMPLEMENTATION only). So disabling that below seems
appropriate, too.

[abarth-chromium, 2013/05/22 18:06:23]

Good point.  Let's try disabling the copy constructor.

[dmichael (off chromium), 2013/05/22 18:44:26]

Done. I kind of liked fixing it better; I wasn't sure how to comment on a copy
ctor that's disabled, for a class that really looks like it should have a copy
constructor. I think it will be confusing to the next person who tries to use it
 (Usual practice is the "rule of three":
http://www.drdobbs.com/c-made-easier-the-rule-of-three/184401400 ). Though the
Google style guide isn't clear on the matter.
"If your class needs to be copyable, prefer providing a copy method, such as
CopyFrom() or Clone(), rather than a copy constructor, because such methods
cannot be invoked implicitly. If a copy method is insufficient in your situation
(e.g. for performance reasons, or because your class needs to be stored by value
in an STL container), provide both a copy constructor and assignment operator."
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Copy_Constructors

...and WebKit/Blink style guides are silent on the matter.

+
     WebPrivatePtr(const PassRefPtr<T>& prp)
         : m_ptr(prp.leakRef())
     {
     }
 
     void reset()
     {
         assign(0);
     }
 
@@ skipping 26 matching lines @@
 
 private:
 #if WEBKIT_IMPLEMENTATION
     void assign(T* p)
     {
         // p is already ref'd for us by the caller
         if (m_ptr)
             m_ptr->deref();
         m_ptr = p;
     }
+#else
+    // Disable copy and assign; we define them above for the implementation, but
+    // we need to make sure that they aren't used outside there; the compiler-
+    // provided versions won't handle reference counting properly.
+    WebPrivatePtr(const WebPrivatePtr<T>&);
+    WebPrivatePtr<T>& operator=(const WebPrivatePtr<T>& other);
 #endif
 
     T* m_ptr;
 };
 
 } // namespace WebKit
 
 #endif