Bug 82500

Summary: Remove redundant updateViewportArguments() call when page is restored from page cache.
Product: WebKit Reporter: zalan <zalan>
Component: WebCore Misc.Assignee: zalan <zalan>
Severity: Minor CC: kenneth, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Description Flags
Patch none

Description zalan 2012-03-28 12:06:46 PDT
When the page is restored from the page cache, Document::updateViewportArguments() is called twice. Once when the document is set (restored) on the mainframe, and once the Page Cache signals restore complete.
Viewport arguments don't change between the 2 calls, so it's enough to update them only once.
Fix: remove updateViewportArguments() from Document::documentDidResumeFromPageCache()
Comment 1 zalan 2012-03-28 12:32:51 PDT
Created attachment 134372 [details]
Comment 2 Kenneth Rohde Christiansen 2012-03-29 04:58:22 PDT
Comment on attachment 134372 [details]

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

> ManualTests/viewport-width-test-after-history-navigation.html:17
> +            originalViewportWidth = window.innerWidth;

Why not check that it is actually 700 ?

> ManualTests/viewport-width-test-after-history-navigation.html:22
> +            document.getElementById('anchor').dispatchEvent(evt);

I would add a newline after this

> Source/WebCore/dom/Document.cpp:4225
>      m_frame->loader()->client()->dispatchDidBecomeFrameset(isFrameSet());
> -
> -    updateViewportArguments();
>  }

I am not convinced... This might fit well with the width etc, but what happens if I navigate back and I am in another orientation? this would break for width = device-width. Also I am not sure we will get all the right limits to our interaction engine, such as maximum-scale etc.
Comment 3 WebKit Review Bot 2012-03-29 05:53:22 PDT
Comment on attachment 134372 [details]

Clearing flags on attachment: 134372

Committed r112526: <http://trac.webkit.org/changeset/112526>
Comment 4 WebKit Review Bot 2012-03-29 05:53:28 PDT
All reviewed patches have been landed.  Closing bug.