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

List:       pykde
Subject:    Re: Bug in QWebEngineProfile::setNotificationPresenter
From:       Phil Thompson <phil () riverbankcomputing ! com>
Date:       2020-05-22 14:07:28
Message-ID: 792a2299ad3a512f6308d4ecf253ece1 () riverbankcomputing ! com
[Download RAW message or body]

On 22/05/2020 11:03, Florian Bruhin wrote:
> On Fri, May 22, 2020 at 09:35:55AM +0100, Phil Thompson wrote:
> > On 21/05/2020 10:42, Florian Bruhin wrote:
> > > Hi,
> > > 
> > > When running the attached example and clicking "authorize", then "show"
> > > 2-3
> > > times, either a segfault or an exception like this will happen:
> > > 
> > > TypeError: 'QWebEngineNotification' object is not callable
> > > 
> > > As a workaround, show_notification_2 can be used instead, which
> > > re-registers
> > > the callback and makes things work fine.
> > > 
> > > Looking at the implementation of setNotificationPresenter in
> > > qwebengineprofile.sip, it does "Py_DECREF(a0);", inside its wrapper,
> > > with a0
> > > being the passed function. That seems to be wrong, as the function gets
> > > called
> > > multiple times with multiple notifications.
> > > 
> > > Compare that to e.g. setCookieFilter in qwebenginecookiestore.sip where
> > > the
> > > callback isn't DECREF'd.
> > 
> > Should be fixed in tonight's snapshot.
> 
> Thanks! There seems to be a similiar issue surrounding the lifetime of 
> the
> QWebEngineNotifcation object, though.
> 
> Based on Qt's example, it should be possible to store the notification 
> in the
> presenter and later e.g. call .click() or .close() on it:
> 
> https://github.com/qt/qtwebengine/blob/v5.14.2/examples/webenginewidgets/notifications/notificationpopup.h#L132
>  
> However, when doing the same in PyQt (like simulated in the attached 
> example),
> it looks like PyQt already deleted the underlying 
> QWebEngineNotification object
> (probably because it uses an unique_ptr in its callback?):
> 
> RuntimeError: wrapped C/C++ object of type QWebEngineNotification has
> been deleted
> 
> Florian

I've changed it so that it takes over the ownership of the 
QWebEngineNotication object.

Phil


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