Bug 31609 - Chrome/mac crashes when opening a popup, waiting until the webpage in the background switches, and then clicking the popup
: Chrome/mac crashes when opening a popup, waiting until the webpage in the bac...
Status: RESOLVED FIXED
: WebKit
WebKit API
: 528+ (Nightly build)
: PC Mac OS X 10.5
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2009-11-17 17:34 PST by
Modified: 2009-11-17 23:13 PST (History)


Attachments
Patch. (1.56 KB, patch)
2009-11-17 17:36 PST, Nico Weber
fishd: review-
Review Patch | Details | Formatted Diff | Diff
Fix type (1.53 KB, patch)
2009-11-17 21:31 PST, Nico Weber
fishd: review+
Review Patch | Details | Formatted Diff | Diff


Note

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


------- Comment #1 From 2009-11-17 17:36:21 PST -------
Created an attachment (id=43394) [details]
Patch.
------- Comment #2 From 2009-11-17 21:19:29 PST -------
(From update of attachment 43394 [details])
> 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 From 2009-11-17 21:31:48 PST -------
Created an attachment (id=43401) [details]
Fix type
------- Comment #4 From 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 From 2009-11-17 22:51:44 PST -------
Landed as http://trac.webkit.org/changeset/51102
------- Comment #6 From 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.)