Bug 51853 - Add code to encode/decode the back/forward tree
Summary: Add code to encode/decode the back/forward tree
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Darin Adler
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-03 16:25 PST by Darin Adler
Modified: 2011-01-04 09:46 PST (History)
8 users (show)

See Also:


Attachments
Patch (16.67 KB, patch)
2011-01-03 16:29 PST, Darin Adler
no flags Details | Formatted Diff | Diff
Patch (18.46 KB, patch)
2011-01-03 17:21 PST, Darin Adler
beidson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>