[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    Re: KDE/kdebase/apps/nsplugins/viewer
From:       "Maksim Orlovich" <mo85 () cornell ! edu>
Date:       2008-10-27 13:17:25
Message-ID: 10359.24.58.20.210.1225113445.squirrel () webmail ! cornell ! edu
[Download RAW message or body]

> SVN commit 876130 by sebsauer:
>
> Fix flash9 crash needed to allow me to watch youtube :)

Thank you for doing this. However, I am quite surprised that this would
fix anything, since all the reported bugs have crashes deep inside Flash,
while
a null NSPluginInstance would surely crash earlier? Could you please
elaborate on what exact scenario this is addressing?

Also, it would be nice if the comments actually reflected reality --- it
doesn't use the latest instance, the *pluginView naming is confusing
(there is no such concept in nspluginviewer), etc. I am also a bit
confused by the specific mention of NPN_UserAgent, though... And, erm.,
WTF is g_NPN_UserAgent doing anyway --- a functional change came in what
was supposed to be a pure Qt3Support-removal patch, and I thought I had it
UA-faking all the time anyway!?

> Patch is partly based on how WebCore works around those case.

And since it is, please make sure to include appropriate credit.

>  Looks as we
> have the same prob in 3.5. Ok to backport to 4.1 and 3.5?

Free free, though we no longer support 3.5 anyway.

> CC_MAIL:kfm-devel@kde.org

It's CCMAIL :)

>
>
>  M  +26 -6     nsplugin.cpp
>  M  +6 -0      nsplugin.h
>
>
> --- trunk/KDE/kdebase/apps/nsplugins/viewer/nsplugin.cpp #876129:876130
> @@ -103,6 +103,20 @@
>  }
>  #endif
>
> +// The plugin view is always the ndata of the instance. Sometimes,
> plug-ins will call an instance-specific function
> +// with a NULL instance. To workaround this, call the last plug-in view
> that made a call to a plug-in.
> +// Currently, the current plug-in view is only set before NPP_New in
> PluginView::start.
> +// This specifically works around Flash and Shockwave. When we call
> NPP_New, they call NPN_Useragent with a NULL instance.
> +NSPluginInstance* NSPluginInstance::s_currentPluginView = 0;
> +NSPluginInstance* NSPluginInstance::currentPluginView() { return
> s_currentPluginView; }
> +void NSPluginInstance::setCurrentPluginView(NSPluginInstance* inst) {
> s_currentPluginView = inst; }
> +static NSPluginInstance* pluginViewForInstance(NPP instance)
> +{
> +    if (instance && instance->ndata)
> +        return static_cast<NSPluginInstance*>(instance->ndata);
> +    return NSPluginInstance::currentPluginView();
> +}
> +
>  // server side functions
> -----------------------------------------------------
>
>  // allocate memory
> @@ -211,7 +225,7 @@
>     // FIXME: is this correct?  I imagine it is not.  (GS)
>     kDebug(1431) << "g_NPN_DestroyStream()";
>
> -   NSPluginInstance *inst = (NSPluginInstance*) instance->ndata;
> +   NSPluginInstance *inst = pluginViewForInstance(instance);
>     inst->streamFinished( (NSPluginStream *)stream->ndata );
>
>     switch (reason) {
> @@ -263,7 +277,7 @@
>  {
>     kDebug(1431) << "g_NPN_GetURL: url=" << url << " target=" << target;
>
> -   NSPluginInstance *inst =
> static_cast<NSPluginInstance*>(instance->ndata);
> +   NSPluginInstance *inst = pluginViewForInstance(instance);
>     if (inst) {
>        inst->requestURL( QString::fromLatin1(url), QString(),
>                          QString::fromLatin1(target), 0 );
> @@ -277,7 +291,7 @@
>                           void* notifyData)
>  {
>      kDebug(1431) << "g_NPN_GetURLNotify: url=" << url << " target=" <<
> target << " inst=" << (void*)instance;
> -   NSPluginInstance *inst =
> static_cast<NSPluginInstance*>(instance->ndata);
> +   NSPluginInstance *inst = pluginViewForInstance(instance);
>     if (inst) {
>        kDebug(1431) << "g_NPN_GetURLNotify: ndata=" << (void*)inst;
>        inst->requestURL( QString::fromLatin1(url), QString(),
> @@ -373,7 +387,7 @@
>        // FIXME
>     }
>
> -   NSPluginInstance *inst =
> static_cast<NSPluginInstance*>(instance->ndata);
> +   NSPluginInstance *inst = pluginViewForInstance(instance);
>     if (inst && !inst->normalizedURL(QString::fromLatin1(url)).isNull()) {
>        inst->postURL( QString::fromLatin1(url), postdata,
> browserArgs.contentType(),
>                       QString::fromLatin1(target), notifyData, args,
> browserArgs, true );
> @@ -471,7 +485,7 @@
>        // FIXME
>     }
>
> -   NSPluginInstance *inst =
> static_cast<NSPluginInstance*>(instance->ndata);
> +   NSPluginInstance *inst = pluginViewForInstance(instance);
>     if (inst && !inst->normalizedURL(QString::fromLatin1(url)).isNull()) {
>        inst->postURL( QString::fromLatin1(url), postdata,
> browserArgs.contentType(),
>                       QString::fromLatin1(target), 0L, args, browserArgs,
> false );
> @@ -493,7 +507,7 @@
>        return;
>
>     // turn into an instance signal
> -   NSPluginInstance *inst = (NSPluginInstance*) instance->ndata;
> +   NSPluginInstance *inst = pluginViewForInstance(instance);
>
>     inst->emitStatus(message);
>  }
> @@ -1429,6 +1443,8 @@
>                                                    baseURL, mimeType,
> appId,
>                                                    callbackId, embed, this
> );
>
> +   // set the current plugin instance
> +   NSPluginInstance::setCurrentPluginView(inst);
>
>     // create source stream
>     if ( !src.isEmpty() )
> @@ -1441,6 +1457,10 @@
>
>  void NSPluginClass::destroyInstance( NSPluginInstance* inst )
>  {
> +    // be sure we don't deal with a dangling pointer
> +    if ( NSPluginInstance::currentPluginView() == inst )
> +        NSPluginInstance::setCurrentPluginView(0);
> +
>      // mark for destruction
>      _trash.append( inst );
>      timer(); //_timer->start( 0, TRUE );
> --- trunk/KDE/kdebase/apps/nsplugins/viewer/nsplugin.h #876129:876130
> @@ -176,6 +176,10 @@
>    void gotFocusIn();
>    void gotFocusOut();
>
> +  // sets and returns the current/last used NSPluginInstance instance.
> +  static NSPluginInstance* currentPluginView();
> +  static void setCurrentPluginView(NSPluginInstance*);
> +
>    // value handling
>    NPError NPGetValue(NPPVariable variable, void *value);
>    NPError NPSetValue(NPNVariable variable, void *value);
> @@ -268,6 +272,8 @@
>
>    QQueue<Request *> _waitingRequests;
>    QMap<int, Request*> _jsrequests;
> +
> +  static NSPluginInstance* s_currentPluginView;
>  };
>
>
>


[prev in list] [next in list] [prev in thread] [next in thread]