Serialize and deserialize editable image strokes
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Nov 2018 20:20:18 +0000 (20:20 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Nov 2018 20:20:18 +0000 (20:20 +0000)
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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238566 268f45cc-cd09-0410-ab3c-d52691b4dbfc

16 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/images/paste-editable-image-expected.txt [new file with mode: 0644]
LayoutTests/editing/images/paste-editable-image.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLImageElement.cpp
Source/WebCore/html/HTMLImageElement.h
Source/WebCore/rendering/RenderImage.cpp
Source/WebKit/ChangeLog
Source/WebKit/Platform/spi/ios/PencilKitSPI.h
Source/WebKit/UIProcess/API/APIAttachment.cpp
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebPageProxy.h
Source/WebKit/UIProcess/ios/EditableImageController.h
Source/WebKit/UIProcess/ios/EditableImageController.mm
Source/WebKit/UIProcess/ios/WKDrawingView.h
Source/WebKit/UIProcess/ios/WKDrawingView.mm

index a97491c..48e1deb 100644 (file)
@@ -1,3 +1,16 @@
+2018-11-27  Tim Horton  <timothy_horton@apple.com>
+
+        Serialize and deserialize editable image strokes
+        https://bugs.webkit.org/show_bug.cgi?id=192002
+        <rdar://problem/30900149>
+
+        Reviewed by Dean Jackson.
+
+        * 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.
+
 2018-11-16  Jiewen Tan  <jiewen_tan@apple.com>
 
         Disallow loading webarchives as iframes
diff --git a/LayoutTests/editing/images/paste-editable-image-expected.txt b/LayoutTests/editing/images/paste-editable-image-expected.txt
new file mode 100644 (file)
index 0000000..01df861
--- /dev/null
@@ -0,0 +1,6 @@
+Had 1 stroke in editable image after drawing.
+Had 1 stroke in editable image after copying and pasting.
+Had 2 strokes in editable image after drawing on pasted image.
+Pasted image got a new attachment ID: yes.
+
+
diff --git a/LayoutTests/editing/images/paste-editable-image.html b/LayoutTests/editing/images/paste-editable-image.html
new file mode 100644 (file)
index 0000000..6c266de
--- /dev/null
@@ -0,0 +1,47 @@
+<!DOCTYPE html><!-- webkit-test-runner [ enableEditableImages=true enableAttachmentElement=true ] -->
+<head>
+<script src="../../resources/ui-helper.js"></script>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+
+addEventListener("load", async () => {
+    const div = document.getElementById("test");
+
+    const selection = window.getSelection();
+
+    selection.setPosition(div, 0);
+    testRunner.execCommand("InsertEditableImage");
+
+    await UIHelper.drawSquareInEditableImage();
+    const numberOfStrokesInEditableImageAfterDrawing = (await UIHelper.numberOfStrokesInEditableImage());
+    const firstImage = document.querySelector("img");
+    const initialAttachmentID = HTMLAttachmentElement.getAttachmentIdentifier(firstImage);
+
+    // We need to make sure that the canvas gets a non-zero size before trying to serialize it.
+    await UIHelper.ensurePresentationUpdate();
+
+    selection.setBaseAndExtent(div.firstChild, 0, div.firstChild, 1);
+    testRunner.execCommand("Copy");
+
+    selection.collapseToEnd();
+    testRunner.execCommand("Paste");
+
+    firstImage.parentElement.removeChild(firstImage);
+
+    const numberOfStrokesInEditableImageAfterPasting = (await UIHelper.numberOfStrokesInEditableImage());
+
+    await UIHelper.drawSquareInEditableImage();
+    const numberOfStrokesInEditableImageAfterSecondDrawing = (await UIHelper.numberOfStrokesInEditableImage());
+    const pastedAttachmentID = HTMLAttachmentElement.getAttachmentIdentifier(document.querySelector("img"));
+
+    const attachmentIDsAreDifferent = (initialAttachmentID != pastedAttachmentID) ? "yes" : "no";
+
+    document.getElementById("log").innerHTML = `Had ${numberOfStrokesInEditableImageAfterDrawing} stroke in editable image after drawing.<br/>Had ${numberOfStrokesInEditableImageAfterPasting} stroke in editable image after copying and pasting.<br/>Had ${numberOfStrokesInEditableImageAfterSecondDrawing} strokes in editable image after drawing on pasted image.<br/>Pasted image got a new attachment ID: ${attachmentIDsAreDifferent}.`;
+    testRunner.notifyDone();
+});
+</script>
+</head>
+<body contenteditable><div id="log"><br/></div><div id="test"><br/></div></body>
index 7f91e62..cef83dd 100644 (file)
@@ -1,3 +1,32 @@
+2018-11-27  Tim Horton  <timothy_horton@apple.com>
+
+        Serialize and deserialize editable image strokes
+        https://bugs.webkit.org/show_bug.cgi?id=192002
+        <rdar://problem/30900149>
+
+        Reviewed by Dean Jackson.
+
+        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.
+
 2018-11-16  Jiewen Tan  <jiewen_tan@apple.com>
 
         Disallow loading webarchives as iframes
index 23f5e07..19cd71d 100644 (file)
@@ -243,7 +243,7 @@ void HTMLImageElement::parseAttribute(const QualifiedName& name, const AtomicStr
         updateImageControls();
 #endif
     } else if (name == x_apple_editable_imageAttr)
-        updateEditableImage(isConnected() ? IsConnectedToDocument::Yes : IsConnectedToDocument::No);
+        updateEditableImage();
     else {
         if (name == nameAttr) {
             bool willHaveName = !value.isNull();
@@ -333,13 +333,13 @@ Node::InsertedIntoAncestorResult HTMLImageElement::insertedIntoAncestor(Insertio
             m_form->registerImgElement(this);
     }
 
-    if (insertionType.connectedToDocument)
-        updateEditableImage(IsConnectedToDocument::Yes);
-
     // Insert needs to complete first, before we start updating the loader. Loader dispatches events which could result
     // in callbacks back to this node.
     Node::InsertedIntoAncestorResult insertNotificationRequest = HTMLElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
 
+    if (insertionType.connectedToDocument && hasEditableImageAttribute())
+        insertNotificationRequest = InsertedIntoAncestorResult::NeedsPostInsertionCallback;
+
     if (insertionType.connectedToDocument && !m_parsedUsemap.isNull())
         treeScope().addImageElementByUsemap(*m_parsedUsemap.impl(), *this);
 
@@ -356,6 +356,12 @@ Node::InsertedIntoAncestorResult HTMLImageElement::insertedIntoAncestor(Insertio
     return insertNotificationRequest;
 }
 
+void HTMLImageElement::didFinishInsertingNode()
+{
+    if (hasEditableImageAttribute())
+        updateEditableImage();
+}
+
 void HTMLImageElement::removedFromAncestor(RemovalType removalType, ContainerNode& oldParentOfRemovedTree)
 {
     if (m_form)
@@ -368,12 +374,19 @@ void HTMLImageElement::removedFromAncestor(RemovalType removalType, ContainerNod
         setPictureElement(nullptr);
 
     if (removalType.disconnectedFromDocument)
-        updateEditableImage(IsConnectedToDocument::No);
+        updateEditableImage();
 
     m_form = nullptr;
     HTMLElement::removedFromAncestor(removalType, oldParentOfRemovedTree);
 }
 
+bool HTMLImageElement::hasEditableImageAttribute() const
+{
+    if (!document().settings().editableImagesEnabled())
+        return false;
+    return hasAttributeWithoutSynchronization(x_apple_editable_imageAttr);
+}
+
 GraphicsLayer::EmbeddedViewID HTMLImageElement::editableImageViewID() const
 {
     if (!m_editableImage)
@@ -381,7 +394,7 @@ GraphicsLayer::EmbeddedViewID HTMLImageElement::editableImageViewID() const
     return m_editableImage->embeddedViewID();
 }
 
-void HTMLImageElement::updateEditableImage(IsConnectedToDocument connected)
+void HTMLImageElement::updateEditableImage()
 {
     if (!document().settings().editableImagesEnabled())
         return;
@@ -390,9 +403,9 @@ void HTMLImageElement::updateEditableImage(IsConnectedToDocument connected)
     if (!page)
         return;
 
-    bool hasEditableAttribute = hasAttributeWithoutSynchronization(x_apple_editable_imageAttr);
+    bool hasEditableAttribute = hasEditableImageAttribute();
     bool isCurrentlyEditable = !!m_editableImage;
-    bool shouldBeEditable = (connected == IsConnectedToDocument::Yes) && hasEditableAttribute;
+    bool shouldBeEditable = isConnected() && hasEditableAttribute;
 
 #if ENABLE(ATTACHMENT_ELEMENT)
     // Create the inner attachment for editable images, or non-editable
index 4a40057..1357f90 100644 (file)
@@ -116,6 +116,7 @@ public:
 #endif
 
     GraphicsLayer::EmbeddedViewID editableImageViewID() const;
+    bool hasEditableImageAttribute() const;
 
 protected:
     HTMLImageElement(const QualifiedName&, Document&, HTMLFormElement* = 0);
@@ -142,6 +143,7 @@ private:
     void addSubresourceAttributeURLs(ListHashSet<URL>&) const override;
 
     InsertedIntoAncestorResult insertedIntoAncestor(InsertionType, ContainerNode&) override;
+    void didFinishInsertingNode() override;
     void removedFromAncestor(RemovalType, ContainerNode&) override;
 
     bool isFormAssociatedElement() const final { return false; }
@@ -153,8 +155,7 @@ private:
 
     ImageCandidate bestFitSourceFromPictureElement();
 
-    enum class IsConnectedToDocument : bool { No, Yes };
-    void updateEditableImage(IsConnectedToDocument);
+    void updateEditableImage();
 
     void copyNonAttributePropertiesFromElement(const Element&) final;
 
index c2d059b..0572bfa 100644 (file)
@@ -214,13 +214,9 @@ ImageSizeChangeType RenderImage::setImageSizeForAltText(CachedImage* newImage /*
 
 bool RenderImage::isEditableImage() const
 {
-    if (!settings().editableImagesEnabled())
+    if (!element() || !is<HTMLImageElement>(element()))
         return false;
-
-    if (!element())
-        return false;
-
-    return element()->hasAttributeWithoutSynchronization(x_apple_editable_imageAttr);
+    return downcast<HTMLImageElement>(element())->hasEditableImageAttribute();
 }
     
 bool RenderImage::requiresLayer() const
index d2077d7..d032547 100644 (file)
@@ -1,3 +1,64 @@
+2018-11-27  Tim Horton  <timothy_horton@apple.com>
+
+        Serialize and deserialize editable image strokes
+        https://bugs.webkit.org/show_bug.cgi?id=192002
+        <rdar://problem/30900149>
+
+        Reviewed by Dean Jackson.
+
+        * 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!
+
 2018-11-27  Chris Dumez  <cdumez@apple.com>
 
         Regression(PSON) crash under WebPageProxy::didReceiveServerRedirectForProvisionalLoadForFrame()
index 00a1e75..ee9796b 100644 (file)
@@ -28,6 +28,7 @@
 #if USE(APPLE_INTERNAL_SDK)
 
 #import <PencilKit/PencilKit.h>
+#import <PencilKit/PKDrawing_Private.h>
 
 #else
 
 
 @end
 
+@interface PKDrawing : NSObject
+
+- (NSData *)serialize;
+
+@end
+
 #endif
 
 #endif // HAVE(PENCILKIT)
index a134bfa..cf4c62e 100644 (file)
@@ -55,6 +55,11 @@ void Attachment::updateAttributes(Function<void(WebKit::CallbackBase::Error)>&&
         return;
     }
 
+    if (m_webPage->willUpdateAttachmentAttributes(*this) == WebKit::WebPageProxy::ShouldUpdateAttachmentAttributes::No) {
+        callback(WebKit::CallbackBase::Error::None);
+        return;
+    }
+
     m_webPage->updateAttachmentAttributes(*this, WTFMove(callback));
 }
 
index af2935d..75c3a9b 100644 (file)
@@ -8108,6 +8108,15 @@ void WebPageProxy::serializedAttachmentDataForIdentifiers(const Vector<String>&
     }
 }
 
+WebPageProxy::ShouldUpdateAttachmentAttributes WebPageProxy::willUpdateAttachmentAttributes(const API::Attachment& attachment)
+{
+#if HAVE(PENCILKIT)
+    return m_editableImageController->willUpdateAttachmentAttributes(attachment);
+#else
+    return ShouldUpdateAttachmentAttributes::Yes;
+#endif
+}
+
 #if !PLATFORM(COCOA)
 
 void WebPageProxy::platformRegisterAttachment(Ref<API::Attachment>&&, const String&, const IPC::DataReference&)
index c122af5..32e9f5d 100644 (file)
@@ -1370,6 +1370,9 @@ public:
     void updateAttachmentAttributes(const API::Attachment&, Function<void(CallbackBase::Error)>&&);
     void serializedAttachmentDataForIdentifiers(const Vector<String>&, Vector<WebCore::SerializedAttachmentData>&);
     void registerAttachmentIdentifier(const String&);
+
+    enum class ShouldUpdateAttachmentAttributes : bool { No, Yes };
+    ShouldUpdateAttachmentAttributes willUpdateAttachmentAttributes(const API::Attachment&);
 #endif
 
 #if ENABLE(APPLICATION_MANIFEST)
index aa9e6c8..bbf93c2 100644 (file)
@@ -27,7 +27,9 @@
 
 #if HAVE(PENCILKIT)
 
+#include "APIAttachment.h"
 #include "MessageReceiver.h"
+#include "WebPageProxy.h"
 #include <WebCore/GraphicsLayer.h>
 #include <wtf/Noncopyable.h>
 #include <wtf/WeakObjCPtr.h>
@@ -38,8 +40,6 @@ OBJC_CLASS WKDrawingView;
 
 namespace WebKit {
 
-class WebPageProxy;
-
 struct EditableImage {
     RetainPtr<WKDrawingView> drawingView;
     String attachmentID;
@@ -59,12 +59,16 @@ public:
 
     void invalidateAttachmentForEditableImage(WebCore::GraphicsLayer::EmbeddedViewID);
 
+    WebPageProxy::ShouldUpdateAttachmentAttributes willUpdateAttachmentAttributes(const API::Attachment&);
+
 private:
     void didCreateEditableImage(WebCore::GraphicsLayer::EmbeddedViewID);
     void didDestroyEditableImage(WebCore::GraphicsLayer::EmbeddedViewID);
 
     void associateWithAttachment(WebCore::GraphicsLayer::EmbeddedViewID, const String& attachmentID);
 
+    void loadStrokesFromAttachment(EditableImage&, const API::Attachment&);
+
     WeakPtr<WebPageProxy> m_webPageProxy;
 
     HashMap<WebCore::GraphicsLayer::EmbeddedViewID, std::unique_ptr<EditableImage>> m_editableImages;
index 50a54d2..2e51e31 100644 (file)
@@ -92,16 +92,31 @@ void EditableImageController::associateWithAttachment(WebCore::GraphicsLayer::Em
     auto& editableImage = ensureEditableImage(embeddedViewID);
     WeakObjCPtr<WKDrawingView> drawingView = editableImage.drawingView.get();
     editableImage.attachmentID = attachmentID;
+
+    loadStrokesFromAttachment(editableImage, attachment);
+
     attachment.setFileWrapperGenerator([drawingView]() -> RetainPtr<NSFileWrapper> {
         if (!drawingView)
             return nil;
-        RetainPtr<NSFileWrapper> fileWrapper = adoptNS([[NSFileWrapper alloc] initRegularFileWithContents:[drawingView PNGRepresentation]]);
+        NSData *data = [drawingView PNGRepresentation];
+        if (!data)
+            return nil;
+        RetainPtr<NSFileWrapper> fileWrapper = adoptNS([[NSFileWrapper alloc] initRegularFileWithContents:data]);
         [fileWrapper setPreferredFilename:@"drawing.png"];
         return fileWrapper;
     });
     attachment.setContentType("public.png");
 }
 
+void EditableImageController::loadStrokesFromAttachment(EditableImage& editableImage, const API::Attachment& attachment)
+{
+    ASSERT(attachment.identifier() == editableImage.attachmentID);
+    NSFileWrapper *fileWrapper = attachment.fileWrapper();
+    if (!fileWrapper.isRegularFile)
+        return;
+    [editableImage.drawingView loadDrawingFromPNGRepresentation:fileWrapper.regularFileContents];
+}
+
 void EditableImageController::invalidateAttachmentForEditableImage(WebCore::GraphicsLayer::EmbeddedViewID embeddedViewID)
 {
     if (!m_webPageProxy)
@@ -119,6 +134,19 @@ void EditableImageController::invalidateAttachmentForEditableImage(WebCore::Grap
     attachment->invalidateGeneratedFileWrapper();
 }
 
+WebPageProxy::ShouldUpdateAttachmentAttributes EditableImageController::willUpdateAttachmentAttributes(const API::Attachment& attachment)
+{
+    for (auto& editableImage : m_editableImages.values()) {
+        if (editableImage->attachmentID != attachment.identifier())
+            continue;
+
+        loadStrokesFromAttachment(*editableImage, attachment);
+        return WebPageProxy::ShouldUpdateAttachmentAttributes::No;
+    }
+
+    return WebPageProxy::ShouldUpdateAttachmentAttributes::Yes;
+}
+
 } // namespace WebKit
 
 #endif // HAVE(PENCILKIT)
index 3f48bca..86b231a 100644 (file)
@@ -33,6 +33,7 @@
 - (instancetype)initWithEmbeddedViewID:(WebCore::GraphicsLayer::EmbeddedViewID)embeddedViewID webPageProxy:(WebKit::WebPageProxy&)webPageProxy;
 
 - (NSData *)PNGRepresentation;
+- (void)loadDrawingFromPNGRepresentation:(NSData *)PNGData;
 
 @end
 
index 70f3ae7..4ea77e7 100644 (file)
@@ -35,6 +35,7 @@
 
 SOFT_LINK_PRIVATE_FRAMEWORK(PencilKit);
 SOFT_LINK_CLASS(PencilKit, PKCanvasView);
+SOFT_LINK_CLASS(PencilKit, PKDrawing);
 SOFT_LINK_CLASS(PencilKit, PKImageRenderer);
 
 @interface WKDrawingView () <PKCanvasViewDelegate>
@@ -77,32 +78,84 @@ SOFT_LINK_CLASS(PencilKit, PKImageRenderer);
         // The renderer is instantiated for a particular size output; if
         // the size changes, we need to re-create the renderer.
         _renderer = nil;
+
+        [self invalidateAttachment];
     }
 }
 
 - (NSData *)PNGRepresentation
 {
+    if (!self.bounds.size.width || !self.bounds.size.height || !self.window.screen.scale)
+        return nil;
+
     if (!_renderQueue)
         _renderQueue = adoptOSObject(dispatch_queue_create("com.apple.WebKit.WKDrawingView.Rendering", DISPATCH_QUEUE_SERIAL));
 
     if (!_renderer)
         _renderer = adoptNS([allocPKImageRendererInstance() initWithSize:self.bounds.size scale:self.window.screen.scale renderQueue:_renderQueue.get()]);
 
+    auto* drawing = [_pencilView drawing];
+
     __block RetainPtr<UIImage> resultImage;
-    [_renderer renderDrawing:[_pencilView drawing] completion:^(UIImage *image) {
+#if PLATFORM(IOS_FAMILY_SIMULATOR)
+    // PKImageRenderer currently doesn't work in the simulator. In order to
+    // allow strokes to persist regardless (mostly for testing), we'll
+    // synthesize an empty 1x1 image.
+    UIGraphicsBeginImageContext(CGSizeMake(1, 1));
+    CGContextClearRect(UIGraphicsGetCurrentContext(), CGRectMake(0, 0, 1, 1));
+    resultImage = UIGraphicsGetImageFromCurrentImageContext();
+    UIGraphicsEndImageContext();
+#else
+    [_renderer renderDrawing:drawing completion:^(UIImage *image) {
         resultImage = image;
     }];
+#endif
 
     // FIXME: Ideally we would not synchronously wait for this rendering,
     // but NSFileWrapper requires data synchronously, and our clients expect
     // an NSFileWrapper to be available synchronously.
     dispatch_sync(_renderQueue.get(), ^{ });
 
-    return UIImagePNGRepresentation(resultImage.get());
+    RetainPtr<NSMutableData> PNGData = adoptNS([[NSMutableData alloc] init]);
+    RetainPtr<CGImageDestinationRef> imageDestination = adoptCF(CGImageDestinationCreateWithData((__bridge CFMutableDataRef)PNGData.get(), kUTTypePNG, 1, nil));
+    NSString *base64Drawing = [[drawing serialize] base64EncodedStringWithOptions:0];
+    NSDictionary *properties = nil;
+    if (base64Drawing) {
+        // FIXME: We should put this somewhere less user-facing than the EXIF User Comment field.
+        properties = @{
+            (__bridge NSString *)kCGImagePropertyExifDictionary : @{
+                (__bridge NSString *)kCGImagePropertyExifUserComment : base64Drawing
+            }
+        };
+    }
+    CGImageDestinationSetProperties(imageDestination.get(), (__bridge CFDictionaryRef)properties);
+    CGImageDestinationAddImage(imageDestination.get(), [resultImage CGImage], (__bridge CFDictionaryRef)properties);
+    CGImageDestinationFinalize(imageDestination.get());
+
+    return PNGData.autorelease();
+}
+
+- (void)loadDrawingFromPNGRepresentation:(NSData *)PNGData
+{
+    RetainPtr<CGImageSourceRef> imageSource = adoptCF(CGImageSourceCreateWithData((__bridge CFDataRef)PNGData, nullptr));
+    if (!imageSource)
+        return;
+    RetainPtr<NSDictionary> properties = adoptNS((__bridge NSDictionary *)CGImageSourceCopyPropertiesAtIndex(imageSource.get(), 0, nil));
+    NSString *base64Drawing = [[properties objectForKey:(NSString *)kCGImagePropertyExifDictionary] objectForKey:(NSString *)kCGImagePropertyExifUserComment];
+    if (!base64Drawing)
+        return;
+    RetainPtr<NSData> drawingData = adoptNS([[NSData alloc] initWithBase64EncodedString:base64Drawing options:0]);
+    RetainPtr<PKDrawing> drawing = adoptNS([allocPKDrawingInstance() initWithData:drawingData.get() error:nil]);
+    [_pencilView setDrawing:drawing.get()];
 }
 
 - (void)drawingDidChange:(PKCanvasView *)canvasView
 {
+    [self invalidateAttachment];
+}
+
+- (void)invalidateAttachment
+{
     if (!_webPageProxy)
         return;
     auto& page = *_webPageProxy;