unofficial mirror of emacs-devel@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
  0 siblings, 1 reply; 2+ 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] 2+ messages in thread

* Re: bug#60703: Patches to xwidget code
  2023-01-10  5:14 bug#60703: Patches to xwidget code Andrew De Angelis
@ 2023-01-10  9:59 ` Po Lu
  0 siblings, 0 replies; 2+ messages in thread
From: Po Lu @ 2023-01-10  9:59 UTC (permalink / raw)
  To: Andrew De Angelis; +Cc: emacs-devel, 60703

Andrew De Angelis <bobodeangelis@gmail.com> writes:

> 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.

Andrew, thanks for working on Emacs.

>
> 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."

This change is fine for Emacs 29.1.  Eli, WDYT?

>  @implementation XwWebView : WKWebView
>  
> +- (void)dealloc
> +{
> +  [super dealloc];
> +}

Thanks.  Please put a space between the part of the Objective-C method
that looks like a cast (I don't know what it's called, sorry) and the
name of the method.

But, is this really necessary?  Without this being defined, the runtime
will just call the super method, correct?

>  - (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");
>  }

These are also fine for Emacs 29.  However, please replace the C++-style
comments with C style ones, add spaces after the cast-like parts in the
method definition, and fill everything to column 80.  I believe the test
is that the xwidget's buffer has not been killed, and could easily be
written:

  !NILP (Fbuffer_live_p (self.xw->buffer))

as well.

>  - (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;
> +}

Please place a space between "nsxwidget_webkit_estimated_load_progress"
and the parameter list.

> +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];

These changes are also fine for Emacs 29, but the TODO seems excessive.
Eli, any comments here?

> 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>
>  
> +

Please undo this extraneous whitespace change.

>  
> +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");

None of this seems to affect anything other than the NS build, so it's
fine by me.

All in all, all your changes are fine for Emacs 29 by me.  What they
need is a proper commit message.  See the node "Style of Change Logs" in
the GNU Coding Standards for examples of how to do this:

  https://www.gnu.org/prep/standards/standards.html#Style-of-Change-Logs

Thanks again for working on Emacs.



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-01-10  9:59 UTC | newest]

Thread overview: 2+ 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

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).