unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#60703: Patches to xwidget code
@ 2023-01-10  5:14 Andrew De Angelis
  2023-01-10  9:59 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
       [not found] ` <87cz7mhfrr.fsf@yahoo.com>
  0 siblings, 2 replies; 20+ messages in thread
From: Andrew De Angelis @ 2023-01-10  5:14 UTC (permalink / raw)
  To: emacs-devel, 60703


[-- Attachment #1.1: Type: text/plain, Size: 1561 bytes --]

Hello everyone and thanks for all your work!

I have some fixes to the xwidget code that I'd like to contribute.

These should achieve three things
1) fix `xwidget-webkit-current-url' in xwidget.el so that it actually
displays the current URL
2) fix the Objective-C code in nsxwidget.m to eliminate memory leaks
3) implement the function `xwidget-webkit-estimated-load-progress' in
Objective-C, making this functionality available for macOS as well

Regarding 2), I have tested my changes with the Instruments app within the
XCode dev tools. I was able to test them on two different machines: a
MacBook Air M2 running macOS Ventura 13, and an Intel processor running
macOS Big Sur 11.6.7.
When testing with Instruments, it appears that Emacs isn't leaking memory
anymore. I am still seeing some leaks, but according to Instruments, the
responsible libraries are system libraries, and not Emacs itself.
If possible, I would appreciate it if other people can test/profile these
changes as well, and share their feedback.

Assuming the changes are correct and accepted, would it be possible to
include them in the upcoming Emacs 29.1 release?
Technically, none of these are new features; it's just fixes to existing
features.

I intend to keep working on xwidget support for macOS (fixing remaining
issues and implementing missing functionalities), but I'm not sure when
I'll be able to get to it; for now I'd like to start by contributing these
first major fixes.

Thanks in advance for your feedback; let me know if you need anything else
from me.

Best,
Andrew

[-- Attachment #1.2: Type: text/html, Size: 1876 bytes --]

[-- Attachment #2: xwidget-patches.patch --]
[-- Type: application/octet-stream, Size: 10051 bytes --]

diff --git a/lisp/xwidget.el b/lisp/xwidget.el
index abbda29081..8095fa9db5 100644
--- a/lisp/xwidget.el
+++ b/lisp/xwidget.el
@@ -924,8 +924,9 @@ xwidget-webkit-reload
 (defun xwidget-webkit-current-url ()
   "Display the current xwidget webkit URL and place it on the `kill-ring'."
   (interactive nil xwidget-webkit-mode)
-  (let ((url (xwidget-webkit-uri (xwidget-webkit-current-session))))
-    (message "URL: %s" (kill-new (or url "")))))
+  (let ((url (or (xwidget-webkit-uri (xwidget-webkit-current-session)) "")))
+    (kill-new url)
+    (message "URL: %s" url)))
 
 (defun xwidget-webkit-browse-history ()
   "Display a buffer containing the history of page loads."
diff --git a/src/nsxwidget.h b/src/nsxwidget.h
index 8d55fac532..2b5596f905 100644
--- a/src/nsxwidget.h
+++ b/src/nsxwidget.h
@@ -36,6 +36,8 @@ #define NSXWIDGET_H_INCLUDED
 Lisp_Object nsxwidget_webkit_title (struct xwidget *xw);
 void nsxwidget_webkit_goto_uri (struct xwidget *xw, const char *uri);
 void nsxwidget_webkit_goto_history (struct xwidget *xw, int rel_pos);
+double nsxwidget_webkit_estimated_load_progress(struct xwidget *xw);
+void nsxwidget_webkit_stop_loading (struct xwidget *xw);
 void nsxwidget_webkit_zoom (struct xwidget *xw, double zoom_change);
 void nsxwidget_webkit_execute_script (struct xwidget *xw, const char *script,
                                       Lisp_Object fun);
diff --git a/src/nsxwidget.m b/src/nsxwidget.m
index e1fbd749b6..aaf867bcdd 100644
--- a/src/nsxwidget.m
+++ b/src/nsxwidget.m
@@ -57,12 +57,18 @@ @interface XwWebView : WKWebView
 @end
 @implementation XwWebView : WKWebView
 
+- (void)dealloc
+{
+  [super dealloc];
+}
+
 - (id)initWithFrame:(CGRect)frame
       configuration:(WKWebViewConfiguration *)configuration
             xwidget:(struct xwidget *)xw
 {
   /* Script controller to add script message handler and user script.  */
-  WKUserContentController *scriptor = [[WKUserContentController alloc] init];
+  WKUserContentController *scriptor = [[[WKUserContentController alloc] init]
+                                        autorelease];
   configuration.userContentController = scriptor;
 
   /* Enable inspect element context menu item for debugging.  */
@@ -81,7 +87,8 @@ - (id)initWithFrame:(CGRect)frame
   if (self)
     {
       self.xw = xw;
-      self.urlScriptBlocked = [[NSMutableDictionary alloc] init];
+      self.urlScriptBlocked = [[[NSMutableDictionary alloc] init]
+                                autorelease];
       self.navigationDelegate = self;
       self.UIDelegate = self;
       self.customUserAgent =
@@ -89,11 +96,13 @@ - (id)initWithFrame:(CGRect)frame
         @" AppleWebKit/603.3.8 (KHTML, like Gecko)"
         @" Version/11.0.1 Safari/603.3.8";
       [scriptor addScriptMessageHandler:self name:@"keyDown"];
-      [scriptor addUserScript:[[WKUserScript alloc]
-                                initWithSource:xwScript
-                                 injectionTime:
-                                  WKUserScriptInjectionTimeAtDocumentStart
-                                forMainFrameOnly:NO]];
+      WKUserScript *userScript = [[[WKUserScript alloc]
+                                    initWithSource:xwScript
+                                     injectionTime:
+                                      WKUserScriptInjectionTimeAtDocumentStart
+                                    forMainFrameOnly:NO]
+                                   autorelease];
+      [scriptor addUserScript:userScript];
     }
   return self;
 }
@@ -102,7 +111,27 @@ - (void)webView:(WKWebView *)webView
 didFinishNavigation:(WKNavigation *)navigation
 {
   if (EQ (Fbuffer_live_p (self.xw->buffer), Qt))
-    store_xwidget_event_string (self.xw, "load-changed", "");
+    store_xwidget_event_string (self.xw, "load-changed", "load-finished");
+}
+
+- (void)webView:(WKWebView *)webView didStartProvisionalNavigation:(WKNavigation *)navigation
+{
+  if (EQ (Fbuffer_live_p (self.xw->buffer), Qt))
+    store_xwidget_event_string (self.xw, "load-changed", "load-started");
+}
+
+- (void)webView:(WKWebView *)webView
+didReceiveServerRedirectForProvisionalNavigation:(WKNavigation *)navigation
+{
+  if (EQ (Fbuffer_live_p (self.xw->buffer), Qt))
+    store_xwidget_event_string (self.xw, "load-changed", "load-redirected");
+}
+
+// Start loading WKWebView
+- (void)webView:(WKWebView *)webView didCommitNavigation:(WKNavigation *)navigation
+{
+  if (EQ (Fbuffer_live_p (self.xw->buffer), Qt)) // what exactly is this test for
+    store_xwidget_event_string (self.xw, "load-changed", "load-committed");
 }
 
 - (void)webView:(WKWebView *)webView
@@ -343,6 +372,20 @@ - (void)userContentController:(WKUserContentController *)userContentController
   }
 }
 
+double
+nsxwidget_webkit_estimated_load_progress(struct xwidget *xw)
+{
+  XwWebView *xwWebView = (XwWebView *) xw->xwWidget;
+  return xwWebView.estimatedProgress;
+}
+
+void
+nsxwidget_webkit_stop_loading (struct xwidget *xw)
+{
+  XwWebView *xwWebView = (XwWebView *) xw->xwWidget;
+  [xwWebView stopLoading];
+}
+
 void
 nsxwidget_webkit_zoom (struct xwidget *xw, double zoom_change)
 {
@@ -452,7 +495,8 @@ - (BOOL)isFlipped { return YES; }
   NSRect rect = NSMakeRect (0, 0, xw->width, xw->height);
   xw->xwWidget = [[XwWebView alloc]
                    initWithFrame:rect
-                   configuration:[[WKWebViewConfiguration alloc] init]
+                   configuration:[[[WKWebViewConfiguration alloc] init]
+                                   autorelease]
                          xwidget:xw];
   xw->xwWindow = [[XwWindow alloc]
                    initWithFrame:rect];
@@ -470,16 +514,18 @@ - (BOOL)isFlipped { return YES; }
         ((XwWebView *) xw->xwWidget).configuration.userContentController;
       [scriptor removeAllUserScripts];
       [scriptor removeScriptMessageHandlerForName:@"keyDown"];
-      [scriptor release];
+
       if (xw->xv)
         xw->xv->model = Qnil; /* Make sure related view stale.  */
 
       /* This stops playing audio when a xwidget-webkit buffer is
-         killed.  I could not find other solution.  */
+         killed.  I could not find other solution.
+         TODO: improve this */
       nsxwidget_webkit_goto_uri (xw, "about:blank");
 
       [((XwWebView *) xw->xwWidget).urlScriptBlocked release];
       [xw->xwWidget removeFromSuperviewWithoutNeedingDisplay];
+
       [xw->xwWidget release];
       [xw->xwWindow removeFromSuperviewWithoutNeedingDisplay];
       [xw->xwWindow release];
diff --git a/src/xwidget.c b/src/xwidget.c
index efe2705562..b9550da460 100644
--- a/src/xwidget.c
+++ b/src/xwidget.c
@@ -54,6 +54,7 @@ Copyright (C) 2011-2023 Free Software Foundation, Inc.
 
 #include <math.h>
 
+
 static Lisp_Object id_to_xwidget_map;
 static Lisp_Object internal_xwidget_view_list;
 static Lisp_Object internal_xwidget_list;
@@ -490,10 +491,10 @@ DEFUN ("xwidget-perform-lispy-event",
 {
   struct xwidget *xw;
   struct frame *f = NULL;
-  int character = -1, keycode = -1;
-  int modifiers = 0;
 
 #ifdef USE_GTK
+  int character = -1, keycode = -1;
+  int modifiers = 0;
   GdkEvent *xg_event;
   GtkContainerClass *klass;
   GtkWidget *widget;
@@ -3063,6 +3064,36 @@ DEFUN ("xwidget-webkit-title",
 #endif
 }
 
+DEFUN ("xwidget-webkit-estimated-load-progress",
+       Fxwidget_webkit_estimated_load_progress, Sxwidget_webkit_estimated_load_progress,
+       1, 1, 0, doc: /* Get the estimated load progress of XWIDGET, a WebKit widget.
+Return a value ranging from 0.0 to 1.0, based on how close XWIDGET
+is to completely loading its page.  */)
+  (Lisp_Object xwidget)
+{
+  struct xwidget *xw;
+#ifdef USE_GTK
+  WebKitWebView *webview;
+#endif
+  double value;
+
+  CHECK_LIVE_XWIDGET (xwidget);
+  xw = XXWIDGET (xwidget);
+  CHECK_WEBKIT_WIDGET (xw);
+
+  block_input ();
+#ifdef USE_GTK
+  webview = WEBKIT_WEB_VIEW (xw->widget_osr);
+  value = webkit_web_view_get_estimated_load_progress (webview);
+#elif defined NS_IMPL_COCOA
+  value = nsxwidget_webkit_estimated_load_progress (xw);
+#endif
+
+  unblock_input ();
+
+  return make_float (value);
+}
+
 DEFUN ("xwidget-webkit-goto-uri",
        Fxwidget_webkit_goto_uri, Sxwidget_webkit_goto_uri,
        2, 2, 0,
@@ -3810,28 +3841,6 @@ DEFUN ("xwidget-webkit-back-forward-list", Fxwidget_webkit_back_forward_list,
   return list3 (back, here, forward);
 }
 
-DEFUN ("xwidget-webkit-estimated-load-progress",
-       Fxwidget_webkit_estimated_load_progress, Sxwidget_webkit_estimated_load_progress,
-       1, 1, 0, doc: /* Get the estimated load progress of XWIDGET, a WebKit widget.
-Return a value ranging from 0.0 to 1.0, based on how close XWIDGET
-is to completely loading its page.  */)
-  (Lisp_Object xwidget)
-{
-  struct xwidget *xw;
-  WebKitWebView *webview;
-  double value;
-
-  CHECK_LIVE_XWIDGET (xwidget);
-  xw = XXWIDGET (xwidget);
-  CHECK_WEBKIT_WIDGET (xw);
-
-  block_input ();
-  webview = WEBKIT_WEB_VIEW (xw->widget_osr);
-  value = webkit_web_view_get_estimated_load_progress (webview);
-  unblock_input ();
-
-  return make_float (value);
-}
 #endif
 
 DEFUN ("xwidget-webkit-set-cookie-storage-file",
@@ -3874,19 +3883,23 @@ DEFUN ("xwidget-webkit-stop-loading", Fxwidget_webkit_stop_loading,
 XWIDGET as part of loading a page.  */)
   (Lisp_Object xwidget)
 {
-#ifdef USE_GTK
   struct xwidget *xw;
+#ifdef USE_GTK
   WebKitWebView *webview;
+#endif
 
   CHECK_LIVE_XWIDGET (xwidget);
   xw = XXWIDGET (xwidget);
   CHECK_WEBKIT_WIDGET (xw);
 
   block_input ();
+#ifdef USE_GTK
   webview = WEBKIT_WEB_VIEW (xw->widget_osr);
   webkit_web_view_stop_loading (webview);
-  unblock_input ();
+#elif defined NS_IMPL_COCOA
+  nsxwidget_webkit_stop_loading (xw);
 #endif
+  unblock_input ();
 
   return Qnil;
 }
@@ -3936,8 +3949,9 @@ syms_of_xwidget (void)
 #ifdef USE_GTK
   defsubr (&Sxwidget_webkit_load_html);
   defsubr (&Sxwidget_webkit_back_forward_list);
-  defsubr (&Sxwidget_webkit_estimated_load_progress);
 #endif
+
+  defsubr (&Sxwidget_webkit_estimated_load_progress);
   defsubr (&Skill_xwidget);
 
   DEFSYM (QCxwidget, ":xwidget");

^ permalink raw reply related	[flat|nested] 20+ messages in thread
end of thread, other threads:[~2023-03-05  0:07 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-10  5:14 bug#60703: Patches to xwidget code Andrew De Angelis
2023-01-10  9:59 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
     [not found] ` <87cz7mhfrr.fsf@yahoo.com>
2023-01-10 13:40   ` Eli Zaretskii
2023-01-10 23:33     ` Andrew De Angelis
2023-01-11  1:15     ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-01-21  6:13       ` Andrew De Angelis
2023-01-21  7:16         ` Eli Zaretskii
2023-01-21 15:32           ` Andrew De Angelis
2023-01-21 15:41             ` Eli Zaretskii
2023-01-26 23:45               ` Andrew De Angelis
2023-01-27  0:44                 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-19 21:00                   ` Andrew De Angelis
2023-02-20  2:41                     ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-20 11:47                     ` Eli Zaretskii
2023-02-21  5:24                       ` Andrew De Angelis
2023-02-22 13:27                         ` Eli Zaretskii
2023-02-24  3:47                           ` Andrew De Angelis
2023-03-02 10:57                             ` Eli Zaretskii
2023-03-04 23:00                               ` Andrew De Angelis
2023-03-05  0:07                                 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).