Bug 95118 – Hook procedure should check to see if hook has already been removed before attempting removal.

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Roger Fong]

When the hookFired method in LayerChangesFlusher.cpp gets called it calls CallNextHook to pass the message to the next hook procedure in the hook chain.

However, sometimes, the hook procedure it gets passed to can dispatch another message that goes back through the hook chain and causes the hookFired method to get called again (before returning from CallNextHookEx).

When the second hookFired call completes, it can remove the hook. 
When CallNextHook returns the original call to hookFired may try to remove the hook as well. However, there is no need as the hook as already been removed and in fact we get an assertion failure assert(m_hook) when we try to call removeHook().

The fix is simply to check to see that m_hook is not null before calling into removeHook, so we don't waste effort trying to remove a hook that has already been removed.

Alternatively, I could just remove the assert but then just call UnhookWindowHookEx regardless of whether the hook has already been removed or not, but that seems a little sketchy to me...

[Radar WebKit Bug Importer]

<rdar://problem/12182019>

[Roger Fong]

> However, sometimes, the hook procedure it gets passed to can dispatch another message that goes back through the hook chain and causes the hookFired method to get called again (before returning from CallNextHookEx).

I think I missed half a sentence...: 
Sometimes, the hook procedure can passed to another hook procedure that can dispatch another message. This message goes back through the hook chain and causes the hookFired method to get called again (before returning from CallNextHookEx).

[Roger Fong]

Created attachment 160795 [details]
patch

[Roger Fong]

Created attachment 160798 [details]
typo in Changelog

[Simon Fraser (smfr)]

Comment on attachment 160798 [details]
typo in Changelog

View in context: https://bugs.webkit.org/attachment.cgi?id=160798&action=review

> Source/WebCore/ChangeLog:8
> +        Sometimes, the hook procedure can passed to another hook procedure that can dispatch another message. 

"can passed to"?

> Source/WebCore/ChangeLog:9
> +        This message goes back through the hook chain and causes the hookFired method to get called again 

Are you saying that it can re-enter?

[Roger Fong]

Created attachment 160803 [details]
more typos/clarification

[WebKit Review Bot]

Comment on attachment 160803 [details]
more typos/clarification

Clearing flags on attachment: 160803

Committed r126805: <http://trac.webkit.org/changeset/126805>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.