Bug 167268 – When Safari reloads pages with Flash objects after Flash is installed, placeholders don't paint (but do work!)

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Anders Carlsson]

When Safari reloads pages with Flash objects after Flash is installed, placeholders don't paint (but do work!)

[Anders Carlsson]

Created attachment 299404 [details]
Patch

[Ricky Mondello]

<rdar://problem/29857388>

[Anders Carlsson]

Created attachment 299525 [details]
Patch

[Anders Carlsson]

Created attachment 299529 [details]
Patch

[Sam Weinig]

Comment on attachment 299529 [details]
Patch

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

> Source/WebCore/page/Page.cpp:540
>      Vector<Ref<Frame>> framesNeedingReload;

Can you remove this? It seems unused now.

> Source/WebCore/page/Page.cpp:546
> +        pluginInfoProvider->refresh(reload);

Is there anything ensuring the PluginInfoProviders are kept alive for this loop? If they are tied to the page's lifetime, do we really need the two loops?

> Source/WebCore/plugins/PluginInfoProvider.cpp:57
> +    for (auto& frame : framesNeedingReload)
> +        frame->loader().reload();

For a specific page, is there any reason to reload a frame that is a child of a frame that has already been reloaded in this loop? For instance, if the main frame is reloaded, what is the value of reloading any subframes?

[Sam Weinig]

Oh, as always, is this testable?

[Anders Carlsson]

Committed r211052: <http://trac.webkit.org/changeset/211052>

[Darin Adler]

Comment on attachment 299529 [details]
Patch

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

>> Source/WebCore/page/Page.cpp:540
>>      Vector<Ref<Frame>> framesNeedingReload;
> 
> Can you remove this? It seems unused now.

Patch landed with this unused vector still there. Should remove it!