master 554a875: Prevent crashes in xwidgets whose buffers have been killed

[Po Lu]

branch: master
commit 554a875493680f8b52821267ee88e191d462ea36
Author: Po Lu <luangruo@yahoo.com>
Commit: Po Lu <luangruo@yahoo.com>

    Prevent crashes in xwidgets whose buffers have been killed
    
    * doc/lispref/display.texi (Xwidgets): Explain meaning of killed
    xwidgets.
    * src/xwidget.c (Fxwidget_live_p): New function.
    (Fxwidget_perform_lispy_event, WEBKIT_FN_INIT)
    (Fxwidget_resize, Fxwidget_size_request)
    (Fxwidget_info, Fxwidget_plist)
    (Fset_xwidget_buffer, Fset_xwidget_plist)
    (Fset_xwidget_query_on_exit_flag)
    (Fxwidget_query_on_exit_flag)
    (Fxwidget_webkit_search)
    (Fxwidget_webkit_next_result)
    (Fxwidget_webkit_previous_result)
    (Fxwidget_webkit_finish_search)
    (Fxwidget_webkit_load_html): Check for live xwidgets instead of
    just xwidgets.
    
    (xwidget_button, xwidget_motion_or_crossing)
    (xv_do_draw, x_draw_xwidget_glyph_string)
    (Fdelete_xwidget_view): Ignore killed xwidgets.
    
    (syms_of_xwidget): Define new symbols and subrs and define
    appropriate weakness of id_to_xwidget map.
    (kill_buffer_xwidgets): Check live xwidgets instead of
    killed xwidgets, set xwidget buffer to nil, and rely on GC to
    free the hash table for us instead.
    
    * src/xwidget.h (XWIDGET_LIVE_P, CHECK_LIVE_XWIDGET): New
    macros.
---
 doc/lispref/display.texi |  14 +++++-
 src/xwidget.c            | 116 +++++++++++++++++++++++++++++++++--------------
 src/xwidget.h            |   7 +++
 3 files changed, 101 insertions(+), 36 deletions(-)

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index ad1077e..a8a7837 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -6804,6 +6804,12 @@ pixels, and @var{title}, a string, specifies its title.  
@var{related}
 is used internally by the WebKit widget, and specifies another WebKit
 widget that the newly created widget should share settings and
 subprocesses with.
+
+The xwidget that is returned will be killed alongside its buffer
+(@pxref{Killing Buffers}).  Once it is killed, the xwidget may
+continue to exist as a Lisp object and act as a @code{display}
+property until all references to it are gone, but most actions that
+can be performed on live xwidgets will no longer be available.
 @end defun
 
 @defun xwidgetp object
@@ -6811,6 +6817,11 @@ This function returns @code{t} if @var{object} is an 
xwidget,
 @code{nil} otherwise.
 @end defun
 
+@defun xwidget-live-p object
+This function returns @code{t} if @var{object} is an xwidget that
+hasn't been killed, and @code{nil} otherwise.
+@end defun
+
 @defun xwidget-plist xwidget
 This function returns the property list of @var{xwidget}.
 @end defun
@@ -6821,7 +6832,8 @@ property list given by @var{plist}.
 @end defun
 
 @defun xwidget-buffer xwidget
-This function returns the buffer of @var{xwidget}.
+This function returns the buffer of @var{xwidget}.  If @var{xwidget}
+has been killed, it returns @code{nil}.
 @end defun
 
 @defun set-xwidget-buffer xwidget buffer
diff --git a/src/xwidget.c b/src/xwidget.c
index fc05f4f..d0cc3b9 100644
--- a/src/xwidget.c
+++ b/src/xwidget.c
@@ -272,6 +272,16 @@ fails.  */)
   return val;
 }
 
+DEFUN ("xwidget-live-p", Fxwidget_live_p, Sxwidget_live_p,
+       1, 1, 0, doc: /* Return t if OBJECT is an xwidget that has not been 
killed.
+Value is nil if OBJECT is not an xwidget or if it has been killed.  */)
+  (Lisp_Object object)
+{
+  return ((XWIDGETP (object)
+          && !NILP (XXWIDGET (object)->buffer))
+         ? Qt : Qnil);
+}
+
 #ifdef USE_GTK
 static void
 set_widget_if_text_view (GtkWidget *widget, void *data)
@@ -304,7 +314,7 @@ selected frame is not an X-Windows frame.  */)
   GtkWidget *temp = NULL;
 #endif
 
-  CHECK_XWIDGET (xwidget);
+  CHECK_LIVE_XWIDGET (xwidget);
   xw = XXWIDGET (xwidget);
 
   if (!NILP (frame))
@@ -806,6 +816,9 @@ xwidget_button (struct xwidget_view *view,
                bool down_p, int x, int y, int button,
                int modifier_state, Time time)
 {
+  if (NILP (XXWIDGET (view->model)->buffer))
+    return;
+
   record_osr_embedder (view);
 
   if (button < 4 || button > 8)
@@ -856,22 +869,29 @@ xwidget_button (struct xwidget_view *view,
 void
 xwidget_motion_or_crossing (struct xwidget_view *view, const XEvent *event)
 {
-  GdkEvent *xg_event = gdk_event_new (event->type == MotionNotify
-                                     ? GDK_MOTION_NOTIFY
-                                     : (event->type == LeaveNotify
-                                        ? GDK_LEAVE_NOTIFY
-                                        : GDK_ENTER_NOTIFY));
+  GdkEvent *xg_event;
   struct xwidget *model = XXWIDGET (view->model);
   int x;
   int y;
-  GtkWidget *target = find_widget_at_pos (model->widgetwindow_osr,
-                                         (event->type == MotionNotify
-                                          ? event->xmotion.x + view->clip_left
-                                          : event->xcrossing.x + 
view->clip_left),
-                                         (event->type == MotionNotify
-                                          ? event->xmotion.y + view->clip_top
-                                          : event->xcrossing.y + 
view->clip_top),
-                                         &x, &y);
+  GtkWidget *target;
+
+  if (NILP (model->buffer))
+    return;
+
+  xg_event = gdk_event_new (event->type == MotionNotify
+                           ? GDK_MOTION_NOTIFY
+                           : (event->type == LeaveNotify
+                              ? GDK_LEAVE_NOTIFY
+                              : GDK_ENTER_NOTIFY));
+
+  target = find_widget_at_pos (model->widgetwindow_osr,
+                              (event->type == MotionNotify
+                               ? event->xmotion.x + view->clip_left
+                               : event->xcrossing.x + view->clip_left),
+                              (event->type == MotionNotify
+                               ? event->xmotion.y + view->clip_top
+                               : event->xcrossing.y + view->clip_top),
+                              &x, &y);
 
   if (!target)
     target = model->widget_osr;
@@ -968,6 +988,13 @@ xv_do_draw (struct xwidget_view *xw, struct xwidget *w)
 {
   GtkOffscreenWindow *wnd;
   cairo_surface_t *surface;
+
+  if (NILP (w->buffer))
+    {
+      XClearWindow (xw->dpy, xw->wdesc);
+      return;
+    }
+
   block_input ();
   wnd = GTK_OFFSCREEN_WINDOW (w->widgetwindow_osr);
   surface = gtk_offscreen_window_get_surface (wnd);
@@ -1650,14 +1677,25 @@ x_draw_xwidget_glyph_string (struct glyph_string *s)
      a redraw.  It seems its possible to get out of sync with emacs
      redraws so emacs background sometimes shows up instead of the
      xwidgets background.  It's just a visual glitch though.  */
-  if (!xwidget_hidden (xv))
+  /* When xww->buffer is nil, that means the xwidget has been killed.  */
+  if (!NILP (xww->buffer))
     {
+      if (!xwidget_hidden (xv))
+       {
 #ifdef USE_GTK
-      gtk_widget_queue_draw (xww->widget_osr);
+         gtk_widget_queue_draw (xww->widget_osr);
 #elif defined NS_IMPL_COCOA
-      nsxwidget_set_needsdisplay (xv);
+         nsxwidget_set_needsdisplay (xv);
 #endif
+       }
+    }
+#ifdef USE_GTK
+  else
+    {
+      XSetWindowBackground (xv->dpy, xv->wdesc,
+                           FRAME_BACKGROUND_PIXEL (s->f));
     }
+#endif
 
 #ifdef USE_GTK
   unblock_input ();
@@ -1676,7 +1714,7 @@ xwidget_is_web_view (struct xwidget *xw)
 
 /* Macro that checks xwidget hold webkit web view first.  */
 #define WEBKIT_FN_INIT()                                               \
-  CHECK_XWIDGET (xwidget);                                             \
+  CHECK_LIVE_XWIDGET (xwidget);                                                
\
   struct xwidget *xw = XXWIDGET (xwidget);                             \
   if (!xwidget_is_web_view (xw))                                       \
     {                                                                  \
@@ -1855,7 +1893,7 @@ DEFUN ("xwidget-resize", Fxwidget_resize, 
Sxwidget_resize, 3, 3, 0,
        doc: /* Resize XWIDGET to NEW_WIDTH, NEW_HEIGHT.  */ )
   (Lisp_Object xwidget, Lisp_Object new_width, Lisp_Object new_height)
 {
-  CHECK_XWIDGET (xwidget);
+  CHECK_LIVE_XWIDGET (xwidget);
   int w = check_integer_range (new_width, 0, INT_MAX);
   int h = check_integer_range (new_height, 0, INT_MAX);
   struct xwidget *xw = XXWIDGET (xwidget);
@@ -1906,7 +1944,7 @@ This can be used to read the xwidget desired size, and 
resizes the
 Emacs allocated area accordingly.  */)
   (Lisp_Object xwidget)
 {
-  CHECK_XWIDGET (xwidget);
+  CHECK_LIVE_XWIDGET (xwidget);
 #ifdef USE_GTK
   GtkRequisition requisition;
   gtk_widget_size_request (XXWIDGET (xwidget)->widget_osr, &requisition);
@@ -1941,7 +1979,7 @@ DEFUN ("xwidget-info",
 Currently [TYPE TITLE WIDTH HEIGHT].  */)
   (Lisp_Object xwidget)
 {
-  CHECK_XWIDGET (xwidget);
+  CHECK_LIVE_XWIDGET (xwidget);
   struct xwidget *xw = XXWIDGET (xwidget);
   return CALLN (Fvector, xw->type, xw->title,
                make_fixed_natnum (xw->width), make_fixed_natnum (xw->height));
@@ -2004,7 +2042,7 @@ DEFUN ("delete-xwidget-view",
       unblock_input ();
     }
 
-  if (xw->embedder_view == xv)
+  if (xw->embedder_view == xv && !NILP (xw->buffer))
     {
       w = gtk_widget_get_window (xw->widgetwindow_osr);
 
@@ -2053,7 +2091,7 @@ DEFUN ("xwidget-plist",
        doc: /* Return the plist of XWIDGET.  */)
   (Lisp_Object xwidget)
 {
-  CHECK_XWIDGET (xwidget);
+  CHECK_LIVE_XWIDGET (xwidget);
   return XXWIDGET (xwidget)->plist;
 }
 
@@ -2073,7 +2111,7 @@ DEFUN ("set-xwidget-buffer",
        doc: /* Set XWIDGET's buffer to BUFFER.  */)
   (Lisp_Object xwidget, Lisp_Object buffer)
 {
-  CHECK_XWIDGET (xwidget);
+  CHECK_LIVE_XWIDGET (xwidget);
   CHECK_BUFFER (buffer);
 
   XXWIDGET (xwidget)->buffer = buffer;
@@ -2087,7 +2125,7 @@ DEFUN ("set-xwidget-plist",
 Returns PLIST.  */)
   (Lisp_Object xwidget, Lisp_Object plist)
 {
-  CHECK_XWIDGET (xwidget);
+  CHECK_LIVE_XWIDGET (xwidget);
   CHECK_LIST (plist);
 
   XXWIDGET (xwidget)->plist = plist;
@@ -2103,7 +2141,7 @@ exiting or killing a buffer if XWIDGET is running.
 This function returns FLAG.  */)
   (Lisp_Object xwidget, Lisp_Object flag)
 {
-  CHECK_XWIDGET (xwidget);
+  CHECK_LIVE_XWIDGET (xwidget);
   XXWIDGET (xwidget)->kill_without_query = NILP (flag);
   return flag;
 }
@@ -2114,7 +2152,7 @@ DEFUN ("xwidget-query-on-exit-flag",
        doc: /* Return the current value of the query-on-exit flag for XWIDGET. 
 */)
   (Lisp_Object xwidget)
 {
-  CHECK_XWIDGET (xwidget);
+  CHECK_LIVE_XWIDGET (xwidget);
   return (XXWIDGET (xwidget)->kill_without_query ? Qnil : Qt);
 }
 
@@ -2147,7 +2185,7 @@ with QUERY.  */)
 #endif
 
   CHECK_STRING (query);
-  CHECK_XWIDGET (xwidget);
+  CHECK_LIVE_XWIDGET (xwidget);
 
 #ifdef USE_GTK
   xw = XXWIDGET (xwidget);
@@ -2191,7 +2229,7 @@ using `xwidget-webkit-search'.  */)
   WebKitFindController *controller;
 #endif
 
-  CHECK_XWIDGET (xwidget);
+  CHECK_LIVE_XWIDGET (xwidget);
   xw = XXWIDGET (xwidget);
 
   if (!xw->find_text)
@@ -2223,7 +2261,7 @@ using `xwidget-webkit-search'.  */)
   WebKitFindController *controller;
 #endif
 
-  CHECK_XWIDGET (xwidget);
+  CHECK_LIVE_XWIDGET (xwidget);
   xw = XXWIDGET (xwidget);
 
   if (!xw->find_text)
@@ -2255,7 +2293,7 @@ using `xwidget-webkit-search'.  */)
   WebKitFindController *controller;
 #endif
 
-  CHECK_XWIDGET (xwidget);
+  CHECK_LIVE_XWIDGET (xwidget);
   xw = XXWIDGET (xwidget);
 
   if (!xw->find_text)
@@ -2293,7 +2331,7 @@ to "about:blank".  */)
   WebKitWebView *webview;
   char *data, *uri;
 
-  CHECK_XWIDGET (xwidget);
+  CHECK_LIVE_XWIDGET (xwidget);
   CHECK_STRING (text);
   if (NILP (base_uri))
     base_uri = build_string ("about:blank");
@@ -2321,7 +2359,9 @@ syms_of_xwidget (void)
 {
   defsubr (&Smake_xwidget);
   defsubr (&Sxwidgetp);
+  defsubr (&Sxwidget_live_p);
   DEFSYM (Qxwidgetp, "xwidgetp");
+  DEFSYM (Qxwidget_live_p, "xwidget-live-p");
   defsubr (&Sxwidget_view_p);
   DEFSYM (Qxwidget_view_p, "xwidget-view-p");
   defsubr (&Sxwidget_info);
@@ -2379,7 +2419,8 @@ syms_of_xwidget (void)
 
   Fprovide (intern ("xwidget-internal"), Qnil);
 
-  id_to_xwidget_map = CALLN (Fmake_hash_table, QCtest, Qeq);
+  id_to_xwidget_map = CALLN (Fmake_hash_table, QCtest, Qeq,
+                            QCweakness, Qvalue);
   staticpro (&id_to_xwidget_map);
 
 #ifdef USE_GTK
@@ -2605,9 +2646,10 @@ kill_buffer_xwidgets (Lisp_Object buffer)
       Vxwidget_list = Fdelq (xwidget, Vxwidget_list);
       /* TODO free the GTK things in xw.  */
       {
-        CHECK_XWIDGET (xwidget);
+        CHECK_LIVE_XWIDGET (xwidget);
         struct xwidget *xw = XXWIDGET (xwidget);
-       Fremhash (make_fixnum (xw->xwidget_id), id_to_xwidget_map);
+       xw->buffer = Qnil;
+
 #ifdef USE_GTK
         if (xw->widget_osr && xw->widgetwindow_osr)
           {
@@ -2624,6 +2666,10 @@ kill_buffer_xwidgets (Lisp_Object buffer)
                xfree (xmint_pointer (XCAR (cb)));
              ASET (xw->script_callbacks, idx, Qnil);
            }
+
+       xw->widget_osr = NULL;
+       xw->widgetwindow_osr = NULL;
+       xw->find_text = NULL;
 #elif defined NS_IMPL_COCOA
         nsxwidget_kill (xw);
 #endif
diff --git a/src/xwidget.h b/src/xwidget.h
index 6e6b39c..4377b50 100644
--- a/src/xwidget.h
+++ b/src/xwidget.h
@@ -138,9 +138,16 @@ struct xwidget_view
 #define XXWIDGET(a) (eassert (XWIDGETP (a)), \
                     XUNTAG (a, Lisp_Vectorlike, struct xwidget))
 
+#define XWIDGET_LIVE_P(w) (!NILP ((w)->buffer))
+
 #define CHECK_XWIDGET(x) \
   CHECK_TYPE (XWIDGETP (x), Qxwidgetp, x)
 
+#define CHECK_LIVE_XWIDGET(x)                          \
+  CHECK_TYPE ((XWIDGETP (x)                            \
+              && XWIDGET_LIVE_P (XXWIDGET (x))),       \
+             Qxwidget_live_p, x)
+
 /* Test for xwidget_view pseudovector.  */
 #define XWIDGET_VIEW_P(x) PSEUDOVECTORP (x, PVEC_XWIDGET_VIEW)
 #define XXWIDGET_VIEW(a) (eassert (XWIDGET_VIEW_P (a)), \