RESOLVED FIXED 110438
[WK2][EFL] Clean up PageViewportControllerClientEfl class 
https://bugs.webkit.org/show_bug.cgi?id=110438
Summary [WK2][EFL] Clean up PageViewportControllerClientEfl class
Mikhail Pozdnyakov
Reported 2013-02-21 01:14:15 PST
PageViewportControllerClientEfl contains surprisingly a lot of methods and members which: 1) useless 2) accessing WK2 internals.
Attachments
patch (7.17 KB, patch)
2013-02-21 02:07 PST, Mikhail Pozdnyakov 		no flags
DetailsFormatted DiffDiff
patch v2 (7.24 KB, patch)
2013-02-21 04:51 PST, Mikhail Pozdnyakov 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Mikhail Pozdnyakov
Comment 1 2013-02-21 02:07:15 PST
Created attachment 189482 [details] patch
Kenneth Rohde Christiansen
Comment 2 2013-02-21 03:52:58 PST
Comment on attachment 189482 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=189482&action=review LGTM > Source/WebKit2/UIProcess/efl/PageViewportControllerClientEfl.h:46 > + void setViewportPosition(const WebCore::FloatPoint&); OVERRIDE? is are they not? > Source/WebKit2/UIProcess/efl/WebView.cpp:159 > + FloatSize size = m_ewkView->size(); > + // The viewport controller expects sizes in UI units, and not raw device units. > + size.scale(1 / m_page->deviceScaleFactor()); cant that be one line? doesnt scale return a size?
Mikhail Pozdnyakov
Comment 3 2013-02-21 04:21:29 PST
(In reply to comment #2) > (From update of attachment 189482 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=189482&action=review > > LGTM > > > Source/WebKit2/UIProcess/efl/PageViewportControllerClientEfl.h:46 > > + void setViewportPosition(const WebCore::FloatPoint&); > > OVERRIDE? is are they not? yeah, I'll add it. > > > Source/WebKit2/UIProcess/efl/WebView.cpp:159 > > + FloatSize size = m_ewkView->size(); > > + // The viewport controller expects sizes in UI units, and not raw device units. > > + size.scale(1 / m_page->deviceScaleFactor()); > > cant that be one line? doesnt scale return a size? Unfortunately not :/
Mikhail Pozdnyakov
Comment 4 2013-02-21 04:51:51 PST
Created attachment 189510 [details] patch v2 Took comments from Kenneth into consideration.
Kenneth Rohde Christiansen
Comment 5 2013-02-22 01:25:28 PST
Comment on attachment 189510 [details] patch v2 LGTM
WebKit Review Bot
Comment 6 2013-02-22 01:35:38 PST
Comment on attachment 189510 [details] patch v2 Clearing flags on attachment: 189510 Committed r143705: <http://trac.webkit.org/changeset/143705>
WebKit Review Bot
Comment 7 2013-02-22 01:35:43 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.