Changeset 238566 in webkit


Ignore:
Timestamp:
Nov 27, 2018 12:20:18 PM (5 years ago)
Author:
timothy_horton@apple.com
Message:

Serialize and deserialize editable image strokes
​https://bugs.webkit.org/show_bug.cgi?id=192002
<rdar://problem/30900149>

Reviewed by Dean Jackson.

Source/WebCore:

Test: editing/images/paste-editable-image.html

  • html/HTMLImageElement.cpp:

(WebCore::HTMLImageElement::parseAttribute):
(WebCore::HTMLImageElement::insertedIntoAncestor):
(WebCore::HTMLImageElement::didFinishInsertingNode):
(WebCore::HTMLImageElement::removedFromAncestor):
(WebCore::HTMLImageElement::hasEditableImageAttribute const):
(WebCore::HTMLImageElement::updateEditableImage):
Call updateEditableImage() from didFinishInsertingNode instead of insertedIntoAncestor.
This is helpful because it means we get the final, deduplicated attachment identifier
instead of the original one when cloning or pasting.

This also means that isConnected() is now always accurate when updateEditableImage()
is called, so we can remove the argument that allowed us to lie from inside insertedIntoAncestor.

  • html/HTMLImageElement.h:
  • rendering/RenderImage.cpp:

(WebCore::RenderImage::isEditableImage const):
Make use of hasEditableImage instead of separately checking for the attribute.

Source/WebKit:

  • UIProcess/API/APIAttachment.cpp:

(API::Attachment::updateAttributes):

  • UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::willUpdateAttachmentAttributes):

  • UIProcess/WebPageProxy.h:

When an attachment would update its DOM attributes, plumb a notification
to EditableImageController, and allow it to block the update (because
we don't really want to set src for editable image attachments,
we just want the UI process to fully own the data).

  • Platform/spi/ios/PencilKitSPI.h:

Add some SPI.

  • UIProcess/ios/EditableImageController.h:
  • UIProcess/ios/EditableImageController.mm:

(WebKit::EditableImageController::loadStrokesFromAttachment):
Add a helper to load strokes from an attachment.

(WebKit::EditableImageController::associateWithAttachment):
Try to load strokes from the attachment when initially associated.
Don't create a file wrapper around a null image, so it will be regenerated later.

(WebKit::EditableImageController::willUpdateAttachmentAttributes):
The aforementioned plumbing at update time.

  • UIProcess/ios/WKDrawingView.h:
  • UIProcess/ios/WKDrawingView.mm:

(-[WKDrawingView layoutSubviews]):
Invalidate the attachment (so it will be regenerated upon request) if the
canvas size changes.

(-[WKDrawingView PNGRepresentation]):
Serialize strokes into the EXIF User Comment field.
We will find a different field to use (ideally a custom vendor-specific
field that nobody else will use for anything), but this works for now.

Don't try to render an image if we don't have a size or scale;
PKImageRenderer will just fail anyway, so bail early.

In the iOS Simulator, PKImageRenderer currently returns an unusable image.
Instead, so that we have a image on which to serialize the strokes,
create a transparent 1x1 image. This makes it possible to serialize strokes
even though we don't have a usable rendered image, so that we can still test
this change (and future changes).

(-[WKDrawingView loadDrawingFromPNGRepresentation:]):
If available, deserialize strokes from the EXIF User Comment field.

(-[WKDrawingView drawingDidChange:]):
(-[WKDrawingView invalidateAttachment]):
Factor invalidateAttachment out of drawingDidChange so we can call
it from layoutSubviews too!

LayoutTests:

  • editing/images/paste-editable-image-expected.txt: Added.
  • editing/images/paste-editable-image.html: Added.

Add a test that we can copy and paste and editable image and
continue to edit it, and are affecting a different attachment than the original.

Location:
trunk
Files:
2 added
14 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r238562 r238566  
     12018-11-27  Tim Horton  <timothy_horton@apple.com>
     2
     3        Serialize and deserialize editable image strokes
     4        https://bugs.webkit.org/show_bug.cgi?id=192002
     5        <rdar://problem/30900149>
     6
     7        Reviewed by Dean Jackson.
     8
     9        * editing/images/paste-editable-image-expected.txt: Added.
     10        * editing/images/paste-editable-image.html: Added.
     11        Add a test that we can copy and paste and editable image and
     12        continue to edit it, and are affecting a different attachment than the original.
     13
    1142018-11-16  Jiewen Tan  <jiewen_tan@apple.com>
    215

  • trunk/Source/WebCore/ChangeLog

    r238562 r238566  
     12018-11-27  Tim Horton  <timothy_horton@apple.com>
     2
     3        Serialize and deserialize editable image strokes
     4        https://bugs.webkit.org/show_bug.cgi?id=192002
     5        <rdar://problem/30900149>
     6
     7        Reviewed by Dean Jackson.
     8
     9        Test: editing/images/paste-editable-image.html
     10
     11        * html/HTMLImageElement.cpp:
     12        (WebCore::HTMLImageElement::parseAttribute):
     13        (WebCore::HTMLImageElement::insertedIntoAncestor):
     14        (WebCore::HTMLImageElement::didFinishInsertingNode):
     15        (WebCore::HTMLImageElement::removedFromAncestor):
     16        (WebCore::HTMLImageElement::hasEditableImageAttribute const):
     17        (WebCore::HTMLImageElement::updateEditableImage):
     18        Call updateEditableImage() from didFinishInsertingNode instead of insertedIntoAncestor.
     19        This is helpful because it means we get the final, deduplicated attachment identifier
     20        instead of the original one when cloning or pasting.
     21
     22        This also means that isConnected() is now always accurate when updateEditableImage()
     23        is called, so we can remove the argument that allowed us to lie from inside insertedIntoAncestor.
     24
     25        * html/HTMLImageElement.h:
     26        * rendering/RenderImage.cpp:
     27        (WebCore::RenderImage::isEditableImage const):
     28        Make use of hasEditableImage instead of separately checking for the attribute.
     29
    1302018-11-16  Jiewen Tan  <jiewen_tan@apple.com>
    231

  • trunk/Source/WebCore/html/HTMLImageElement.cpp

    r238538 r238566  
    244244#endif
    245245    } else if (name == x_apple_editable_imageAttr)
    246         updateEditableImage(isConnected() ? IsConnectedToDocument::Yes : IsConnectedToDocument::No);
     246        updateEditableImage();
    247247    else {
    248248        if (name == nameAttr) {
    … …  
    334334    }
    335335
    336     if (insertionType.connectedToDocument)
    337         updateEditableImage(IsConnectedToDocument::Yes);
    338 
    339336    // Insert needs to complete first, before we start updating the loader. Loader dispatches events which could result
    340337    // in callbacks back to this node.
    341338    Node::InsertedIntoAncestorResult insertNotificationRequest = HTMLElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
     339
     340    if (insertionType.connectedToDocument && hasEditableImageAttribute())
     341        insertNotificationRequest = InsertedIntoAncestorResult::NeedsPostInsertionCallback;
    342342
    343343    if (insertionType.connectedToDocument && !m_parsedUsemap.isNull())
    … …  
    357357}
    358358
     359void HTMLImageElement::didFinishInsertingNode()
     360{
     361    if (hasEditableImageAttribute())
     362        updateEditableImage();
     363}
     364
    359365void HTMLImageElement::removedFromAncestor(RemovalType removalType, ContainerNode& oldParentOfRemovedTree)
    360366{
    … …  
    369375
    370376    if (removalType.disconnectedFromDocument)
    371         updateEditableImage(IsConnectedToDocument::No);
     377        updateEditableImage();
    372378
    373379    m_form = nullptr;
    374380    HTMLElement::removedFromAncestor(removalType, oldParentOfRemovedTree);
     381}
     382
     383bool HTMLImageElement::hasEditableImageAttribute() const
     384{
     385    if (!document().settings().editableImagesEnabled())
     386        return false;
     387    return hasAttributeWithoutSynchronization(x_apple_editable_imageAttr);
    375388}
    376389
    … …  
    382395}
    383396
    384 void HTMLImageElement::updateEditableImage(IsConnectedToDocument connected)
     397void HTMLImageElement::updateEditableImage()
    385398{
    386399    if (!document().settings().editableImagesEnabled())
    … …  
    391404        return;
    392405
    393     bool hasEditableAttribute = hasAttributeWithoutSynchronization(x_apple_editable_imageAttr);
     406    bool hasEditableAttribute = hasEditableImageAttribute();
    394407    bool isCurrentlyEditable = !!m_editableImage;
    395     bool shouldBeEditable = (connected == IsConnectedToDocument::Yes) && hasEditableAttribute;
     408    bool shouldBeEditable = isConnected() && hasEditableAttribute;
    396409
    397410#if ENABLE(ATTACHMENT_ELEMENT)

  • trunk/Source/WebCore/html/HTMLImageElement.h

    r238538 r238566  
    117117
    118118    GraphicsLayer::EmbeddedViewID editableImageViewID() const;
     119    bool hasEditableImageAttribute() const;
    119120
    120121protected:
    … …  
    143144
    144145    InsertedIntoAncestorResult insertedIntoAncestor(InsertionType, ContainerNode&) override;
     146    void didFinishInsertingNode() override;
    145147    void removedFromAncestor(RemovalType, ContainerNode&) override;
    146148
    … …  
    154156    ImageCandidate bestFitSourceFromPictureElement();
    155157
    156     enum class IsConnectedToDocument : bool { No, Yes };
    157     void updateEditableImage(IsConnectedToDocument);
     158    void updateEditableImage();
    158159
    159160    void copyNonAttributePropertiesFromElement(const Element&) final;

  • trunk/Source/WebCore/rendering/RenderImage.cpp

    r238463 r238566  
    215215bool RenderImage::isEditableImage() const
    216216{
    217     if (!settings().editableImagesEnabled())
    218         return false;
    219 
    220     if (!element())
    221         return false;
    222 
    223     return element()->hasAttributeWithoutSynchronization(x_apple_editable_imageAttr);
     217    if (!element() || !is<HTMLImageElement>(element()))
     218        return false;
     219    return downcast<HTMLImageElement>(element())->hasEditableImageAttribute();
    224220}
    225221   

  • trunk/Source/WebKit/ChangeLog

    r238565 r238566  
     12018-11-27  Tim Horton  <timothy_horton@apple.com>
     2
     3        Serialize and deserialize editable image strokes
     4        https://bugs.webkit.org/show_bug.cgi?id=192002
     5        <rdar://problem/30900149>
     6
     7        Reviewed by Dean Jackson.
     8
     9        * UIProcess/API/APIAttachment.cpp:
     10        (API::Attachment::updateAttributes):
     11        * UIProcess/WebPageProxy.cpp:
     12        (WebKit::WebPageProxy::willUpdateAttachmentAttributes):
     13        * UIProcess/WebPageProxy.h:
     14        When an attachment would update its DOM attributes, plumb a notification
     15        to EditableImageController, and allow it to block the update (because
     16        we don't really want to set src for editable image attachments,
     17        we just want the UI process to fully own the data).
     18
     19        * Platform/spi/ios/PencilKitSPI.h:
     20        Add some SPI.
     21
     22        * UIProcess/ios/EditableImageController.h:
     23        * UIProcess/ios/EditableImageController.mm:
     24        (WebKit::EditableImageController::loadStrokesFromAttachment):
     25        Add a helper to load strokes from an attachment.
     26
     27        (WebKit::EditableImageController::associateWithAttachment):
     28        Try to load strokes from the attachment when initially associated.
     29        Don't create a file wrapper around a null image, so it will be regenerated later.
     30
     31        (WebKit::EditableImageController::willUpdateAttachmentAttributes):
     32        The aforementioned plumbing at update time.
     33
     34        * UIProcess/ios/WKDrawingView.h:
     35        * UIProcess/ios/WKDrawingView.mm:
     36        (-[WKDrawingView layoutSubviews]):
     37        Invalidate the attachment (so it will be regenerated upon request) if the
     38        canvas size changes.
     39
     40        (-[WKDrawingView PNGRepresentation]):
     41        Serialize strokes into the EXIF User Comment field.
     42        We will find a different field to use (ideally a custom vendor-specific
     43        field that nobody else will use for anything), but this works for now.
     44
     45        Don't try to render an image if we don't have a size or scale;
     46        PKImageRenderer will just fail anyway, so bail early.
     47
     48        In the iOS Simulator, PKImageRenderer currently returns an unusable image.
     49        Instead, so that we have a image on which to serialize the strokes,
     50        create a transparent 1x1 image. This makes it possible to serialize strokes
     51        even though we don't have a usable rendered image, so that we can still test
     52        this change (and future changes).
     53
     54        (-[WKDrawingView loadDrawingFromPNGRepresentation:]):
     55        If available, deserialize strokes from the EXIF User Comment field.
     56
     57        (-[WKDrawingView drawingDidChange:]):
     58        (-[WKDrawingView invalidateAttachment]):
     59        Factor invalidateAttachment out of drawingDidChange so we can call
     60        it from layoutSubviews too!
     61
    1622018-11-27  Chris Dumez  <cdumez@apple.com>
    263

  • trunk/Source/WebKit/Platform/spi/ios/PencilKitSPI.h

    r238474 r238566  
    2929
    3030#import <PencilKit/PencilKit.h>
     31#import <PencilKit/PKDrawing_Private.h>
    3132
    3233#else
    … …  
    3839@end
    3940
     41@interface PKDrawing : NSObject
     42
     43- (NSData *)serialize;
     44
     45@end
     46
    4047#endif
    4148

  • trunk/Source/WebKit/UIProcess/API/APIAttachment.cpp

    r237624 r238566  
    5353    if (!m_webPage) {
    5454        callback(WebKit::CallbackBase::Error::OwnerWasInvalidated);
     55        return;
     56    }
     57
     58    if (m_webPage->willUpdateAttachmentAttributes(*this) == WebKit::WebPageProxy::ShouldUpdateAttachmentAttributes::No) {
     59        callback(WebKit::CallbackBase::Error::None);
    5560        return;
    5661    }

  • trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

    r238565 r238566  
    81098109}
    81108110
     8111WebPageProxy::ShouldUpdateAttachmentAttributes WebPageProxy::willUpdateAttachmentAttributes(const API::Attachment& attachment)
     8112{
     8113#if HAVE(PENCILKIT)
     8114    return m_editableImageController->willUpdateAttachmentAttributes(attachment);
     8115#else
     8116    return ShouldUpdateAttachmentAttributes::Yes;
     8117#endif
     8118}
     8119
    81118120#if !PLATFORM(COCOA)
    81128121

  • trunk/Source/WebKit/UIProcess/WebPageProxy.h

    r238538 r238566  
    13711371    void serializedAttachmentDataForIdentifiers(const Vector<String>&, Vector<WebCore::SerializedAttachmentData>&);
    13721372    void registerAttachmentIdentifier(const String&);
     1373
     1374    enum class ShouldUpdateAttachmentAttributes : bool { No, Yes };
     1375    ShouldUpdateAttachmentAttributes willUpdateAttachmentAttributes(const API::Attachment&);
    13731376#endif
    13741377

  • trunk/Source/WebKit/UIProcess/ios/EditableImageController.h

    r238538 r238566  
    2828#if HAVE(PENCILKIT)
    2929
     30#include "APIAttachment.h"
    3031#include "MessageReceiver.h"
     32#include "WebPageProxy.h"
    3133#include <WebCore/GraphicsLayer.h>
    3234#include <wtf/Noncopyable.h>
    … …  
    3840
    3941namespace WebKit {
    40 
    41 class WebPageProxy;
    4242
    4343struct EditableImage {
    … …  
    6060    void invalidateAttachmentForEditableImage(WebCore::GraphicsLayer::EmbeddedViewID);
    6161
     62    WebPageProxy::ShouldUpdateAttachmentAttributes willUpdateAttachmentAttributes(const API::Attachment&);
     63
    6264private:
    6365    void didCreateEditableImage(WebCore::GraphicsLayer::EmbeddedViewID);
    … …  
    6567
    6668    void associateWithAttachment(WebCore::GraphicsLayer::EmbeddedViewID, const String& attachmentID);
     69
     70    void loadStrokesFromAttachment(EditableImage&, const API::Attachment&);
    6771
    6872    WeakPtr<WebPageProxy> m_webPageProxy;

  • trunk/Source/WebKit/UIProcess/ios/EditableImageController.mm

    r238538 r238566  
    9393    WeakObjCPtr<WKDrawingView> drawingView = editableImage.drawingView.get();
    9494    editableImage.attachmentID = attachmentID;
     95
     96    loadStrokesFromAttachment(editableImage, attachment);
     97
    9598    attachment.setFileWrapperGenerator([drawingView]() -> RetainPtr<NSFileWrapper> {
    9699        if (!drawingView)
    97100            return nil;
    98         RetainPtr<NSFileWrapper> fileWrapper = adoptNS([[NSFileWrapper alloc] initRegularFileWithContents:[drawingView PNGRepresentation]]);
     101        NSData *data = [drawingView PNGRepresentation];
     102        if (!data)
     103            return nil;
     104        RetainPtr<NSFileWrapper> fileWrapper = adoptNS([[NSFileWrapper alloc] initRegularFileWithContents:data]);
    99105        [fileWrapper setPreferredFilename:@"drawing.png"];
    100106        return fileWrapper;
    101107    });
    102108    attachment.setContentType("public.png");
     109}
     110
     111void EditableImageController::loadStrokesFromAttachment(EditableImage& editableImage, const API::Attachment& attachment)
     112{
     113    ASSERT(attachment.identifier() == editableImage.attachmentID);
     114    NSFileWrapper *fileWrapper = attachment.fileWrapper();
     115    if (!fileWrapper.isRegularFile)
     116        return;
     117    [editableImage.drawingView loadDrawingFromPNGRepresentation:fileWrapper.regularFileContents];
    103118}
    104119
    … …  
    120135}
    121136
     137WebPageProxy::ShouldUpdateAttachmentAttributes EditableImageController::willUpdateAttachmentAttributes(const API::Attachment& attachment)
     138{
     139    for (auto& editableImage : m_editableImages.values()) {
     140        if (editableImage->attachmentID != attachment.identifier())
     141            continue;
     142
     143        loadStrokesFromAttachment(*editableImage, attachment);
     144        return WebPageProxy::ShouldUpdateAttachmentAttributes::No;
     145    }
     146
     147    return WebPageProxy::ShouldUpdateAttachmentAttributes::Yes;
     148}
     149
    122150} // namespace WebKit
    123151

  • trunk/Source/WebKit/UIProcess/ios/WKDrawingView.h

    r238538 r238566  
    3434
    3535- (NSData *)PNGRepresentation;
     36- (void)loadDrawingFromPNGRepresentation:(NSData *)PNGData;
    3637
    3738@end

  • trunk/Source/WebKit/UIProcess/ios/WKDrawingView.mm

    r238538 r238566  
    3636SOFT_LINK_PRIVATE_FRAMEWORK(PencilKit);
    3737SOFT_LINK_CLASS(PencilKit, PKCanvasView);
     38SOFT_LINK_CLASS(PencilKit, PKDrawing);
    3839SOFT_LINK_CLASS(PencilKit, PKImageRenderer);
    3940
    … …  
    7879        // the size changes, we need to re-create the renderer.
    7980        _renderer = nil;
     81
     82        [self invalidateAttachment];
    8083    }
    8184}
    … …  
    8386- (NSData *)PNGRepresentation
    8487{
     88    if (!self.bounds.size.width || !self.bounds.size.height || !self.window.screen.scale)
     89        return nil;
     90
    8591    if (!_renderQueue)
    8692        _renderQueue = adoptOSObject(dispatch_queue_create("com.apple.WebKit.WKDrawingView.Rendering", DISPATCH_QUEUE_SERIAL));
    … …  
    8995        _renderer = adoptNS([allocPKImageRendererInstance() initWithSize:self.bounds.size scale:self.window.screen.scale renderQueue:_renderQueue.get()]);
    9096
     97    auto* drawing = [_pencilView drawing];
     98
    9199    __block RetainPtr<UIImage> resultImage;
    92     [_renderer renderDrawing:[_pencilView drawing] completion:^(UIImage *image) {
     100#if PLATFORM(IOS_FAMILY_SIMULATOR)
     101    // PKImageRenderer currently doesn't work in the simulator. In order to
     102    // allow strokes to persist regardless (mostly for testing), we'll
     103    // synthesize an empty 1x1 image.
     104    UIGraphicsBeginImageContext(CGSizeMake(1, 1));
     105    CGContextClearRect(UIGraphicsGetCurrentContext(), CGRectMake(0, 0, 1, 1));
     106    resultImage = UIGraphicsGetImageFromCurrentImageContext();
     107    UIGraphicsEndImageContext();
     108#else
     109    [_renderer renderDrawing:drawing completion:^(UIImage *image) {
    93110        resultImage = image;
    94111    }];
     112#endif
    95113
    96114    // FIXME: Ideally we would not synchronously wait for this rendering,
    … …  
    99117    dispatch_sync(_renderQueue.get(), ^{ });
    100118
    101     return UIImagePNGRepresentation(resultImage.get());
     119    RetainPtr<NSMutableData> PNGData = adoptNS([[NSMutableData alloc] init]);
     120    RetainPtr<CGImageDestinationRef> imageDestination = adoptCF(CGImageDestinationCreateWithData((__bridge CFMutableDataRef)PNGData.get(), kUTTypePNG, 1, nil));
     121    NSString *base64Drawing = [[drawing serialize] base64EncodedStringWithOptions:0];
     122    NSDictionary *properties = nil;
     123    if (base64Drawing) {
     124        // FIXME: We should put this somewhere less user-facing than the EXIF User Comment field.
     125        properties = @{
     126            (__bridge NSString *)kCGImagePropertyExifDictionary : @{
     127                (__bridge NSString *)kCGImagePropertyExifUserComment : base64Drawing
     128            }
     129        };
     130    }
     131    CGImageDestinationSetProperties(imageDestination.get(), (__bridge CFDictionaryRef)properties);
     132    CGImageDestinationAddImage(imageDestination.get(), [resultImage CGImage], (__bridge CFDictionaryRef)properties);
     133    CGImageDestinationFinalize(imageDestination.get());
     134
     135    return PNGData.autorelease();
     136}
     137
     138- (void)loadDrawingFromPNGRepresentation:(NSData *)PNGData
     139{
     140    RetainPtr<CGImageSourceRef> imageSource = adoptCF(CGImageSourceCreateWithData((__bridge CFDataRef)PNGData, nullptr));
     141    if (!imageSource)
     142        return;
     143    RetainPtr<NSDictionary> properties = adoptNS((__bridge NSDictionary *)CGImageSourceCopyPropertiesAtIndex(imageSource.get(), 0, nil));
     144    NSString *base64Drawing = [[properties objectForKey:(NSString *)kCGImagePropertyExifDictionary] objectForKey:(NSString *)kCGImagePropertyExifUserComment];
     145    if (!base64Drawing)
     146        return;
     147    RetainPtr<NSData> drawingData = adoptNS([[NSData alloc] initWithBase64EncodedString:base64Drawing options:0]);
     148    RetainPtr<PKDrawing> drawing = adoptNS([allocPKDrawingInstance() initWithData:drawingData.get() error:nil]);
     149    [_pencilView setDrawing:drawing.get()];
    102150}
    103151
    104152- (void)drawingDidChange:(PKCanvasView *)canvasView
     153{
     154    [self invalidateAttachment];
     155}
     156
     157- (void)invalidateAttachment
    105158{
    106159    if (!_webPageProxy)
Note: See TracChangeset for help on using the changeset viewer.