[webkit-reviews] review denied: [Bug 46385] Keep viewport information in Document : [Attachment 68564] patch

[bugzilla-daemon at webkit.org]

Kenneth Rohde Christiansen <kenneth at webkit.org> has denied Luiz Agostini
<luiz at webkit.org>'s request for review:
Bug 46385: Keep viewport information in Document
https://bugs.webkit.org/show_bug.cgi?id=46385

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

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=68564&action=review

> WebCore/ChangeLog:8
> +	   For viewport meta tag information to be kept by page cache it needs
to be in Document class.

*in the*.

Add some more info, like "The viewport meta data (layout viewport, scale) needs
to be set again when bringing back a page from the page cache. As the viewport
data are only retrieved while parsing the document, we need to store it in the
Document class." 

As the viewport meta data is only retrieved during parsing,

> WebCore/dom/Document.cpp:3815
> +	      
frame()->page()->chrome()->client()->didReceiveViewportArguments(frame(),
m_viewportArguments);

I think we need to rename didReceiveViewportArguments so better describe that
it does. maybe dispatchViewportChangeRequested or similar?

> WebCore/dom/Document.h:280
> +    ViewportArguments viewportArguments() const { return
m_viewportArguments; }

I think it is about time to change ViewportArguments to something like
ViewportMetaData, but that can be in another patch.

> WebCore/dom/Document.h:1319
> +    ViewportArguments m_viewportArguments;

Maybe the m_valid in ViewportArguments doesn't make sense anymore