This is a follow-up patch for r142072. Now we are passing raw pointers from EventContext::handleLocalEvents() to Event::setTarget() and Event::setRelatedTarget(). They should be PassRefPtrs.
Created attachment 187062 [details] Patch
Comment on attachment 187062 [details] Patch There is no rationale, and I think that this change is incorrect. PassRefPtr is a performance optimization to be used when passing ownership, which is not the case here.
Huh? This change just makes these call sites more readable by removing the unnecessary call to get().
(In reply to comment #2) > (From update of attachment 187062 [details]) > There is no rationale, and I think that this change is incorrect. PassRefPtr is a performance optimization to be used when passing ownership, which is not the case here. ap: r142072 introduced the following code. event->setTarget(m_target.get()); event->setCurrentTarget(m_currentTarget.get()); if (m_relatedTarget.get() && event->isMouseEvent()) toMouseEvent(event)->setRelatedTarget(m_relatedTarget.get()); + else if (m_relatedTarget.get() && event->isFocusEvent()) + toFocusEvent(event)->setRelatedTarget(m_relatedTarget); The change made get() usage in EventContext.cpp inconsistent, so I just tried to clean them up.
Yes, it's possible that there were many changed introducing PassRefPtr unnecessarily. It's not good to use it without a reason, as that makes semantics of the function confusing.
(In reply to comment #4) > event->setTarget(m_target.get()); > event->setCurrentTarget(m_currentTarget.get()); > if (m_relatedTarget.get() && event->isMouseEvent()) > toMouseEvent(event)->setRelatedTarget(m_relatedTarget.get()); > + else if (m_relatedTarget.get() && event->isFocusEvent()) > + toFocusEvent(event)->setRelatedTarget(m_relatedTarget); > Yes, it's possible that there were many changed introducing PassRefPtr unnecessarily. It's not good to use it without a reason, as that makes semantics of the function confusing. Thanks. Then the above change should have been: > + else if (m_relatedTarget.get() && event->isFocusEvent()) > + toFocusEvent(event)->setRelatedTarget(m_relatedTarget.get()); ? I think the current inconsistent situation is not good at least.
Created attachment 187206 [details] Patch
Looks like this issue is already fixed on trunk.