Title: [238566] trunk
Revision
238566
Author
timothy_hor...@apple.com
Date
2018-11-27 12:20:18 -0800 (Tue, 27 Nov 2018)

Log 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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (238565 => 238566)


--- trunk/LayoutTests/ChangeLog	2018-11-27 19:52:36 UTC (rev 238565)
+++ trunk/LayoutTests/ChangeLog	2018-11-27 20:20:18 UTC (rev 238566)
@@ -1,3 +1,16 @@
+2018-11-27  Tim Horton  <timothy_hor...@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_...@apple.com>
 
         Disallow loading webarchives as iframes

Added: trunk/LayoutTests/editing/images/paste-editable-image-expected.txt (0 => 238566)


--- trunk/LayoutTests/editing/images/paste-editable-image-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/images/paste-editable-image-expected.txt	2018-11-27 20:20:18 UTC (rev 238566)
@@ -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.
+
+

Added: trunk/LayoutTests/editing/images/paste-editable-image.html (0 => 238566)


--- trunk/LayoutTests/editing/images/paste-editable-image.html	                        (rev 0)
+++ trunk/LayoutTests/editing/images/paste-editable-image.html	2018-11-27 20:20:18 UTC (rev 238566)
@@ -0,0 +1,47 @@
+<!DOCTYPE html><!-- webkit-test-runner [ enableEditableImages=true enableAttachmentElement=true ] -->
+<head>
+<script src=""
+<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>

Modified: trunk/Source/WebCore/ChangeLog (238565 => 238566)


--- trunk/Source/WebCore/ChangeLog	2018-11-27 19:52:36 UTC (rev 238565)
+++ trunk/Source/WebCore/ChangeLog	2018-11-27 20:20:18 UTC (rev 238566)
@@ -1,3 +1,32 @@
+2018-11-27  Tim Horton  <timothy_hor...@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_...@apple.com>
 
         Disallow loading webarchives as iframes

Modified: trunk/Source/WebCore/html/HTMLImageElement.cpp (238565 => 238566)


--- trunk/Source/WebCore/html/HTMLImageElement.cpp	2018-11-27 19:52:36 UTC (rev 238565)
+++ trunk/Source/WebCore/html/HTMLImageElement.cpp	2018-11-27 20:20:18 UTC (rev 238566)
@@ -243,7 +243,7 @@
         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 @@
             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 @@
     return insertNotificationRequest;
 }
 
+void HTMLImageElement::didFinishInsertingNode()
+{
+    if (hasEditableImageAttribute())
+        updateEditableImage();
+}
+
 void HTMLImageElement::removedFromAncestor(RemovalType removalType, ContainerNode& oldParentOfRemovedTree)
 {
     if (m_form)
@@ -368,12 +374,19 @@
         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 @@
     return m_editableImage->embeddedViewID();
 }
 
-void HTMLImageElement::updateEditableImage(IsConnectedToDocument connected)
+void HTMLImageElement::updateEditableImage()
 {
     if (!document().settings().editableImagesEnabled())
         return;
@@ -390,9 +403,9 @@
     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

Modified: trunk/Source/WebCore/html/HTMLImageElement.h (238565 => 238566)


--- trunk/Source/WebCore/html/HTMLImageElement.h	2018-11-27 19:52:36 UTC (rev 238565)
+++ trunk/Source/WebCore/html/HTMLImageElement.h	2018-11-27 20:20:18 UTC (rev 238566)
@@ -116,6 +116,7 @@
 #endif
 
     GraphicsLayer::EmbeddedViewID editableImageViewID() const;
+    bool hasEditableImageAttribute() const;
 
 protected:
     HTMLImageElement(const QualifiedName&, Document&, HTMLFormElement* = 0);
@@ -142,6 +143,7 @@
     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 @@
 
     ImageCandidate bestFitSourceFromPictureElement();
 
-    enum class IsConnectedToDocument : bool { No, Yes };
-    void updateEditableImage(IsConnectedToDocument);
+    void updateEditableImage();
 
     void copyNonAttributePropertiesFromElement(const Element&) final;
 

Modified: trunk/Source/WebCore/rendering/RenderImage.cpp (238565 => 238566)


--- trunk/Source/WebCore/rendering/RenderImage.cpp	2018-11-27 19:52:36 UTC (rev 238565)
+++ trunk/Source/WebCore/rendering/RenderImage.cpp	2018-11-27 20:20:18 UTC (rev 238566)
@@ -214,13 +214,9 @@
 
 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

Modified: trunk/Source/WebKit/ChangeLog (238565 => 238566)


--- trunk/Source/WebKit/ChangeLog	2018-11-27 19:52:36 UTC (rev 238565)
+++ trunk/Source/WebKit/ChangeLog	2018-11-27 20:20:18 UTC (rev 238566)
@@ -1,3 +1,64 @@
+2018-11-27  Tim Horton  <timothy_hor...@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  <cdu...@apple.com>
 
         Regression(PSON) crash under WebPageProxy::didReceiveServerRedirectForProvisionalLoadForFrame()

Modified: trunk/Source/WebKit/Platform/spi/ios/PencilKitSPI.h (238565 => 238566)


--- trunk/Source/WebKit/Platform/spi/ios/PencilKitSPI.h	2018-11-27 19:52:36 UTC (rev 238565)
+++ trunk/Source/WebKit/Platform/spi/ios/PencilKitSPI.h	2018-11-27 20:20:18 UTC (rev 238566)
@@ -28,6 +28,7 @@
 #if USE(APPLE_INTERNAL_SDK)
 
 #import <PencilKit/PencilKit.h>
+#import <PencilKit/PKDrawing_Private.h>
 
 #else
 
@@ -37,6 +38,12 @@
 
 @end
 
+@interface PKDrawing : NSObject
+
+- (NSData *)serialize;
+
+@end
+
 #endif
 
 #endif // HAVE(PENCILKIT)

Modified: trunk/Source/WebKit/UIProcess/API/APIAttachment.cpp (238565 => 238566)


--- trunk/Source/WebKit/UIProcess/API/APIAttachment.cpp	2018-11-27 19:52:36 UTC (rev 238565)
+++ trunk/Source/WebKit/UIProcess/API/APIAttachment.cpp	2018-11-27 20:20:18 UTC (rev 238566)
@@ -55,6 +55,11 @@
         return;
     }
 
+    if (m_webPage->willUpdateAttachmentAttributes(*this) == WebKit::WebPageProxy::ShouldUpdateAttachmentAttributes::No) {
+        callback(WebKit::CallbackBase::Error::None);
+        return;
+    }
+
     m_webPage->updateAttachmentAttributes(*this, WTFMove(callback));
 }
 

Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (238565 => 238566)


--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2018-11-27 19:52:36 UTC (rev 238565)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2018-11-27 20:20:18 UTC (rev 238566)
@@ -8108,6 +8108,15 @@
     }
 }
 
+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&)

Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.h (238565 => 238566)


--- trunk/Source/WebKit/UIProcess/WebPageProxy.h	2018-11-27 19:52:36 UTC (rev 238565)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.h	2018-11-27 20:20:18 UTC (rev 238566)
@@ -1370,6 +1370,9 @@
     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)

Modified: trunk/Source/WebKit/UIProcess/ios/EditableImageController.h (238565 => 238566)


--- trunk/Source/WebKit/UIProcess/ios/EditableImageController.h	2018-11-27 19:52:36 UTC (rev 238565)
+++ trunk/Source/WebKit/UIProcess/ios/EditableImageController.h	2018-11-27 20:20:18 UTC (rev 238566)
@@ -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 @@
 
 namespace WebKit {
 
-class WebPageProxy;
-
 struct EditableImage {
     RetainPtr<WKDrawingView> drawingView;
     String attachmentID;
@@ -59,6 +59,8 @@
 
     void invalidateAttachmentForEditableImage(WebCore::GraphicsLayer::EmbeddedViewID);
 
+    WebPageProxy::ShouldUpdateAttachmentAttributes willUpdateAttachmentAttributes(const API::Attachment&);
+
 private:
     void didCreateEditableImage(WebCore::GraphicsLayer::EmbeddedViewID);
     void didDestroyEditableImage(WebCore::GraphicsLayer::EmbeddedViewID);
@@ -65,6 +67,8 @@
 
     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;

Modified: trunk/Source/WebKit/UIProcess/ios/EditableImageController.mm (238565 => 238566)


--- trunk/Source/WebKit/UIProcess/ios/EditableImageController.mm	2018-11-27 19:52:36 UTC (rev 238565)
+++ trunk/Source/WebKit/UIProcess/ios/EditableImageController.mm	2018-11-27 20:20:18 UTC (rev 238566)
@@ -92,10 +92,16 @@
     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 = "" PNGRepresentation];
+        if (!data)
+            return nil;
+        RetainPtr<NSFileWrapper> fileWrapper = adoptNS([[NSFileWrapper alloc] initRegularFileWithContents:data]);
         [fileWrapper setPreferredFilename:@"drawing.png"];
         return fileWrapper;
     });
@@ -102,6 +108,15 @@
     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 @@
     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)

Modified: trunk/Source/WebKit/UIProcess/ios/WKDrawingView.h (238565 => 238566)


--- trunk/Source/WebKit/UIProcess/ios/WKDrawingView.h	2018-11-27 19:52:36 UTC (rev 238565)
+++ trunk/Source/WebKit/UIProcess/ios/WKDrawingView.h	2018-11-27 20:20:18 UTC (rev 238566)
@@ -33,6 +33,7 @@
 - (instancetype)initWithEmbeddedViewID:(WebCore::GraphicsLayer::EmbeddedViewID)embeddedViewID webPageProxy:(WebKit::WebPageProxy&)webPageProxy;
 
 - (NSData *)PNGRepresentation;
+- (void)loadDrawingFromPNGRepresentation:(NSData *)PNGData;
 
 @end
 

Modified: trunk/Source/WebKit/UIProcess/ios/WKDrawingView.mm (238565 => 238566)


--- trunk/Source/WebKit/UIProcess/ios/WKDrawingView.mm	2018-11-27 19:52:36 UTC (rev 238565)
+++ trunk/Source/WebKit/UIProcess/ios/WKDrawingView.mm	2018-11-27 20:20:18 UTC (rev 238566)
@@ -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,11 +78,16 @@
         // 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));
 
@@ -88,10 +94,22 @@
     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
@@ -98,11 +116,46 @@
     // 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;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to