Bug 234266

Summary: Attachment does not update after markup.
Product: WebKit Reporter: Megan Gardner <megan_gardner>
Component: New BugsAssignee: Megan Gardner <megan_gardner>
Status: RESOLVED FIXED    
Severity: Normal CC: webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
wenson_hsieh: review+
Patch
none
Patch none

Description Megan Gardner 2021-12-13 14:08:09 PST
Attachment does not update after markup.
Comment 1 Megan Gardner 2021-12-13 14:11:20 PST
Created attachment 447062 [details]
Patch
Comment 2 Wenson Hsieh 2021-12-13 14:15:26 PST
Comment on attachment 447062 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=447062&action=review

> Source/WebKit/UIProcess/mac/WKSharingServicePickerDelegate.mm:158
> +            _menuProxy->page()->didInvalidateDataForAttachment(*apiAttachment.get());

Not new in this patch, but we should weakly capture WebPageProxy here (instead of accessing it through the raw pointer to _menuProxy.)
Comment 3 Wenson Hsieh 2021-12-13 14:16:14 PST
Comment on attachment 447062 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=447062&action=review

>> Source/WebKit/UIProcess/mac/WKSharingServicePickerDelegate.mm:158
>> +            _menuProxy->page()->didInvalidateDataForAttachment(*apiAttachment.get());
> 
> Not new in this patch, but we should weakly capture WebPageProxy here (instead of accessing it through the raw pointer to _menuProxy.)

(Let's also add a null check for `apiAttachment`, since it's not guaranteed that the attachment ID will map to an actual API::Attachment here)
Comment 4 Megan Gardner 2021-12-13 14:35:32 PST
Created attachment 447068 [details]
Patch
Comment 5 Wenson Hsieh 2021-12-13 14:39:55 PST
Comment on attachment 447068 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=447068&action=review

> Source/WebKit/UIProcess/mac/WKSharingServicePickerDelegate.mm:166
>          [itemProvider loadDataRepresentationForTypeIdentifier:(NSString *)kUTTypeData completionHandler:^(NSData *data, NSError *error) {
> +            WeakPtr webPage = _menuProxy->page();
> +            
> +            if (!webPage)
> +                return;
> +            
>              if (error)
>                  return;
>              
> -            auto apiAttachment = _menuProxy->page()->attachmentForIdentifier(_attachmentID);
> +            auto apiAttachment = webPage->attachmentForIdentifier(_attachmentID);
> +            if (!apiAttachment)
> +                return;
> +            
>              auto attachment = wrapper(apiAttachment);
>              [attachment setData:data newContentType:String(NSPasteboardTypeTIFF)];
> +            webPage->didInvalidateDataForAttachment(*apiAttachment.get());

You probably want something like this:

```
WeakPtr weakPage = _menuProxy->page();
[itemProvider loadDataRepresentationForTypeIdentifier:(NSString *)kUTTypeData completionHandler:[weakPage, attachmentID = _attachmentID](NSData *data, NSError *error) {
    RefPtr webPage = weakPage.get();
    if (!webPage)
        return;

    if (error)
        return;

    auto apiAttachment = webPage->attachmentForIdentifier(attachmentID);
    if (!apiAttachment)
        return;

    auto attachment = wrapper(apiAttachment);
    [attachment setData:data newContentType:String(NSPasteboardTypeTIFF)];
    webPage->didInvalidateDataForAttachment(*apiAttachment.get());
}];
```
Comment 6 Megan Gardner 2021-12-13 14:54:18 PST
Created attachment 447072 [details]
Patch
Comment 7 Wenson Hsieh 2021-12-13 14:57:52 PST
Comment on attachment 447072 [details]
Patch

r=mews
Comment 8 EWS 2021-12-14 11:24:18 PST
Committed r287034 (245234@main): <https://commits.webkit.org/245234@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 447072 [details].
Comment 9 Radar WebKit Bug Importer 2021-12-14 11:25:16 PST
<rdar://problem/86480352>