[webkit-reviews] review denied: [Bug 51853] Add code to encode/decode the back/forward tree : [Attachment 77851] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jan 3 16:52:31 PST 2011


Brady Eidson <beidson at apple.com> has denied Darin Adler <darin at apple.com>'s
request for review:
Bug 51853: Add code to encode/decode the back/forward tree
https://bugs.webkit.org/show_bug.cgi?id=51853

Attachment 77851: Patch
https://bugs.webkit.org/attachment.cgi?id=77851&action=review

------- Additional Comments from Brady Eidson <beidson at apple.com>
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


More information about the webkit-reviews mailing list