Bug 37111

Summary: Draw replacement text when plug-in host crashes
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: Plug-insAssignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, webkit-ews, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Macintosh Intel   
OS: OS X 10.6   
Attachments:
Description Flags
proposed patch
darin: review+, darin: commit-queue-
updated patch darin: review+

Description Alexey Proskuryakov 2010-04-05 13:43:00 PDT
Currently, it's just an empty space.
Comment 1 Alexey Proskuryakov 2010-04-05 13:43:16 PDT
<rdar://problem/7790327>
Comment 2 Alexey Proskuryakov 2010-04-05 14:04:21 PDT
Created attachment 52566 [details]
proposed patch
Comment 3 Darin Adler 2010-04-05 14:09:50 PDT
Comment on attachment 52566 [details]
proposed patch

>      , m_showsMissingPluginIndicator(false)
> +    , m_showsCrashedPluginIndicator(false)

An enum would be better than two booleans. We never want to show both.

>      void setShowsMissingPluginIndicator(bool showsMissingPluginIndicator) { m_showsMissingPluginIndicator = showsMissingPluginIndicator; }
>      bool showsMissingPluginIndicator() const { return m_showsMissingPluginIndicator; }
> +    void setShowsCrashedPluginIndicator(bool showsCrashedPluginIndicator) { m_showsCrashedPluginIndicator = showsCrashedPluginIndicator; }
> +    bool showsCrashedPluginIndicator() const { return m_showsCrashedPluginIndicator; }

I know that someone (Simon) suggested adding the getter to Kevin's original patch, but I don't think we need the getters. Nor do I think we need a way to set either value to false.

> +2010-04-05  Alexey Proskuryakov  <ap@apple.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        https://bugs.webkit.org/show_bug.cgi?id=37111
> +        <rdar://problem/7790327> Draw replacement text when plug-in host crashes
> +
> +        * English.lproj/Localizable.strings: Added a string for plug-in failure.
> +
> +2010-04-05  Alexey Proskuryakov  <ap@apple.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Need a short description and bug URL (OOPS!)
> +
> +        * English.lproj/Localizable.strings:
> +

Double change log.

I think this will break the build on platforms other than Mac because the crashedPluginText function will be called, but not defined.

An interesting way to sidestep that would be to have the API take a string and have WebHostedNetscapePluginView pass the string in. Then you'd use a String instead of an enum in RenderEmbeddedObject.

r=me as long as you make it build on non-Mac platforms
Comment 4 WebKit Review Bot 2010-04-05 14:11:53 PDT
Attachment 52566 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/1652169
Comment 5 Early Warning System Bot 2010-04-05 14:13:30 PDT
Attachment 52566 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/1550283
Comment 6 Alexey Proskuryakov 2010-04-05 14:42:45 PDT
Created attachment 52575 [details]
updated patch

Updated patch. I chose to keep a common mechanism for both localized strings, since other ports may want to support out of process plug-ins.
Comment 7 Alexey Proskuryakov 2010-04-05 14:49:41 PDT
Committed <http://trac.webkit.org/changeset/57095>.