Bug 131203 – Crash in plugin process

• Status: RESOLVED
• Resolution: FIXED
• Component: Plug-ins

[Stephanie Lewis]

Created attachment 228573 [details]
patch

<rdar://problem/16479432>

[WebKit Commit Bot]

Attachment 228573 [details] did not pass style-queue:


ERROR: Source/WebKit2/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Geoffrey Garen]

Comment on attachment 228573 [details]
patch

r=me

You should explain in your ChangeLog that the old code crashed because it the C++ scope would delete its pluginController before the call to pluginDidBecomeHidden.

Would it be better for this function to take a pluginInstanceID argument, instead of a raw (dangerous) PluginControllerProxy pointer?

[Stephanie Lewis]

committed http://trac.webkit.org/projects/webkit/changeset/166907

[Darin Adler]

Comment on attachment 228573 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=228573&action=review

> Source/WebKit2/PluginProcess/WebProcessConnection.cpp:96
> +    unsigned pluginInstanceID = pluginController->pluginInstanceID();
>      {
> -        ASSERT(m_pluginControllers.contains(pluginController->pluginInstanceID()));
> +        ASSERT(m_pluginControllers.contains(pluginInstanceID));
>  
> -        std::unique_ptr<PluginControllerProxy> pluginControllerUniquePtr = m_pluginControllers.take(pluginController->pluginInstanceID());
> +        std::unique_ptr<PluginControllerProxy> pluginControllerUniquePtr = m_pluginControllers.take(pluginInstanceID);
>          ASSERT(pluginControllerUniquePtr.get() == pluginController);
>      }
>  
> -    pluginDidBecomeHidden(pluginController->pluginInstanceID());
> +    pluginDidBecomeHidden(pluginInstanceID);

Not really sure why the original code uses take. I would write this like this:

    unsigned instanceID = pluginController->pluginInstanceID();

    ASSERT(m_pluginControllers.get(pluginInstanceID) == pluginController);
    m_pluginControllers.remove(instanceID);

    pluginDidBecomeHidden(pluginInstanceID);