Bug 31609 – Chrome/mac crashes when opening a popup, waiting until the webpage in the background switches, and then clicking the popup

Bug 31609 - Chrome/mac crashes when opening a popup, waiting until the webpage in the background switches, and then clicking the popup


Summary:	Chrome/mac crashes when opening a popup, waiting until the webpage in the bac...

Status:	RESOLVED FIXED

Product:	WebKit
Classification:	Unclassified
Component:	WebKit API
Version:	528+ (Nightly build)
Platform:	PC OS X 10.5

Importance:	P2 Normal
Assigned To:	Nobody

URL:
Keywords:

Depends on:
Blocks:
	Show dependency tree / graph

Reported:	2009-11-17 17:34 PST by Nico Weber
Modified:	2009-11-17 23:13 PST (History)
CC List:	2 users (show)

See Also:

Attachments
Patch. (1.56 KB, patch) 2009-11-17 17:36 PST, Nico Weber	fishd: review-	Details \| Formatted Diff \| Diff
Fix type (1.53 KB, patch) 2009-11-17 21:31 PST, Nico Weber	fishd: review+	Details \| Formatted Diff \| Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Nico Weber 2009-11-17 17:34:28 PST

See http://code.google.com/p/chromium/issues/detail?id=23869 for details.

Comment 1 Nico Weber 2009-11-17 17:36:21 PST

Created attachment 43394 [details]
Patch.

Comment 2 Darin Fisher (:fishd, Google) 2009-11-17 21:19:29 PST

Comment on attachment 43394 [details]
Patch.

> Index: WebCore/platform/chromium/PopupMenuChromium.cpp
...
> +    // The popup sends its "closed" notification through its parentx Set the

                                                                      ^^^ typo


> +    // parent, even though external popups have no real on-screen widget but a
> +    // native menu (see |PopupListBox::hidePopup()|);
> +    if (!m_listBox->parent())
> +        addChild(m_listBox.get());

Why would m_listBox's parent not be null?


R- for the typo

Comment 3 Nico Weber 2009-11-17 21:31:48 PST

Created attachment 43401 [details]
Fix type

Comment 4 Nico Weber 2009-11-17 21:34:01 PST

Heh, typo in "Fix typo". Nice.

> Why would m_listBox's parent not be null?

I was surprised too, but the branch for regular popups (PopupContainer::showPopup()) checks this too. So I figured it's safer to check this here as well.

Comment 5 Darin Fisher (:fishd, Google) 2009-11-17 22:51:44 PST

Landed as http://trac.webkit.org/changeset/51102

Comment 6 Mark Mentovai 2009-11-17 23:13:18 PST

I'm working off a tree from yesterday and I just wound up having to make this change to fix http://crbug.com/27723.  Then I went to merge my work into WebKit so I could get it landed here and I got a merge conflict because you already hit this.

Excellent!

(Yeah, this is the correct fix.)