Re: Bug in QWebEngineProfile::setNotificationPresenter

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