<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[279444] trunk</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/279444">279444</a></dd>
<dt>Author</dt> <dd>megan_gardner@apple.com</dd>
<dt>Date</dt> <dd>2021-06-30 16:54:40 -0700 (Wed, 30 Jun 2021)</dd>
</dl>

<h3>Log Message</h3>
<pre>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):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWTFChangeLog">trunk/Source/WTF/ChangeLog</a></li>
<li><a href="#trunkSourceWTFwtfpersistencePersistentDecodercpp">trunk/Source/WTF/wtf/persistence/PersistentDecoder.cpp</a></li>
<li><a href="#trunkSourceWTFwtfpersistencePersistentDecoderh">trunk/Source/WTF/wtf/persistence/PersistentDecoder.h</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreModuleshighlightAppHighlightRangeDatacpp">trunk/Source/WebCore/Modules/highlight/AppHighlightRangeData.cpp</a></li>
<li><a href="#trunkSourceWebCoreModuleshighlightAppHighlightRangeDatah">trunk/Source/WebCore/Modules/highlight/AppHighlightRangeData.h</a></li>
<li><a href="#trunkSourceWebCoreModuleshighlightAppHighlightStoragecpp">trunk/Source/WebCore/Modules/highlight/AppHighlightStorage.cpp</a></li>
<li><a href="#trunkSourceWebKitChangeLog">trunk/Source/WebKit/ChangeLog</a></li>
<li><a href="#trunkSourceWebKitWebProcessWebPageWebPagecpp">trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp</a></li>
<li><a href="#trunkToolsChangeLog">trunk/Tools/ChangeLog</a></li>
<li><a href="#trunkToolsTestWebKitAPITestsWebKitCocoaWKAppHighlightsmm">trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAppHighlights.mm</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWTFChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WTF/ChangeLog (279443 => 279444)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,17 @@
</span><ins>+2021-06-30  Megan Gardner  <megan_gardner@apple.com>
+
+        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:
+
</ins><span class="cx"> 2021-06-30  Ryosuke Niwa  <rniwa@webkit.org>
</span><span class="cx"> 
</span><span class="cx">         Use WeakHashMap and WeakPtr with Node in more places
</span></span></pre></div>
<a id="trunkSourceWTFwtfpersistencePersistentDecodercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WTF/wtf/persistence/PersistentDecoder.cpp (279443 => 279444)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -68,6 +68,15 @@
</span><span class="cx">     return true;
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+bool Decoder::rewind(size_t size)
+{
+    if (m_bufferPosition - size >= m_buffer) {
+        m_bufferPosition -= size;
+        return true;
+    }
+    return false;
+}
+
</ins><span class="cx"> template<typename T>
</span><span class="cx"> Decoder& Decoder::decodeNumber(std::optional<T>& optional)
</span><span class="cx"> {
</span></span></pre></div>
<a id="trunkSourceWTFwtfpersistencePersistentDecoderh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WTF/wtf/persistence/PersistentDecoder.h (279443 => 279444)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -40,6 +40,8 @@
</span><span class="cx"> 
</span><span class="cx">     size_t length() const { return m_bufferEnd - m_buffer; }
</span><span class="cx">     size_t currentOffset() const { return m_bufferPosition - m_buffer; }
</span><ins>+    
+    WTF_EXPORT_PRIVATE WARN_UNUSED_RETURN bool rewind(size_t);
</ins><span class="cx"> 
</span><span class="cx">     WTF_EXPORT_PRIVATE WARN_UNUSED_RETURN bool verifyChecksum();
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (279443 => 279444)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,34 @@
</span><ins>+2021-06-30  Megan Gardner  <megan_gardner@apple.com>
+
+        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):
+
</ins><span class="cx"> 2021-06-30  Ryosuke Niwa  <rniwa@webkit.org>
</span><span class="cx"> 
</span><span class="cx">         RemotePlayback must keep its media element alive when there is a pending activity
</span></span></pre></div>
<a id="trunkSourceWebCoreModuleshighlightAppHighlightRangeDatacpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/highlight/AppHighlightRangeData.cpp (279443 => 279444)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -41,6 +41,8 @@
</span><span class="cx"> 
</span><span class="cx"> namespace WebCore {
</span><span class="cx"> 
</span><ins>+constexpr uint64_t highlightFileSignature = 0x4141504832303231; // File Signature  (A)pple(AP)plication(H)ighlights(2021)
+
</ins><span class="cx"> std::optional<AppHighlightRangeData> AppHighlightRangeData::create(const SharedBuffer& buffer)
</span><span class="cx"> {
</span><span class="cx">     auto decoder = buffer.decoder();
</span><span class="lines">@@ -81,7 +83,7 @@
</span><span class="cx">     if (!textData)
</span><span class="cx">         return std::nullopt;
</span><span class="cx"> 
</span><del>-    std::optional<unsigned> pathIndex;
</del><ins>+    std::optional<uint32_t> pathIndex;
</ins><span class="cx">     decoder >> pathIndex;
</span><span class="cx">     if (!pathIndex)
</span><span class="cx">         return std::nullopt;
</span><span class="lines">@@ -91,6 +93,12 @@
</span><span class="cx"> 
</span><span class="cx"> template<class Encoder> void AppHighlightRangeData::encode(Encoder& encoder) const
</span><span class="cx"> {
</span><ins>+    static_assert(!Encoder::isIPCEncoder, "AppHighlightRangeData should not be used by IPC::Encoder");
+    constexpr uint64_t currentAppHighlightVersion = 1;
+    
+    encoder << highlightFileSignature;
+    encoder << currentAppHighlightVersion;
+    encoder << m_identifier;
</ins><span class="cx">     encoder << m_text;
</span><span class="cx">     encoder << m_startContainer;
</span><span class="cx">     encoder << m_startOffset;
</span><span class="lines">@@ -100,6 +108,33 @@
</span><span class="cx"> 
</span><span class="cx"> template<class Decoder> std::optional<AppHighlightRangeData> AppHighlightRangeData::decode(Decoder& decoder)
</span><span class="cx"> {
</span><ins>+    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;
+    }
+
</ins><span class="cx">     std::optional<String> text;
</span><span class="cx">     decoder >> text;
</span><span class="cx">     if (!text)
</span><span class="lines">@@ -110,7 +145,7 @@
</span><span class="cx">     if (!startContainer)
</span><span class="cx">         return std::nullopt;
</span><span class="cx"> 
</span><del>-    std::optional<unsigned> startOffset;
</del><ins>+    std::optional<uint32_t> startOffset;
</ins><span class="cx">     decoder >> startOffset;
</span><span class="cx">     if (!startOffset)
</span><span class="cx">         return std::nullopt;
</span><span class="lines">@@ -120,13 +155,12 @@
</span><span class="cx">     if (!endContainer)
</span><span class="cx">         return std::nullopt;
</span><span class="cx"> 
</span><del>-    std::optional<unsigned> endOffset;
</del><ins>+    std::optional<uint32_t> endOffset;
</ins><span class="cx">     decoder >> endOffset;
</span><span class="cx">     if (!endOffset)
</span><span class="cx">         return std::nullopt;
</span><span class="cx"> 
</span><del>-
-    return {{ WTFMove(*text), WTFMove(*startContainer), *startOffset, WTFMove(*endContainer), *endOffset }};
</del><ins>+    return {{ WTFMove(*identifier), WTFMove(*text), WTFMove(*startContainer), *startOffset, WTFMove(*endContainer), *endOffset }};
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> } // namespace WebCore
</span></span></pre></div>
<a id="trunkSourceWebCoreModuleshighlightAppHighlightRangeDatah"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/highlight/AppHighlightRangeData.h (279443 => 279444)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -43,9 +43,9 @@
</span><span class="cx">         String identifier;
</span><span class="cx">         String nodeName;
</span><span class="cx">         String textData;
</span><del>-        unsigned pathIndex { 0 };
</del><ins>+        uint32_t pathIndex { 0 };
</ins><span class="cx"> 
</span><del>-        NodePathComponent(String&& elementIdentifier, String&& name, String&& data, unsigned index)
</del><ins>+        NodePathComponent(String&& elementIdentifier, String&& name, String&& data, uint32_t index)
</ins><span class="cx">             : identifier(WTFMove(elementIdentifier))
</span><span class="cx">             , nodeName(WTFMove(name))
</span><span class="cx">             , textData(WTFMove(data))
</span><span class="lines">@@ -53,7 +53,7 @@
</span><span class="cx">         {
</span><span class="cx">         }
</span><span class="cx"> 
</span><del>-        NodePathComponent(const String& elementIdentifier, const String& name, const String& data, unsigned index)
</del><ins>+        NodePathComponent(const String& elementIdentifier, const String& name, const String& data, uint32_t index)
</ins><span class="cx">             : identifier(elementIdentifier)
</span><span class="cx">             , nodeName(name)
</span><span class="cx">             , textData(data)
</span><span class="lines">@@ -79,8 +79,9 @@
</span><span class="cx"> 
</span><span class="cx">     AppHighlightRangeData(const AppHighlightRangeData&) = default;
</span><span class="cx">     AppHighlightRangeData() = default;
</span><del>-    AppHighlightRangeData(String&& text, NodePath&& startContainer, unsigned startOffset, NodePath&& endContainer, unsigned endOffset)
-        : m_text(WTFMove(text))
</del><ins>+    AppHighlightRangeData(String&& identifier, String&& text, NodePath&& startContainer, uint64_t startOffset, NodePath&& endContainer, uint64_t endOffset)
+        : m_identifier(WTFMove(identifier))
+        , m_text(WTFMove(text))
</ins><span class="cx">         , m_startContainer(WTFMove(startContainer))
</span><span class="cx">         , m_startOffset(startOffset)
</span><span class="cx">         , m_endContainer(WTFMove(endContainer))
</span><span class="lines">@@ -88,8 +89,9 @@
</span><span class="cx">     {
</span><span class="cx">     }
</span><span class="cx"> 
</span><del>-    AppHighlightRangeData(const String& text, const NodePath& startContainer, unsigned startOffset, const NodePath& endContainer, unsigned endOffset)
-        : m_text(text)
</del><ins>+    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)
</ins><span class="cx">         , m_startContainer(startContainer)
</span><span class="cx">         , m_startOffset(startOffset)
</span><span class="cx">         , m_endContainer(endContainer)
</span><span class="lines">@@ -97,11 +99,12 @@
</span><span class="cx">     {
</span><span class="cx">     }
</span><span class="cx"> 
</span><ins>+    const String& identifier() const { return m_identifier; }
</ins><span class="cx">     const String& text() const { return m_text; }
</span><span class="cx">     const NodePath& startContainer() const { return m_startContainer; }
</span><del>-    unsigned startOffset() const { return m_startOffset; }
</del><ins>+    uint32_t startOffset() const { return m_startOffset; }
</ins><span class="cx">     const NodePath& endContainer() const { return m_endContainer; }
</span><del>-    unsigned endOffset() const { return m_endOffset; }
</del><ins>+    uint32_t endOffset() const { return m_endOffset; }
</ins><span class="cx"> 
</span><span class="cx">     template<class Encoder> void encode(Encoder&) const;
</span><span class="cx">     template<class Decoder> static std::optional<AppHighlightRangeData> decode(Decoder&);
</span><span class="lines">@@ -109,11 +112,12 @@
</span><span class="cx">     Ref<SharedBuffer> toSharedBuffer() const;
</span><span class="cx"> 
</span><span class="cx"> private:
</span><ins>+    String m_identifier;
</ins><span class="cx">     String m_text;
</span><span class="cx">     NodePath m_startContainer;
</span><del>-    unsigned m_startOffset { 0 };
</del><ins>+    uint32_t m_startOffset { 0 };
</ins><span class="cx">     NodePath m_endContainer;
</span><del>-    unsigned m_endOffset { 0 };
</del><ins>+    uint32_t m_endOffset { 0 };
</ins><span class="cx"> };
</span><span class="cx"> 
</span><span class="cx"> #endif
</span></span></pre></div>
<a id="trunkSourceWebCoreModuleshighlightAppHighlightStoragecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/highlight/AppHighlightStorage.cpp (279443 => 279444)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -43,6 +43,7 @@
</span><span class="cx"> #include "StaticRange.h"
</span><span class="cx"> #include "TextIndicator.h"
</span><span class="cx"> #include "TextIterator.h"
</span><ins>+#include <wtf/UUID.h>
</ins><span class="cx"> 
</span><span class="cx"> namespace WebCore {
</span><span class="cx"> 
</span><span class="lines">@@ -203,7 +204,10 @@
</span><span class="cx"> {
</span><span class="cx">     auto text = plainText(range);
</span><span class="cx">     text.truncate(textPreviewLength);
</span><ins>+    auto identifier = createCanonicalUUIDString();
+
</ins><span class="cx">     return {
</span><ins>+        identifier,
</ins><span class="cx">         text,
</span><span class="cx">         makeNodePath(&range.startContainer()),
</span><span class="cx">         range.startOffset(),
</span></span></pre></div>
<a id="trunkSourceWebKitChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/ChangeLog (279443 => 279444)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,19 @@
</span><ins>+2021-06-30  Megan Gardner  <megan_gardner@apple.com>
+
+        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):
+
</ins><span class="cx"> 2021-06-30  Truitt Savell  <tsavell@apple.com>
</span><span class="cx"> 
</span><span class="cx">         Unreviewed, reverting r279405.
</span></span></pre></div>
<a id="trunkSourceWebKitWebProcessWebPageWebPagecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (279443 => 279444)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -7617,7 +7617,7 @@
</span><span class="cx">         if (!sharedMemory)
</span><span class="cx">             continue;
</span><span class="cx"> 
</span><del>-        document->appHighlightStorage().restoreAndScrollToAppHighlight(SharedBuffer::create(static_cast<const uint8_t*>(sharedMemory->data()), sharedMemory->size()), i == index ? ScrollToHighlight::Yes : ScrollToHighlight::No);
</del><ins>+        document->appHighlightStorage().restoreAndScrollToAppHighlight(SharedBuffer::create(static_cast<const uint8_t*>(sharedMemory->data()), ipcHandle.dataSize), i == index ? ScrollToHighlight::Yes : ScrollToHighlight::No);
</ins><span class="cx">         i++;
</span><span class="cx">     }
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkToolsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Tools/ChangeLog (279443 => 279444)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/ChangeLog    2021-06-30 23:53:16 UTC (rev 279443)
+++ trunk/Tools/ChangeLog       2021-06-30 23:54:40 UTC (rev 279444)
</span><span class="lines">@@ -1,3 +1,15 @@
</span><ins>+2021-06-30  Megan Gardner  <megan_gardner@apple.com>
+
+        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):
+
</ins><span class="cx"> 2021-06-30  Wenson Hsieh  <wenson_hsieh@apple.com>
</span><span class="cx"> 
</span><span class="cx">         [iOS] [Live Text] "Text from Camera" should not be shown in callout bar when selecting text
</span></span></pre></div>
<a id="trunkToolsTestWebKitAPITestsWebKitCocoaWKAppHighlightsmm"></a>
<div class="modfile"><h4>Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAppHighlights.mm (279443 => 279444)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -56,94 +56,165 @@
</span><span class="cx"> 
</span><span class="cx"> namespace TestWebKitAPI {
</span><span class="cx"> 
</span><del>-TEST(AppHighlights, AppHighlightCreateAndRestore)
</del><ins>+RetainPtr<_WKAppHighlight> createAppHighlightWithHTML(NSString *HTMLString, NSString *javaScript, NSString *highlightText)
</ins><span class="cx"> {
</span><span class="cx">     WKWebViewConfiguration *configuration = [WKWebViewConfiguration _test_configurationWithTestPlugInClassName:@"WebProcessPlugInWithInternals" configureJSCForTesting:YES];
</span><span class="cx">     auto delegate = adoptNS([[AppHighlightDelegate alloc] init]);
</span><span class="cx">     auto webViewCreate = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500) configuration:configuration]);
</span><del>-    auto webViewRestore = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500) configuration:configuration]);
</del><span class="cx">     [webViewCreate _setAppHighlightDelegate:delegate.get()];
</span><del>-    [webViewCreate synchronouslyLoadHTMLString:@"Test"];
-    [webViewCreate stringByEvaluatingJavaScript:@"document.execCommand('SelectAll')"];
-    __block bool finished = NO;
</del><ins>+    [webViewCreate synchronouslyLoadHTMLString:HTMLString];
+    [webViewCreate stringByEvaluatingJavaScript:javaScript];
+    __block RetainPtr<_WKAppHighlight> resultHighlight;
+    __block bool done = false;
</ins><span class="cx">     [delegate setStoreAppHighlightCallback:^(WKWebView *delegateWebView, _WKAppHighlight *highlight, BOOL inNewGroup, BOOL requestOriginatedInApp) {
</span><span class="cx">         EXPECT_EQ(delegateWebView, webViewCreate.get());
</span><span class="cx">         EXPECT_NOT_NULL(highlight);
</span><del>-        EXPECT_WK_STREQ(highlight.text, @"Test");
</del><ins>+        EXPECT_WK_STREQ(highlight.text, highlightText);
</ins><span class="cx">         EXPECT_NOT_NULL(highlight.highlight);
</span><del>-        
-        [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;
</del><ins>+        resultHighlight = highlight;
+        done = true;
</ins><span class="cx">     }];
</span><span class="cx">     [webViewCreate _addAppHighlight];
</span><del>-    TestWebKitAPI::Util::run(&finished);
</del><ins>+    TestWebKitAPI::Util::run(&done);
+    return resultHighlight;
</ins><span class="cx"> }
</span><span class="cx"> 
</span><del>-TEST(AppHighlights, AppHighlightCreateAndRestoreAndScroll)
</del><ins>+RetainPtr<WKWebView> createWebViewForAppHighlightsWithHTML(NSString *HTMLString)
</ins><span class="cx"> {
</span><span class="cx">     WKWebViewConfiguration *configuration = [WKWebViewConfiguration _test_configurationWithTestPlugInClassName:@"WebProcessPlugInWithInternals" configureJSCForTesting:YES];
</span><del>-    auto delegate = adoptNS([[AppHighlightDelegate alloc] init]);
-    auto webViewCreate = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500) configuration:configuration]);
</del><span class="cx">     auto webViewRestore = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500) configuration:configuration]);
</span><del>-    [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);
</del><ins>+    [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");
</ins><span class="cx">         
</span><del>-        [webViewRestore synchronouslyLoadHTMLString:@"<div style='height: 1000px'></div>Test"];
-        [webViewRestore _restoreAndScrollToAppHighlight:highlight.highlight];
</del><ins>+    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");
</ins><span class="cx">         
</span><del>-        TestWebKitAPI::Util::waitForConditionWithLogging([&] () -> bool {
-            return [webViewRestore stringByEvaluatingJavaScript:@"internals.numberOfAppHighlights()"].intValue == 1;
-        }, 2, @"Expected Highlights to be populated.");
-        EXPECT_NE(0, [[webViewRestore objectByEvaluatingJavaScript:@"pageYOffset"] floatValue]);
</del><ins>+    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");
</ins><span class="cx">         
</span><del>-        finished = YES;
-    }];
-    [webViewCreate _addAppHighlight];
-    TestWebKitAPI::Util::run(&finished);
</del><ins>+    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.");
</ins><span class="cx"> }
</span><span class="cx"> 
</span><del>-TEST(AppHighlights, AppHighlightRestoreFailure)
</del><ins>+// Ensure that the original data format will always be able to be decoded in the future.
+TEST(AppHighlights, AppHighlightRestoreFromStorageV0)
</ins><span class="cx"> {
</span><span class="cx">     WKWebViewConfiguration *configuration = [WKWebViewConfiguration _test_configurationWithTestPlugInClassName:@"WebProcessPlugInWithInternals" configureJSCForTesting:YES];
</span><del>-    auto delegate = adoptNS([[AppHighlightDelegate alloc] init]);
-    auto webViewCreate = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500) configuration:configuration]);
</del><ins>+    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];
+
</ins><span class="cx">     auto webViewRestore = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500) configuration:configuration]);
</span><del>-    [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);
</del><ins>+
+    [webViewRestore synchronouslyLoadHTMLString:@"Test"];
+    [webViewRestore _restoreAppHighlights:@[ storedData ]];
+    
+    TestWebKitAPI::Util::waitForConditionWithLogging([&] () -> bool {
+        return [webViewRestore stringByEvaluatingJavaScript:@"internals.numberOfAppHighlights()"].intValue == 1;
+    }, 2, @"Expected Highlights to be populated.");
</ins><span class="cx"> }
</span><span class="cx"> 
</span><ins>+
+// 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.");
+}
+
</ins><span class="cx"> #if PLATFORM(IOS_FAMILY)
</span><span class="cx"> 
</span><span class="cx"> TEST(AppHighlights, AvoidForcingCalloutBarInitialization)
</span></span></pre>
</div>
</div>

</body>
</html>