Re: [PATCH] ui/cocoa: Add clipboard support

[Akihiko Odaki]

On Wed, Feb 9, 2022 at 2:54 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Wed, 16 Jun 2021 at 15:20, Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
> >
> > Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
>
> Hi Akihiko -- I have a similar question here to the other
> patch about doing things not on the Cocoa UI thread...
>
> > +static void cocoa_clipboard_notify(Notifier *notifier, void *data)
> > +{
> > +    QemuClipboardInfo *info = data;
> > +
> > +    if (info->owner == &cbpeer || info->selection != 
> > QEMU_CLIPBOARD_SELECTION_CLIPBOARD) {
> > +        return;
> > +    }
> > +
> > +    if (info != cbinfo) {
> > +        NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
> > +        qemu_clipboard_info_unref(cbinfo);
> > +        cbinfo = qemu_clipboard_info_ref(info);
> > +        cbchangecount = [[NSPasteboard generalPasteboard] 
> > declareTypes:@[NSPasteboardTypeString] owner:cbowner];
> > +        [pool release];
>
> Is this OK to do on a non-Cocoa thread with an autorelease pool,
> or should it be done via dispatch_async ?
>
> > +    }
> > +
> > +    qemu_event_set(&cbevent);
> > +}
>
> >  /*
> >   * The startup process for the OSX/Cocoa UI is complicated, because
> >   * OSX insists that the UI runs on the initial main thread, and so we
> > @@ -1845,6 +1937,7 @@ static void addRemovableDevicesMenuItems(void)
> >      COCOA_DEBUG("Second thread: calling qemu_main()\n");
> >      status = qemu_main(gArgc, gArgv, *_NSGetEnviron());
> >      COCOA_DEBUG("Second thread: qemu_main() returned, exiting\n");
> > +    [cbowner release];
> >      exit(status);
> >  }
> >
> > @@ -1965,6 +2058,18 @@ static void cocoa_refresh(DisplayChangeListener *dcl)
> >              [cocoaView setAbsoluteEnabled:YES];
> >          });
> >      }
> > +
> > +    if (cbchangecount != [[NSPasteboard generalPasteboard] changeCount]) {
> > +        qemu_clipboard_info_unref(cbinfo);
> > +        cbinfo = qemu_clipboard_info_new(&cbpeer, 
> > QEMU_CLIPBOARD_SELECTION_CLIPBOARD);
> > +        if ([[NSPasteboard generalPasteboard] 
> > availableTypeFromArray:@[NSPasteboardTypeString]]) {
> > +            cbinfo->types[QEMU_CLIPBOARD_TYPE_TEXT].available = true;
> > +        }
> > +        qemu_clipboard_update(cbinfo);
> > +        cbchangecount = [[NSPasteboard generalPasteboard] changeCount];
> > +        qemu_event_set(&cbevent);
> > +    }
> > +
>
> This work in the cocoa_refresh() function is done not on the Cocoa
> UI thread. Is it OK for it to do that, or should we put it into a
> dispatch_async block ?
>
> >      [pool release];
> >  }
>
> thanks
> -- PMM

It should be fine since it doesn't touch NSView. The following
documentation is the latest one which I have found so far:
https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Multithreading/ThreadSafetySummary/ThreadSafetySummary.html#//apple_ref/doc/uid/10000057i-CH12-SW1

It is unfortunate that Apple no longer updates it. They should at
least note about the main thread requirement in the documentation of
NSView.

Regards,
Akihiko Odaki