Bug 107036 – [GTK] Remove null check for WebPopupMenuProxy::Client in WebPopupMenuProxyGtk::showPopupMenu

• Status: RESOLVED
• Resolution: INVALID
• Component: WebKitGTK

[Zan Dobersek]

Based on a Coverity warning listed in bug #104114.

[Zan Dobersek]

Created attachment 183014 [details]
Patch

[Martin Robinson]

Comment on attachment 183014 [details]
Patch

Hrm. Is this correct? Could it be possible for m_client to become null during the execution of the event loop?

[Zan Dobersek]

Comment on attachment 183014 [details]
Patch

No, this approach is incorrect.

[Zan Dobersek]

Created attachment 183666 [details]
Patch

[Zan Dobersek]

This now deserves a better title.

[Zan Dobersek]

Created attachment 183667 [details]
Patch

[Martin Robinson]

Comment on attachment 183667 [details]
Patch

So you're positive that the client cannot be invalidated by running the main loop?

[Zan Dobersek]

(In reply to comment #7)
> (From update of attachment 183667 [details])
> So you're positive that the client cannot be invalidated by running the main loop?

There are three places where the popup can get invalidated.
1. http://trac.webkit.org/browser/trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp#L3065
This invalidation happens after the hidePopupMenu is called on the WebPopupMenuProxy. This call pops down the actual menu, which causes the unmap signal to be handled - there, the loop of the popup menu proxy is stopped and the selected item (if any) is signaled to the client. Only after that is the popup menu proxy invalidated.

2. http://trac.webkit.org/browser/trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp#L3083
This invalidation happens after the call to the showPopupMenu method on the WebPopupMenuProxy returns. This method runs a loop until a selection is made in the popup menu or the popup menu is unmapped. After the loop is stopped (and still in the showPopupMenu) the selected item (if any) is signaled to the client. Again, only after that is the popup menu proxy invalidated.

3. http://trac.webkit.org/browser/trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp#L3094
Same as #1.

So I believe the current code cannot cause the invalidation while the loop is running.
I guess a WebKit2 owner is required to look at this?

[Martin Robinson]

Okay. I guess we need a nod from a WebKit2 OWNER.

[Zan Dobersek]

Adding WK2 owners to the CC list.

[Zan Dobersek]

Committed r162768: <http://trac.webkit.org/changeset/162768>

[WebKit Commit Bot]

Re-opened since this is blocked by bug 128495

[Zan Dobersek]

The popup menu proxy can be invalidated through the HidePopupMenu message from the WebKitWebProces before it exits the run loop. Because of that the early return is actually needed.