[Webkit-unassigned] [Bug 100674] [EFL][WK2] Allow using ACCELERATED_COMPOSITING without COORDINATED_GRAPHICS

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=100674





--- Comment #32 from Yael <yael.aharon.m at gmail.com>  2012-11-02 19:48:51 PST ---
(In reply to comment #26)
> View in context: https://bugs.webkit.org/attachment.cgi?id=172138&action=review
> 
> > Source/WebKit2/UIProcess/API/efl/EwkViewImpl.cpp:133
> > +        m_pageProxy->setUseFixedLayout(true);
> 
> m_pageProxy->setUseFixedLayout(mode == FixedLayoutMode);  on 1 line?
> 
That will trigger calling  setUseFixedLayout(false) needlessly.

> > Source/WebKit2/UIProcess/API/efl/EwkViewImpl.h:187
> > +    WebCore::IntPoint scrollPosition() const { return m_scrollPosition; }
> 
> You could return a const reference here.
> 
ok
> > Source/WebKit2/UIProcess/efl/PageClientImplFixedLayout.cpp:51
> > +void PageClientImplFixedLayout::initialize()
> 
> Now that we have 3 classes, why do we still need this initialise() method? Why can't we move this code to the constructor?
> 
ok
> > Source/WebKit2/UIProcess/efl/PageClientImplFixedLayout.h:49
> > +
> 
> extra line here.
> 
ok
> > Source/WebKit2/UIProcess/efl/PageViewportControllerClientEfl.cpp:89
> > +    m_viewImpl->setScrollPosition(IntPoint(contentsPoint.x(), contentsPoint.y()));
> 
> We could store IntPoint(contentsPoint.x(), contentsPoint.y()) in a variable to construct it once instead of twice.
I was told before not to create this kind of temporary variables. I wonder if there is something in the coding style about that. (Did it anyways :)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.