Bug 87013 - [Qt] Mini clean ups in the interaction engine
Summary: [Qt] Mini clean ups in the interaction engine
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kenneth Rohde Christiansen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-21 06:15 PDT by Kenneth Rohde Christiansen
Modified: 2012-05-22 05:29 PDT (History)
5 users (show)

See Also:


Attachments
Patch (3.18 KB, patch)
2012-05-21 06:16 PDT, Kenneth Rohde Christiansen
hausmann: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kenneth Rohde Christiansen 2012-05-21 06:15:36 PDT
SSIA
Comment 1 Kenneth Rohde Christiansen 2012-05-21 06:16:51 PDT
Created attachment 143017 [details]
Patch
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]
Patch

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.