Bug 33468 – PluginView::m_mode not initialized when constructed with null PluginPackage

• Component: Plug-ins
• Status: RESOLVED FIXED

[Andreas Kling]

Created attachment 46275 [details]
Proposed patch

Valgrind complains like so:

==19266== Conditional jump or move depends on uninitialised value(s)
==19266==    at 0x6BFD294: WebCore::PluginView::setFrameRect(WebCore::IntRect const&) (PluginView.cpp:133)
==19266==    by 0x6D13EA5: WebCore::RenderWidget::setWidgetGeometry(WebCore::IntRect const&) (RenderWidget.cpp:159)
==19266==    by 0x6D14DFC: WebCore::RenderWidget::updateWidgetPosition() (RenderWidget.cpp:308)
==19266==    by 0x6D0B4FF: WebCore::RenderView::updateWidgetPositions() (RenderView.cpp:543)
==19266==    by 0x6B1E6F3: WebCore::FrameView::performPostLayoutTasks() (FrameView.cpp:1380)
==19266==    by 0x6B1C05E: WebCore::FrameView::layout(bool) (FrameView.cpp:732)
==19266==    by 0x6B1D21A: WebCore::FrameView::visibleContentsResized() (FrameView.cpp:1010)
==19266==    by 0x6BC6D06: WebCore::ScrollView::updateScrollbars(WebCore::IntSize const&) (ScrollView.cpp:403)
==19266==    by 0x6BC6347: WebCore::ScrollView::setContentsSize(WebCore::IntSize const&) (ScrollView.cpp:239)
==19266==    by 0x6B1B0CF: WebCore::FrameView::setContentsSize(WebCore::IntSize const&) (FrameView.cpp:372)
==19266==    by 0x6B1B244: WebCore::FrameView::adjustViewSize() (FrameView.cpp:392)
==19266==    by 0x6B1BEE7: WebCore::FrameView::layout(bool) (FrameView.cpp:698)

For something like:

<object type="application/some-invalid-type"></object>

[Andreas Kling]

Created attachment 46276 [details]
Proposed patch with bug ID in ChangeLog

[Darin Adler]

Comment on attachment 46276 [details]
Proposed patch with bug ID in ChangeLog

If we're fixing this, why not use member construction syntax instead of assignment?

[Andreas Kling]

Created attachment 46280 [details]
Same thing, but using member construction syntax

Good point.

[Andreas Kling]

Created attachment 46281 [details]
Same thing yet again, but not adding a new bug..

[Darin Adler]

Comment on attachment 46281 [details]
Same thing yet again, but not adding a new bug..

Another way to fix this is to get rid of m_mode and replace it with a function.

The value of m_mode never changes and so is always based on m_loadManually and is used in only 3 places in the tree.

I'm especially mystified by the code in Qt's PluginView::setNPWindowIfNeeded that checks for m_mode values other than NP_EMBED and NP_FULL. Seems clearly to be dead code.

[WebKit Commit Bot]

Comment on attachment 46281 [details]
Same thing yet again, but not adding a new bug..

Clearing flags on attachment: 46281

Committed r53109: <http://trac.webkit.org/changeset/53109>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Adele Peterson]

This change caused the following bug:

REGRESSION (r53109): Find My iPhone message dialog is offset from original position once it gets focus
https://bugs.webkit.org/show_bug.cgi?id=33821

[Adele Peterson]

Please ignore my last comment.  There was an error in my testing.

[Andreas Kling]

*** Bug 25933 has been marked as a duplicate of this bug. ***