Bug 213094

Created attachment 401688 [details] Patch

At a first glance the doc is confusing and I cannot see if manette_monitor_new returns transfer full or floating. If it's floating, no problem. If it's full, you need to change ManetteGamepadProvider.cpp:59 to adopt.

Comment on attachment 401688 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=401688&action=review > Source/WebCore/platform/gamepad/manette/ManetteGamepadProvider.h:72 > - GUniquePtr<ManetteMonitor> m_monitor; > + GRefPtr<ManetteMonitor> m_monitor; This is not enough, you should adopt the ref in the constructor.

(In reply to Carlos Garcia Campos from comment #3) > Comment on attachment 401688 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=401688&action=review > > > Source/WebCore/platform/gamepad/manette/ManetteGamepadProvider.h:72 > > - GUniquePtr<ManetteMonitor> m_monitor; > > + GRefPtr<ManetteMonitor> m_monitor; > > This is not enough, you should adopt the ref in the constructor. I am confused with a refcount of the ManetteMonitor instance here. manette_monitor_new returns a full object. As my understanding the gobject in terms of its refcount, it looks like... manette_monitor_new // its refcount is 1 when the monitor is created. adoptGRef(manette_monitor_new()) // refcount is still 1 since adoptGRef does not increase the refcount. m_monitor(adoptGRef(manette_monitor_new())) // refcount is 2 since the copy constructor increases the refcount by 1. In the destructor of ManetteGamepadProvider: the refcount of m_monitor will be decreased by 1 so the monitor would be eventually leaked since its refcount would be still 1? What am I missing?

(In reply to ChangSeok Oh from comment #4) > m_monitor(adoptGRef(manette_monitor_new())) // refcount is 2 since the copy constructor increases the refcount by 1. Aha.. maybe rvalue reference copy constructor is called here? i.e., GRefPtr(GRefPtr&& o) : m_ptr(o.leakRef()) { }

Created attachment 401836 [details] Patch

(In reply to ChangSeok Oh from comment #5) > (In reply to ChangSeok Oh from comment #4) > > m_monitor(adoptGRef(manette_monitor_new())) // refcount is 2 since the copy constructor increases the refcount by 1. > > Aha.. maybe rvalue reference copy constructor is called here? i.e., > GRefPtr(GRefPtr&& o) : m_ptr(o.leakRef()) { } Yes.

Committed r263040: <https://trac.webkit.org/changeset/263040> All reviewed patches have been landed. Closing bug and clearing flags on attachment 401836 [details].