Bug 87013

Summary: [Qt] Mini clean ups in the interaction engine
Product: WebKit Reporter: Kenneth Rohde Christiansen <kenneth>
Component: WebKit QtAssignee: Kenneth Rohde Christiansen <kenneth>
Severity: Normal CC: hausmann, menard, webkit.review.bot, zalan, zoltan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Description Flags
Patch hausmann: review+

Description Kenneth Rohde Christiansen 2012-05-21 06:15:36 PDT
Comment 1 Kenneth Rohde Christiansen 2012-05-21 06:16:51 PDT
Created attachment 143017 [details]
Comment 2 zalan 2012-05-21 07:13:56 PDT
a little bit unrelated but since you are touching animateItemRectVisible(): it looks like after a few refactorings, it does not need to be public method anymore (neither it's pair setItemRectVisible()) Could you commit them as private, unless there's a good reason to still have them around as public?
Comment 3 Simon Hausmann 2012-05-22 02:43:39 PDT
Comment on attachment 143017 [details]

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

I'll have to trust you on the logic and welcome any change that adds test coverage to those magic if statements ;-)

> Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:191
> +    ASSERT(m_suspended);

I bet Zalan is going to run into this one ;)

> Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:618
> + * FIXME: This is currently called twise if you concurrently change width and height.

twise -> twice
Comment 4 Kenneth Rohde Christiansen 2012-05-22 04:55:00 PDT
Landed in 117952

Sure Zalan will run into the assert :-) that is why we add them!
Comment 5 zalan 2012-05-22 05:29:28 PDT
(In reply to comment #4)
> Landed in 117952
> Sure Zalan will run into the assert :-) that is why we add them!

and i am so grateful for that.