[Webkit-unassigned] [Bug 36721] Return correct load result to NPAPI plugins calling getUrlNotify on target frames

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=36721


Alexey Proskuryakov <ap at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #75628|review?                     |review-
               Flag|                            |




--- Comment #28 from Alexey Proskuryakov <ap at webkit.org>  2010-12-06 21:04:15 PST ---
(From update of attachment 75628)
View in context: https://bugs.webkit.org/attachment.cgi?id=75628&action=review

> LayoutTests/ChangeLog:14
> +        * platform/qt/Skipped: unskip plugins/get-url-with-blank-target.html
> +        * platform/gtk/Skipped: ditto

What about platform/win?

> WebCore/loader/FrameLoader.cpp:207
>      , m_sandboxFlags(SandboxAll)
>      , m_forcedSandboxFlags(SandboxNone)
>  {
> +    m_PluginToNotify = 0;

This should be initialized, not assigned.

Also, wrong style, should be m_pluginToNotify.

> WebCore/loader/FrameLoader.cpp:1403
>      Frame* frame = findFrameForNavigation(frameName);
>      if (frame) {
> -        frame->loader()->load(request, lockHistory);
> +        frame->loader()->load(request, lockHistory, m_PluginToNotify);
> +        m_PluginToNotify = 0;
>          return;

What guarantees that frame->loader() != this?

> WebCore/loader/FrameLoader.h:494
> +    PluginView* m_PluginToNotify;

Can the view be destroyed before FrameLoader? Will we end up with a stale pointer then?

I imagine that this may not be too bad, as the request will be stopped when destroying plug-in, too. But to program defensively, we should not keep a stale pointer, even if we're not going to use it.

> WebCore/plugins/PluginView.cpp:426
> +        if (m_requestsAwaitingNotification[i]->request() == request && m_requestsAwaitingNotification[i]->sendNotification()) {

I still don't believe that comparing requests can ever be good. Am I missing something?

> WebCore/plugins/PluginView.h:117
> +        ResourceRequest m_resourceRequest;

If we actually don't need to compare requests, then this will go away, too.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.