Title: [279444] trunk
Revision
279444
Author
[email protected]
Date
2021-06-30 16:54:40 -0700 (Wed, 30 Jun 2021)

Log Message

Add ID and versioning support for AppHighlights
https://bugs.webkit.org/show_bug.cgi?id=227279

Reviewed by Tim Horton.

Source/WebCore:

AppHighlights.AppHighlightRestoreFromStorage
AppHighlights.AppHighlightCreateAndRestoreAndDropBytes
AppHighlights.AppHighlightCreateAndRestoreWithLaterVersion
AppHighlights.AppHighlightCreateAndRestoreWithExtraBytes
AppHighlights.AppHighlightRestoreFromStorageV0
AppHighlights.AppHighlightRestoreFromStorageV1

Reformat the storage of Highlight Data to allow for accurate deletion of active
highlights, as well as making them more robust and future-proof. Support decoding v0
highlights as well.

* Modules/highlight/AppHighlightRangeData.cpp:
(WebCore::AppHighlightRangeData::NodePathComponent::decode):
(WebCore::AppHighlightRangeData::encode const):
(WebCore::AppHighlightRangeData::decode):
* Modules/highlight/AppHighlightRangeData.h:
(WebCore::AppHighlightRangeData::NodePathComponent::NodePathComponent):
(WebCore::AppHighlightRangeData::AppHighlightRangeData):
(WebCore::AppHighlightRangeData::identifier const):
(WebCore::AppHighlightRangeData::startOffset const):
(WebCore::AppHighlightRangeData::endOffset const):
* Modules/highlight/AppHighlightStorage.cpp:
(WebCore::createAppHighlightRangeData):

Source/WebKit:

Reformat the storage of Highlight Data to allow for accurate deletion of active
highlights, as well as making them more robust and future-proof.

Also found an issue with creating SharedBuffers from the memory map, in that the ipcHandle size
should be used instead of the sharedMemory->size().

* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::restoreAppHighlightsAndScrollToIndex):

Source/WTF:

Allow PersistentDecoders to rewind, to help support v0 highlight data.

* wtf/persistence/PersistentDecoder.cpp:
(WTF::Persistence::Decoder::Decoder):
(WTF::Persistence::Decoder::rewind):
* wtf/persistence/PersistentDecoder.h:

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/WKAppHighlights.mm:
(TestWebKitAPI::createAppHighlightWithHTML):
(TestWebKitAPI::createWebViewForAppHighlightsWithHTML):
(TestWebKitAPI::TEST):

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (279443 => 279444)


--- trunk/Source/WTF/ChangeLog	2021-06-30 23:53:16 UTC (rev 279443)
+++ trunk/Source/WTF/ChangeLog	2021-06-30 23:54:40 UTC (rev 279444)
@@ -1,3 +1,17 @@
+2021-06-30  Megan Gardner  <[email protected]>
+
+        Add ID and versioning support for AppHighlights
+        https://bugs.webkit.org/show_bug.cgi?id=227279
+
+        Reviewed by Tim Horton.
+
+        Allow PersistentDecoders to rewind, to help support v0 highlight data.
+
+        * wtf/persistence/PersistentDecoder.cpp:
+        (WTF::Persistence::Decoder::Decoder):
+        (WTF::Persistence::Decoder::rewind):
+        * wtf/persistence/PersistentDecoder.h:
+
 2021-06-30  Ryosuke Niwa  <[email protected]>
 
         Use WeakHashMap and WeakPtr with Node in more places

Modified: trunk/Source/WTF/wtf/persistence/PersistentDecoder.cpp (279443 => 279444)


--- trunk/Source/WTF/wtf/persistence/PersistentDecoder.cpp	2021-06-30 23:53:16 UTC (rev 279443)
+++ trunk/Source/WTF/wtf/persistence/PersistentDecoder.cpp	2021-06-30 23:54:40 UTC (rev 279444)
@@ -68,6 +68,15 @@
     return true;
 }
 
+bool Decoder::rewind(size_t size)
+{
+    if (m_bufferPosition - size >= m_buffer) {
+        m_bufferPosition -= size;
+        return true;
+    }
+    return false;
+}
+
 template<typename T>
 Decoder& Decoder::decodeNumber(std::optional<T>& optional)
 {

Modified: trunk/Source/WTF/wtf/persistence/PersistentDecoder.h (279443 => 279444)


--- trunk/Source/WTF/wtf/persistence/PersistentDecoder.h	2021-06-30 23:53:16 UTC (rev 279443)
+++ trunk/Source/WTF/wtf/persistence/PersistentDecoder.h	2021-06-30 23:54:40 UTC (rev 279444)
@@ -40,6 +40,8 @@
 
     size_t length() const { return m_bufferEnd - m_buffer; }
     size_t currentOffset() const { return m_bufferPosition - m_buffer; }
+    
+    WTF_EXPORT_PRIVATE WARN_UNUSED_RETURN bool rewind(size_t);
 
     WTF_EXPORT_PRIVATE WARN_UNUSED_RETURN bool verifyChecksum();
 

Modified: trunk/Source/WebCore/ChangeLog (279443 => 279444)


--- trunk/Source/WebCore/ChangeLog	2021-06-30 23:53:16 UTC (rev 279443)
+++ trunk/Source/WebCore/ChangeLog	2021-06-30 23:54:40 UTC (rev 279444)
@@ -1,3 +1,34 @@
+2021-06-30  Megan Gardner  <[email protected]>
+
+        Add ID and versioning support for AppHighlights
+        https://bugs.webkit.org/show_bug.cgi?id=227279
+
+        Reviewed by Tim Horton.
+
+        AppHighlights.AppHighlightRestoreFromStorage
+        AppHighlights.AppHighlightCreateAndRestoreAndDropBytes
+        AppHighlights.AppHighlightCreateAndRestoreWithLaterVersion
+        AppHighlights.AppHighlightCreateAndRestoreWithExtraBytes
+        AppHighlights.AppHighlightRestoreFromStorageV0
+        AppHighlights.AppHighlightRestoreFromStorageV1
+
+        Reformat the storage of Highlight Data to allow for accurate deletion of active
+        highlights, as well as making them more robust and future-proof. Support decoding v0 
+        highlights as well.
+
+        * Modules/highlight/AppHighlightRangeData.cpp:
+        (WebCore::AppHighlightRangeData::NodePathComponent::decode):
+        (WebCore::AppHighlightRangeData::encode const):
+        (WebCore::AppHighlightRangeData::decode):
+        * Modules/highlight/AppHighlightRangeData.h:
+        (WebCore::AppHighlightRangeData::NodePathComponent::NodePathComponent):
+        (WebCore::AppHighlightRangeData::AppHighlightRangeData):
+        (WebCore::AppHighlightRangeData::identifier const):
+        (WebCore::AppHighlightRangeData::startOffset const):
+        (WebCore::AppHighlightRangeData::endOffset const):
+        * Modules/highlight/AppHighlightStorage.cpp:
+        (WebCore::createAppHighlightRangeData):
+
 2021-06-30  Ryosuke Niwa  <[email protected]>
 
         RemotePlayback must keep its media element alive when there is a pending activity

Modified: trunk/Source/WebCore/Modules/highlight/AppHighlightRangeData.cpp (279443 => 279444)


--- trunk/Source/WebCore/Modules/highlight/AppHighlightRangeData.cpp	2021-06-30 23:53:16 UTC (rev 279443)
+++ trunk/Source/WebCore/Modules/highlight/AppHighlightRangeData.cpp	2021-06-30 23:54:40 UTC (rev 279444)
@@ -41,6 +41,8 @@
 
 namespace WebCore {
 
+constexpr uint64_t highlightFileSignature = 0x4141504832303231; // File Signature  (A)pple(AP)plication(H)ighlights(2021)
+
 std::optional<AppHighlightRangeData> AppHighlightRangeData::create(const SharedBuffer& buffer)
 {
     auto decoder = buffer.decoder();
@@ -81,7 +83,7 @@
     if (!textData)
         return std::nullopt;
 
-    std::optional<unsigned> pathIndex;
+    std::optional<uint32_t> pathIndex;
     decoder >> pathIndex;
     if (!pathIndex)
         return std::nullopt;
@@ -91,6 +93,12 @@
 
 template<class Encoder> void AppHighlightRangeData::encode(Encoder& encoder) const
 {
+    static_assert(!Encoder::isIPCEncoder, "AppHighlightRangeData should not be used by IPC::Encoder");
+    constexpr uint64_t currentAppHighlightVersion = 1;
+    
+    encoder << highlightFileSignature;
+    encoder << currentAppHighlightVersion;
+    encoder << m_identifier;
     encoder << m_text;
     encoder << m_startContainer;
     encoder << m_startOffset;
@@ -100,6 +108,33 @@
 
 template<class Decoder> std::optional<AppHighlightRangeData> AppHighlightRangeData::decode(Decoder& decoder)
 {
+    static_assert(!Decoder::isIPCDecoder, "AppHighlightRangeData should not be used by IPC::Decoder");
+    
+    std::optional<uint64_t> version;
+    
+    std::optional<uint64_t> decodedHighlightFileSignature;
+    decoder >> decodedHighlightFileSignature;
+    if (!decodedHighlightFileSignature)
+        return std::nullopt;
+    if (decodedHighlightFileSignature != highlightFileSignature) {
+        if (!decoder.rewind(sizeof(highlightFileSignature)))
+            return std::nullopt;
+        version = 0;
+    }
+    
+    std::optional<String> identifier;
+    if (version)
+        identifier = nullString();
+    else {
+        decoder >> version;
+        if (!version)
+            return std::nullopt;
+        
+        decoder >> identifier;
+        if (!identifier)
+            return std::nullopt;
+    }
+
     std::optional<String> text;
     decoder >> text;
     if (!text)
@@ -110,7 +145,7 @@
     if (!startContainer)
         return std::nullopt;
 
-    std::optional<unsigned> startOffset;
+    std::optional<uint32_t> startOffset;
     decoder >> startOffset;
     if (!startOffset)
         return std::nullopt;
@@ -120,13 +155,12 @@
     if (!endContainer)
         return std::nullopt;
 
-    std::optional<unsigned> endOffset;
+    std::optional<uint32_t> endOffset;
     decoder >> endOffset;
     if (!endOffset)
         return std::nullopt;
 
-
-    return {{ WTFMove(*text), WTFMove(*startContainer), *startOffset, WTFMove(*endContainer), *endOffset }};
+    return {{ WTFMove(*identifier), WTFMove(*text), WTFMove(*startContainer), *startOffset, WTFMove(*endContainer), *endOffset }};
 }
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/Modules/highlight/AppHighlightRangeData.h (279443 => 279444)


--- trunk/Source/WebCore/Modules/highlight/AppHighlightRangeData.h	2021-06-30 23:53:16 UTC (rev 279443)
+++ trunk/Source/WebCore/Modules/highlight/AppHighlightRangeData.h	2021-06-30 23:54:40 UTC (rev 279444)
@@ -43,9 +43,9 @@
         String identifier;
         String nodeName;
         String textData;
-        unsigned pathIndex { 0 };
+        uint32_t pathIndex { 0 };
 
-        NodePathComponent(String&& elementIdentifier, String&& name, String&& data, unsigned index)
+        NodePathComponent(String&& elementIdentifier, String&& name, String&& data, uint32_t index)
             : identifier(WTFMove(elementIdentifier))
             , nodeName(WTFMove(name))
             , textData(WTFMove(data))
@@ -53,7 +53,7 @@
         {
         }
 
-        NodePathComponent(const String& elementIdentifier, const String& name, const String& data, unsigned index)
+        NodePathComponent(const String& elementIdentifier, const String& name, const String& data, uint32_t index)
             : identifier(elementIdentifier)
             , nodeName(name)
             , textData(data)
@@ -79,8 +79,9 @@
 
     AppHighlightRangeData(const AppHighlightRangeData&) = default;
     AppHighlightRangeData() = default;
-    AppHighlightRangeData(String&& text, NodePath&& startContainer, unsigned startOffset, NodePath&& endContainer, unsigned endOffset)
-        : m_text(WTFMove(text))
+    AppHighlightRangeData(String&& identifier, String&& text, NodePath&& startContainer, uint64_t startOffset, NodePath&& endContainer, uint64_t endOffset)
+        : m_identifier(WTFMove(identifier))
+        , m_text(WTFMove(text))
         , m_startContainer(WTFMove(startContainer))
         , m_startOffset(startOffset)
         , m_endContainer(WTFMove(endContainer))
@@ -88,8 +89,9 @@
     {
     }
 
-    AppHighlightRangeData(const String& text, const NodePath& startContainer, unsigned startOffset, const NodePath& endContainer, unsigned endOffset)
-        : m_text(text)
+    AppHighlightRangeData(const String& identifier, const String& text, const NodePath& startContainer, uint64_t startOffset, const NodePath& endContainer, uint64_t endOffset)
+        : m_identifier(identifier)
+        , m_text(text)
         , m_startContainer(startContainer)
         , m_startOffset(startOffset)
         , m_endContainer(endContainer)
@@ -97,11 +99,12 @@
     {
     }
 
+    const String& identifier() const { return m_identifier; }
     const String& text() const { return m_text; }
     const NodePath& startContainer() const { return m_startContainer; }
-    unsigned startOffset() const { return m_startOffset; }
+    uint32_t startOffset() const { return m_startOffset; }
     const NodePath& endContainer() const { return m_endContainer; }
-    unsigned endOffset() const { return m_endOffset; }
+    uint32_t endOffset() const { return m_endOffset; }
 
     template<class Encoder> void encode(Encoder&) const;
     template<class Decoder> static std::optional<AppHighlightRangeData> decode(Decoder&);
@@ -109,11 +112,12 @@
     Ref<SharedBuffer> toSharedBuffer() const;
 
 private:
+    String m_identifier;
     String m_text;
     NodePath m_startContainer;
-    unsigned m_startOffset { 0 };
+    uint32_t m_startOffset { 0 };
     NodePath m_endContainer;
-    unsigned m_endOffset { 0 };
+    uint32_t m_endOffset { 0 };
 };
 
 #endif

Modified: trunk/Source/WebCore/Modules/highlight/AppHighlightStorage.cpp (279443 => 279444)


--- trunk/Source/WebCore/Modules/highlight/AppHighlightStorage.cpp	2021-06-30 23:53:16 UTC (rev 279443)
+++ trunk/Source/WebCore/Modules/highlight/AppHighlightStorage.cpp	2021-06-30 23:54:40 UTC (rev 279444)
@@ -43,6 +43,7 @@
 #include "StaticRange.h"
 #include "TextIndicator.h"
 #include "TextIterator.h"
+#include <wtf/UUID.h>
 
 namespace WebCore {
 
@@ -203,7 +204,10 @@
 {
     auto text = plainText(range);
     text.truncate(textPreviewLength);
+    auto identifier = createCanonicalUUIDString();
+
     return {
+        identifier,
         text,
         makeNodePath(&range.startContainer()),
         range.startOffset(),

Modified: trunk/Source/WebKit/ChangeLog (279443 => 279444)


--- trunk/Source/WebKit/ChangeLog	2021-06-30 23:53:16 UTC (rev 279443)
+++ trunk/Source/WebKit/ChangeLog	2021-06-30 23:54:40 UTC (rev 279444)
@@ -1,3 +1,19 @@
+2021-06-30  Megan Gardner  <[email protected]>
+
+        Add ID and versioning support for AppHighlights
+        https://bugs.webkit.org/show_bug.cgi?id=227279
+
+        Reviewed by Tim Horton.
+
+        Reformat the storage of Highlight Data to allow for accurate deletion of active
+        highlights, as well as making them more robust and future-proof.
+
+        Also found an issue with creating SharedBuffers from the memory map, in that the ipcHandle size
+        should be used instead of the sharedMemory->size(). 
+
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::restoreAppHighlightsAndScrollToIndex):
+
 2021-06-30  Truitt Savell  <[email protected]>
 
         Unreviewed, reverting r279405.

Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (279443 => 279444)


--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp	2021-06-30 23:53:16 UTC (rev 279443)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp	2021-06-30 23:54:40 UTC (rev 279444)
@@ -7617,7 +7617,7 @@
         if (!sharedMemory)
             continue;
 
-        document->appHighlightStorage().restoreAndScrollToAppHighlight(SharedBuffer::create(static_cast<const uint8_t*>(sharedMemory->data()), sharedMemory->size()), i == index ? ScrollToHighlight::Yes : ScrollToHighlight::No);
+        document->appHighlightStorage().restoreAndScrollToAppHighlight(SharedBuffer::create(static_cast<const uint8_t*>(sharedMemory->data()), ipcHandle.dataSize), i == index ? ScrollToHighlight::Yes : ScrollToHighlight::No);
         i++;
     }
 }

Modified: trunk/Tools/ChangeLog (279443 => 279444)


--- trunk/Tools/ChangeLog	2021-06-30 23:53:16 UTC (rev 279443)
+++ trunk/Tools/ChangeLog	2021-06-30 23:54:40 UTC (rev 279444)
@@ -1,3 +1,15 @@
+2021-06-30  Megan Gardner  <[email protected]>
+
+        Add ID and versioning support for AppHighlights
+        https://bugs.webkit.org/show_bug.cgi?id=227279
+
+        Reviewed by Tim Horton.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/WKAppHighlights.mm:
+        (TestWebKitAPI::createAppHighlightWithHTML):
+        (TestWebKitAPI::createWebViewForAppHighlightsWithHTML):
+        (TestWebKitAPI::TEST):
+
 2021-06-30  Wenson Hsieh  <[email protected]>
 
         [iOS] [Live Text] "Text from Camera" should not be shown in callout bar when selecting text

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAppHighlights.mm (279443 => 279444)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAppHighlights.mm	2021-06-30 23:53:16 UTC (rev 279443)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAppHighlights.mm	2021-06-30 23:54:40 UTC (rev 279444)
@@ -56,94 +56,165 @@
 
 namespace TestWebKitAPI {
 
-TEST(AppHighlights, AppHighlightCreateAndRestore)
+RetainPtr<_WKAppHighlight> createAppHighlightWithHTML(NSString *HTMLString, NSString *_javascript_, NSString *highlightText)
 {
     WKWebViewConfiguration *configuration = [WKWebViewConfiguration _test_configurationWithTestPlugInClassName:@"WebProcessPlugInWithInternals" configureJSCForTesting:YES];
     auto delegate = adoptNS([[AppHighlightDelegate alloc] init]);
     auto webViewCreate = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500) configuration:configuration]);
-    auto webViewRestore = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500) configuration:configuration]);
     [webViewCreate _setAppHighlightDelegate:delegate.get()];
-    [webViewCreate synchronouslyLoadHTMLString:@"Test"];
-    [webViewCreate stringByEvaluatingJavaScript:@"document.execCommand('SelectAll')"];
-    __block bool finished = NO;
+    [webViewCreate synchronouslyLoadHTMLString:HTMLString];
+    [webViewCreate stringByEvaluatingJavaScript:_javascript_];
+    __block RetainPtr<_WKAppHighlight> resultHighlight;
+    __block bool done = false;
     [delegate setStoreAppHighlightCallback:^(WKWebView *delegateWebView, _WKAppHighlight *highlight, BOOL inNewGroup, BOOL requestOriginatedInApp) {
         EXPECT_EQ(delegateWebView, webViewCreate.get());
         EXPECT_NOT_NULL(highlight);
-        EXPECT_WK_STREQ(highlight.text, @"Test");
+        EXPECT_WK_STREQ(highlight.text, highlightText);
         EXPECT_NOT_NULL(highlight.highlight);
-        
-        [webViewRestore synchronouslyLoadHTMLString:@"Test"];
-        [webViewRestore _restoreAppHighlights:@[ highlight.highlight ]];
-        
-        TestWebKitAPI::Util::waitForConditionWithLogging([&] () -> bool {
-            return [webViewRestore stringByEvaluatingJavaScript:@"internals.numberOfAppHighlights()"].intValue == 1;
-        }, 2, @"Expected Highlights to be populated.");
-        
-        finished = YES;
+        resultHighlight = highlight;
+        done = true;
     }];
     [webViewCreate _addAppHighlight];
-    TestWebKitAPI::Util::run(&finished);
+    TestWebKitAPI::Util::run(&done);
+    return resultHighlight;
 }
 
-TEST(AppHighlights, AppHighlightCreateAndRestoreAndScroll)
+RetainPtr<WKWebView> createWebViewForAppHighlightsWithHTML(NSString *HTMLString)
 {
     WKWebViewConfiguration *configuration = [WKWebViewConfiguration _test_configurationWithTestPlugInClassName:@"WebProcessPlugInWithInternals" configureJSCForTesting:YES];
-    auto delegate = adoptNS([[AppHighlightDelegate alloc] init]);
-    auto webViewCreate = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500) configuration:configuration]);
     auto webViewRestore = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500) configuration:configuration]);
-    [webViewCreate _setAppHighlightDelegate:delegate.get()];
-    [webViewCreate synchronouslyLoadHTMLString:@"<div style='height: 1000px'></div>Test"];
-    [webViewCreate stringByEvaluatingJavaScript:@"document.execCommand('SelectAll')"];
-    __block bool finished = NO;
-    [delegate setStoreAppHighlightCallback:^(WKWebView *delegateWebView, _WKAppHighlight *highlight, BOOL inNewGroup, BOOL requestOriginatedInApp) {
-        EXPECT_EQ(delegateWebView, webViewCreate.get());
-        EXPECT_NOT_NULL(highlight);
-        EXPECT_WK_STREQ(highlight.text, @"Test");
-        EXPECT_NOT_NULL(highlight.highlight);
+    [webViewRestore synchronouslyLoadHTMLString:HTMLString];
+    
+    return webViewRestore;
+}
+
+TEST(AppHighlights, AppHighlightCreateAndRestore)
+{
+    auto highlight = createAppHighlightWithHTML(@"Test", @"document.execCommand('SelectAll')", @"Test");
+    auto webViewRestore = createWebViewForAppHighlightsWithHTML(@"Test");
+    
+    [webViewRestore _restoreAppHighlights:@[[highlight highlight]]];
+    
+    TestWebKitAPI::Util::waitForConditionWithLogging([&] () -> bool {
+        return [webViewRestore stringByEvaluatingJavaScript:@"internals.numberOfAppHighlights()"].intValue == 1;
+    }, 2, @"Expected Highlights to be populated.");
+}
+
+TEST(AppHighlights, AppHighlightCreateAndRestoreAndScroll)
+{
+    auto highlight = createAppHighlightWithHTML(@"<div style='height: 10000px'></div>Test", @"document.execCommand('SelectAll')", @"Test");
+    auto webViewRestore = createWebViewForAppHighlightsWithHTML(@"<div style='height: 10000px'></div>Test");
+
+    [webViewRestore _restoreAndScrollToAppHighlight:[highlight highlight]];
+
+    TestWebKitAPI::Util::waitForConditionWithLogging([&] () -> bool {
+        return [webViewRestore stringByEvaluatingJavaScript:@"internals.numberOfAppHighlights()"].intValue == 1;
+    }, 2, @"Expected Highlights to be populated.");
+    EXPECT_NE(0, [[webViewRestore objectByEvaluatingJavaScript:@"pageYOffset"] floatValue]);
+}
+
+TEST(AppHighlights, AppHighlightRestoreFailure)
+{
+    auto highlight = createAppHighlightWithHTML(@"Test", @"document.execCommand('SelectAll')", @"Test");
+    auto webViewRestore = createWebViewForAppHighlightsWithHTML(@"Not The Same");
+    
+    [webViewRestore _restoreAppHighlights:@[[highlight highlight]]];
+    
+    TestWebKitAPI::Util::waitForConditionWithLogging([&] () -> bool {
+        return ![webViewRestore stringByEvaluatingJavaScript:@"internals.numberOfAppHighlights()"].intValue;
+    }, 2, @"Expected Highlights not to be populated.");
+}
+
+// Ensure that future versions of the blob format can add additional data and still be decoded successfully by version of WebKit that only know about the current format.
+TEST(AppHighlights, AppHighlightCreateAndRestoreWithExtraBytes)
+{
+    auto highlight = createAppHighlightWithHTML(@"Test", @"document.execCommand('SelectAll')", @"Test");
+    auto webViewRestore = createWebViewForAppHighlightsWithHTML(@"Test");
         
-        [webViewRestore synchronouslyLoadHTMLString:@"<div style='height: 1000px'></div>Test"];
-        [webViewRestore _restoreAndScrollToAppHighlight:highlight.highlight];
+    NSMutableData *modifiedHighlightData = [NSMutableData dataWithData: [highlight highlight] ];
+    [modifiedHighlightData appendData:[@"TestV2Data" dataUsingEncoding:NSUTF8StringEncoding]];
+    
+    [webViewRestore synchronouslyLoadHTMLString:@"Test"];
+    [webViewRestore _restoreAppHighlights:@[ modifiedHighlightData ]];
+    
+    TestWebKitAPI::Util::waitForConditionWithLogging([&] () -> bool {
+        return [webViewRestore stringByEvaluatingJavaScript:@"internals.numberOfAppHighlights()"].intValue == 1;
+    }, 2, @"Expected Highlights to be populated.");
+}
+
+// Older versions of WebKit need to be able to decode blobs encoded on newer versions of WebKit, so ensure that is possible.
+TEST(AppHighlights, AppHighlightCreateAndRestoreWithLaterVersion)
+{
+    auto highlight = createAppHighlightWithHTML(@"Test", @"document.execCommand('SelectAll')", @"Test");
+    auto webViewRestore = createWebViewForAppHighlightsWithHTML(@"Test");
         
-        TestWebKitAPI::Util::waitForConditionWithLogging([&] () -> bool {
-            return [webViewRestore stringByEvaluatingJavaScript:@"internals.numberOfAppHighlights()"].intValue == 1;
-        }, 2, @"Expected Highlights to be populated.");
-        EXPECT_NE(0, [[webViewRestore objectByEvaluatingJavaScript:@"pageYOffset"] floatValue]);
+    uint64_t maximumVersion = std::numeric_limits<uint64_t>::max();
+    NSData *versionData = [NSData dataWithBytes:&maximumVersion length:sizeof(maximumVersion)];
+    NSMutableData *modifiedHighlightData = [NSMutableData dataWithData:[highlight highlight]];
+
+    [modifiedHighlightData replaceBytesInRange:NSMakeRange(sizeof(uint64_t), sizeof(maximumVersion)) withBytes:versionData];
+
+    [webViewRestore synchronouslyLoadHTMLString:@"Test"];
+    [webViewRestore _restoreAppHighlights:@[ modifiedHighlightData ]];
+    
+    TestWebKitAPI::Util::waitForConditionWithLogging([&] () -> bool {
+        return [webViewRestore stringByEvaluatingJavaScript:@"internals.numberOfAppHighlights()"].intValue == 1;
+    }, 2, @"Expected Highlights to be populated.");
+}
+
+// Ensure that shorter data blobs will correctly fail to decode.
+TEST(AppHighlights, AppHighlightCreateAndRestoreAndDropBytes)
+{
+    auto highlight = createAppHighlightWithHTML(@"Test", @"document.execCommand('SelectAll')", @"Test");
+    auto webViewRestore = createWebViewForAppHighlightsWithHTML(@"Test");
         
-        finished = YES;
-    }];
-    [webViewCreate _addAppHighlight];
-    TestWebKitAPI::Util::run(&finished);
+    NSMutableData *modifiedHighlightData = [NSMutableData dataWithBytes:[highlight highlight].bytes length:[highlight highlight].length / 2];
+
+    [webViewRestore synchronouslyLoadHTMLString:@"Test"];
+    [webViewRestore _restoreAppHighlights:@[ modifiedHighlightData ]];
+    
+    TestWebKitAPI::Util::waitForConditionWithLogging([&] () -> bool {
+        return ![webViewRestore stringByEvaluatingJavaScript:@"internals.numberOfAppHighlights()"].intValue;
+    }, 2, @"Expected Highlights not to be populated.");
 }
 
-TEST(AppHighlights, AppHighlightRestoreFailure)
+// Ensure that the original data format will always be able to be decoded in the future.
+TEST(AppHighlights, AppHighlightRestoreFromStorageV0)
 {
     WKWebViewConfiguration *configuration = [WKWebViewConfiguration _test_configurationWithTestPlugInClassName:@"WebProcessPlugInWithInternals" configureJSCForTesting:YES];
-    auto delegate = adoptNS([[AppHighlightDelegate alloc] init]);
-    auto webViewCreate = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500) configuration:configuration]);
+    const unsigned char bytes[] = {0x04, 0x00, 0x00, 0x00, 0x01, 0x54, 0x65, 0x73, 0x74, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0x05, 0x00, 0x00, 0x00, 0x01, 0x23, 0x74, 0x65, 0x78, 0x74, 0x04, 0x00, 0x00, 0x00, 0x01, 0x54, 0x65, 0x73, 0x74, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0x05, 0x00, 0x00, 0x00, 0x01, 0x23, 0x74, 0x65, 0x78, 0x74, 0x04, 0x00, 0x00, 0x00, 0x01, 0x54, 0x65, 0x73, 0x74, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00};
+    
+    NSData *storedData = [NSData dataWithBytes:&bytes length:87];
+
     auto webViewRestore = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500) configuration:configuration]);
-    [webViewCreate _setAppHighlightDelegate:delegate.get()];
-    [webViewCreate synchronouslyLoadHTMLString:@"Test"];
-    [webViewCreate stringByEvaluatingJavaScript:@"document.execCommand('SelectAll')"];
-    __block bool finished = NO;
-    [delegate setStoreAppHighlightCallback:^(WKWebView *delegateWebView, _WKAppHighlight *highlight, BOOL inNewGroup, BOOL requestOriginatedInApp) {
-        EXPECT_EQ(delegateWebView, webViewCreate.get());
-        EXPECT_NOT_NULL(highlight);
-        EXPECT_WK_STREQ(highlight.text, @"Test");
-        EXPECT_NOT_NULL(highlight.highlight);
-        
-        [webViewRestore synchronouslyLoadHTMLString:@"Not the same"];
-        [webViewRestore _restoreAppHighlights:@[ highlight.highlight ]];
-        
-        TestWebKitAPI::Util::waitForConditionWithLogging([&] () -> bool {
-            return ![webViewRestore stringByEvaluatingJavaScript:@"internals.numberOfAppHighlights()"].intValue;
-        }, 2, @"Expected Highlights to be populated.");
-        
-        finished = YES;
-    }];
-    [webViewCreate _addAppHighlight];
-    TestWebKitAPI::Util::run(&finished);
+
+    [webViewRestore synchronouslyLoadHTMLString:@"Test"];
+    [webViewRestore _restoreAppHighlights:@[ storedData ]];
+    
+    TestWebKitAPI::Util::waitForConditionWithLogging([&] () -> bool {
+        return [webViewRestore stringByEvaluatingJavaScript:@"internals.numberOfAppHighlights()"].intValue == 1;
+    }, 2, @"Expected Highlights to be populated.");
 }
 
+
+// Ensure that the V1 data format will always be able to be decoded in the future.
+TEST(AppHighlights, AppHighlightRestoreFromStorageV1)
+{
+    WKWebViewConfiguration *configuration = [WKWebViewConfiguration _test_configurationWithTestPlugInClassName:@"WebProcessPlugInWithInternals" configureJSCForTesting:YES];
+    const unsigned char bytes[] = {0x31, 0x32, 0x30, 0x32, 0x48, 0x50, 0x41, 0x41, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, 0x01, 0x31, 0x36, 0x30, 0x61, 0x36, 0x30, 0x31, 0x62, 0x2d, 0x31, 0x62, 0x37, 0x32, 0x2d, 0x34, 0x31, 0x31, 0x38, 0x2d, 0x62, 0x36, 0x64, 0x31, 0x2d, 0x39, 0x30, 0x32, 0x62, 0x37, 0x34, 0x66, 0x65, 0x61, 0x36, 0x36, 0x31, 0x04, 0x00, 0x00, 0x00, 0x01, 0x54, 0x65, 0x73, 0x74, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0x05, 0x00, 0x00, 0x00, 0x01, 0x23, 0x74, 0x65, 0x78, 0x74, 0x04, 0x00, 0x00, 0x00, 0x01, 0x54, 0x65, 0x73, 0x74, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0x05, 0x00, 0x00, 0x00, 0x01, 0x23, 0x74, 0x65, 0x78, 0x74, 0x04, 0x00, 0x00, 0x00, 0x01, 0x54, 0x65, 0x73, 0x74, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00};
+    
+    NSData *storedData = [NSData dataWithBytes:&bytes length:144];
+
+    auto webViewRestore = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500) configuration:configuration]);
+
+    [webViewRestore synchronouslyLoadHTMLString:@"Test"];
+    [webViewRestore _restoreAppHighlights:@[ storedData ]];
+    
+    TestWebKitAPI::Util::waitForConditionWithLogging([&] () -> bool {
+        return [webViewRestore stringByEvaluatingJavaScript:@"internals.numberOfAppHighlights()"].intValue == 1;
+    }, 2, @"Expected Highlights to be populated.");
+}
+
 #if PLATFORM(IOS_FAMILY)
 
 TEST(AppHighlights, AvoidForcingCalloutBarInitialization)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to