[webkit-reviews] review granted: [Bug 41721] Missing plug-in indicator button should have a pressed state : [Attachment 60936] patch

[bugzilla-daemon at webkit.org]

Jon Honeycutt <jhoneycutt at apple.com> has granted Adele Peterson
<adele at apple.com>'s request for review:
Bug 41721: Missing plug-in indicator button should have a pressed state
https://bugs.webkit.org/show_bug.cgi?id=41721

Attachment 60936: patch
https://bugs.webkit.org/attachment.cgi?id=60936&action=review

------- Additional Comments from Jon Honeycutt <jhoneycutt at apple.com>
> ===================================================================
> Index: WebCore/plugins/PluginView.cpp
> ===================================================================
> --- WebCore/plugins/PluginView.cpp	(revision 62605)
> +++ WebCore/plugins/PluginView.cpp	(working copy)
> @@ -284,7 +284,9 @@ PluginView::~PluginView()
>  
>      ASSERT(!m_lifeSupportTimer.isActive());
>  
> -    instanceMap().remove(m_instance);
> +    // If the plug-in can't be found then m_instance is 0
> +    if (m_instance)
> +	   instanceMap().remove(m_instance);
>  
>      if (m_isWaitingToStart)
>	   m_parentFrame->document()->removeMediaCanStartListener(this);
> @@ -811,6 +813,7 @@ PluginView::PluginView(Frame* parentFram
>      , m_paramNames(0)
>      , m_paramValues(0)
>      , m_mimeType(mimeType)
> +    , m_instance(0)
>  #if defined(XP_MACOSX)
>      , m_isWindowed(false)
>  #else

This change isn't mentioned in your ChangeLog. This fix is also separate from
your other changes and could be split out into its own change. Also, although
your new test does test this on Windows, this fix could also have its own test
that ran on all platforms that use PluginView.

r=me