Bug 51853

Summary: Add code to encode/decode the back/forward tree
Product: WebKit Reporter: Darin Adler <darin>
Component: Page LoadingAssignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, beidson, buildbot, ddkilzer, dglazkov, eric, gyuyoung.kim, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch beidson: review+

Description Darin Adler 2011-01-03 16:25:41 PST
Add code to encode/decode the back/forward tree
Comment 1 Darin Adler 2011-01-03 16:29:20 PST
Created attachment 77851 [details]
Patch
Comment 2 WebKit Review Bot 2011-01-03 16:44:13 PST
Attachment 77851 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7231354
Comment 3 Brady Eidson 2011-01-03 16:52:32 PST
Comment on attachment 77851 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=77851&action=review

> WebCore/history/HistoryItem.cpp:642
> +        encoder->encodeString(child.m_title);

We probably don't need the titles for children.

> WebCore/history/HistoryItem.cpp:644
> +        encoder->encodeString(child.m_urlString);

If we don't need this for children, it'd simplify the code a lot (or a bit (or not at all))

> WebCore/history/HistoryItem.cpp:710
> +            return 0;

As above, probably don't need this.

> WebCore/history/HistoryItem.cpp:715
> +        recursionStack.append(DecodeBackForwardTreeRecursionStackElement(node.release(), i));

Need to store size here, too.

> WebCore/history/HistoryItem.cpp:778
> +        recursionStack.removeLast();

Move this down till you're done using "element"

> WebCore/platform/network/FormData.cpp:338
> +        encoder->encodeBytes(reinterpret_cast<const uint8_t*>(element.m_data.data()), element.m_data.size());

This cast probably isn't needed anymore.

> WebCore/platform/network/FormData.cpp:420
> +}

The encoder doesn't encode everything the decoder decodes....!!!!  Boo-urns
Comment 4 Darin Adler 2011-01-03 17:21:07 PST
Created attachment 77854 [details]
Patch
Comment 5 Darin Adler 2011-01-03 17:35:30 PST
Committed r74951: <http://trac.webkit.org/changeset/74951>
Comment 6 WebKit Review Bot 2011-01-03 17:35:53 PST
Attachment 77854 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7343188
Comment 7 Build Bot 2011-01-03 17:43:51 PST
Attachment 77854 [details] did not build on win:
Build output: http://queues.webkit.org/results/7282335
Comment 8 WebKit Review Bot 2011-01-03 17:47:07 PST
Attachment 77851 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7220396
Comment 9 Gyuyoung Kim 2011-01-03 17:59:33 PST
It seems to me that this patch makes a build break in WebKitEFL as below,


[ 99%] Building C object WebCore/CMakeFiles/webcore_efl.dir/platform/network/soup/cache/soup-directory-input-stream.c.o
/home/gyuyoung/webkit/working-source/WebKit-video/WebCore/platform/network/soup/ResourceHandleSoup.cpp: In function ‘bool WebCore::startHttp(WebCore::ResourceHandle*)’:
/home/gyuyoung/webkit/working-source/WebKit-video/WebCore/platform/network/soup/ResourceHandleSoup.cpp:617: error: no matching function for call to ‘WebCore::FormData::flatten(WTF::Vector<char, 0u>&)’
/home/gyuyoung/webkit/working-source/WebKit-video/WebCore/platform/network/FormData.h:114: note: candidates are: void WebCore::FormData::flatten(WTF::Vector<unsigned char, 0u>&) const
[ 99%] [ 99%] Building C object WebCore/CMakeFiles/webcore_efl.dir/platform/network/soup/cache/soup-http-input-stream.c.o
Building C object WebCore/CMakeFiles/webcore_efl.dir/platform/network/soup/cache/soup-request-data.c.o
make[2]: *** [WebCore/CMakeFiles/webcore_efl.dir/platform/network/soup/ResourceHandleSoup.cpp.o] Error 1
Comment 10 WebKit Review Bot 2011-01-03 18:03:40 PST
http://trac.webkit.org/changeset/74951 might have broken Leopard Intel Debug (Build)
Comment 11 David Kilzer (:ddkilzer) 2011-01-03 21:13:30 PST
(In reply to comment #10)
> http://trac.webkit.org/changeset/74951 might have broken Leopard Intel Debug (Build)

/Volumes/Big/WebKit-BuildSlave/leopard-intel-release/build/WebCore/history/HistoryItem.cpp:727: warning: implicit conversion shortens 64-bit value into a 32-bit value

A uint64_t variable is passed as a size_t parameter.
Comment 12 David Kilzer (:ddkilzer) 2011-01-04 09:46:42 PST
(In reply to comment #10)
> http://trac.webkit.org/changeset/74951 might have broken Leopard Intel Debug (Build)

World build fix:  r74952  <http://trac.webkit.org/changeset/74952>
Leopard build fix: r74955  <http://trac.webkit.org/changeset/74955>
Chromium build fix: r74956  <http://trac.webkit.org/changeset/74956>