Code cleanups, make child kill/reap more robust, add some comments.
authorIlpo Ruotsalainen <ilpo.ruotsalainen@movial.fi>
Fri, 12 Sep 2008 15:17:04 +0000 (18:17 +0300)
committerIlpo Ruotsalainen <ilpo.ruotsalainen@movial.fi>
Fri, 12 Sep 2008 15:17:04 +0000 (18:17 +0300)
isatis-plugin/plugin.c

index 0503cfc..c71cc44 100644 (file)
@@ -74,7 +74,22 @@ NPError NPP_Destroy(NPP instance, NPSavedData **save)
                        close(priv->data_fd);
 
                if (priv->player_pid)
-                       waitpid(priv->player_pid, NULL, WNOHANG);
+               {
+                       int i;
+
+                       kill(priv->player_pid, SIGTERM);
+
+                       for (i=0; i<20; i++)
+                       {
+                               /* We're done here if waitpid() fails (the child was already reaped by someone) or succeeds
+                                * (the child exited and we just reaped it) we're done */
+                               if (waitpid(priv->player_pid, NULL, WNOHANG) != 0)
+                                       break;
+
+                               /* Sleep 10ms per iteration for maximal total delay of 200ms if the child refuses to exit */
+                               usleep(10000);
+                       }
+               }
 
                free(priv);
        }
@@ -94,6 +109,7 @@ NPError NPP_SetWindow(NPP instance, NPWindow *window)
 
        DBG(("  window = %p pos = %d,%d dim = %dx%d\n", window->window, window->x, window->y, window->width, window->height));
 
+       /* Ignore repeated NPP_SetWindow() calls, we only want the XID of the window and that does not change */
        if (priv->player_pid > 0)
                return NPERR_NO_ERROR;
 
@@ -113,8 +129,11 @@ NPError NPP_SetWindow(NPP instance, NPWindow *window)
                                char xid_buf[32];
                                char uri[32];
 
+                               /* Close the input side of the pipe in the child process */
                                close(pipe_fds[1]);
 
+                               /* XXX Pass the stream length (if known) to child, and shortcut file:// URIs XXX */
+
                                snprintf(xid_buf, sizeof(xid_buf), "0x%x", (unsigned int)window->window);
                                snprintf(uri, sizeof(uri), "fd://%d", pipe_fds[0]);
 
@@ -141,6 +160,7 @@ NPError NPP_SetWindow(NPP instance, NPWindow *window)
 
        DBG(("  child pid is %d\n", r));
 
+       /* Close the output side of the pipe in the parent process */
        close(pipe_fds[0]);
 
        fcntl(pipe_fds[1], F_SETFL, O_NONBLOCK);
@@ -180,6 +200,8 @@ NPError NPP_DestroyStream(NPP instance, NPStream *stream, NPReason reason)
 
        GET_PLUGIN_PRIVATE(instance, priv);
 
+       /* Unfortunately we cannot pass the reason why the stream was closed to the player, oh well... */
+
        if (priv->data_fd != -1)
        {
                close(priv->data_fd);
@@ -189,11 +211,6 @@ NPError NPP_DestroyStream(NPP instance, NPStream *stream, NPReason reason)
        return NPERR_NO_ERROR;
 }
 
-void NPP_StreamAsFile(NPP instance, NPStream *stream, const char *fname)
-{
-       DBG(("NPP_StreamAsFile(%p, %p, \"%s\")\n", instance, stream, fname));
-}
-
 int32_t NPP_WriteReady(NPP instance, NPStream *stream)
 {
        struct plugin_private *priv;
@@ -206,8 +223,6 @@ int32_t NPP_WriteReady(NPP instance, NPStream *stream)
        fds.fd = priv->data_fd;
        fds.events = POLLOUT | POLLERR | POLLHUP;
 
-       /* XXX Handle child dying XXX */
-
        switch (poll(&fds, 1, 0))
        {
                case 0:
@@ -236,6 +251,8 @@ int32_t NPP_Write(NPP instance, NPStream *stream, int32_t offset, int32_t len, v
 
        int r = write(priv->data_fd, buffer, len);
 
+       /* XXX What about EAGAIN? It might happen if PIPE_BUF is ridiculously small for some reason XXX */
+
        if (r < 0)
                DBG(("write() failed: %s\n", strerror(errno)));
 
@@ -246,6 +263,9 @@ NPError NPP_GetValue(NPP instance, NPPVariable variable, void *value)
 {
        DBG(("NPP_GetValue(%p, %d)\n", instance, variable));
 
+       /* For simplicity's sake (and because the docs are very unclear on this) we handle both instanced
+        * and non-instanced value queries here */
+
        switch (variable)
        {
                case NPPVpluginNameString:
@@ -293,11 +313,12 @@ NPError NP_Initialize(NPNetscapeFuncs *host_vtable, NPPluginFuncs *plugin_vtable
        plugin_vtable->setwindow     = NewNPP_SetWindowProc(NPP_SetWindow);
        plugin_vtable->newstream     = NewNPP_NewStreamProc(NPP_NewStream);
        plugin_vtable->destroystream = NewNPP_DestroyStreamProc(NPP_DestroyStream);
-       plugin_vtable->asfile        = NewNPP_StreamAsFileProc(NPP_StreamAsFile);
        plugin_vtable->writeready    = NewNPP_WriteReadyProc(NPP_WriteReady);
        plugin_vtable->write         = NewNPP_WriteProc(NPP_Write);
        plugin_vtable->getvalue      = NewNPP_GetValueProc(NPP_GetValue);
 
+       /* Not sure if missing XEmbed should be fatal error; GTK seems quite unhappy without it, even though
+        * technically things should work (albeit with focus issues and such) */
        err = CallNPN_GetValueProc(host_vtable->getvalue, NULL, NPNVSupportsXEmbedBool, &hasxembed);
        if (err != NPERR_NO_ERROR || !hasxembed)
        {