Bug 105978 - [EFL][Qt][WK2] We should consider a page scale factor in WebCore instead of our own scale factor.
Summary: [EFL][Qt][WK2] We should consider a page scale factor in WebCore instead of o...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dongseong Hwang
URL:
Keywords:
: 104374 (view as bug list)
Depends on: 107361 107424 107653 107760 107802
Blocks: 105187 103105 105766 107656 108561
  Show dependency treegraph
 
Reported: 2013-01-02 17:34 PST by Dongseong Hwang
Modified: 2013-01-31 18:33 PST (History)
22 users (show)

See Also:


Attachments
WIP Patch (56.01 KB, patch)
2013-01-02 17:40 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
WIP Patch2 (55.44 KB, patch)
2013-01-09 01:30 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
Patch (34.03 KB, patch)
2013-01-23 03:42 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
Patch (27.00 KB, patch)
2013-01-24 01:44 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
Patch (27.00 KB, patch)
2013-01-24 01:58 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
Patch (26.79 KB, patch)
2013-01-30 02:47 PST, Dongseong Hwang
simon.fraser: review+
simon.fraser: commit-queue+
Details | Formatted Diff | Diff
Patch for landing (26.69 KB, patch)
2013-01-30 13:55 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dongseong Hwang 2013-01-02 17:34:44 PST
Curruntly, Coordinated Graphics manages a duplicated scale. Remove code related to a duplicated scale.

There are two major changes.
1. Don't keep scale. Instead query the scale from WebPage[Proxy].
2. CoordinatedLayerTreeHost::SetVisibleContentsRect message does not send a scale anymore. Instead reuse IPC communication of WebPage, and callback mechanism of GraphicsLayer.

FYI, note what is effectiveScaleFactor.
effectiveScaleFactor is pageScaleFactor * deviceScaleFactor.
platform webview and pageViewportController know both pageScaleFactor and deviceScaleFactor.
But CoordinatedGraphicsLayer and TiledBackingStore need to know only effectiveScaleFactor.
CoordinatedGraphicsLayer would snap pixel alignment using effectiveScaleFactor, and TiledBackingStore paints contents using effectiveScaleFactor.

However, GraphicsLayerCA snaps pixel alignment using pageScaleFactor. I think it is a potential bug, but apple uses only 2 as a deviceScaleFactor, so this potential bug does not appear yet.
In conclusion, CoordinatedGraphicsLayer and TiledBackingStore need to know final scale for device, and it is effectiveScaleFactor.
Comment 1 Dongseong Hwang 2013-01-02 17:40:34 PST
Created attachment 181120 [details]
WIP Patch
Comment 2 Dongseong Hwang 2013-01-02 17:42:54 PST
Comment on attachment 181120 [details]
WIP Patch

Post WIP patch to clarify the direction.

I'm not sure yet this code is correct in each platform specific code: webview, pageClient, pageViewportControllerClient. And I need more test.

I worked based on kenneth's xmaxpatch :)
Comment 3 Build Bot 2013-01-02 18:36:24 PST
Comment on attachment 181120 [details]
WIP Patch

Attachment 181120 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15639622
Comment 4 Mikhail Pozdnyakov 2013-01-03 00:26:17 PST
Comment on attachment 181120 [details]
WIP Patch

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

> Source/WebKit2/UIProcess/API/efl/EwkViewImpl.h:114
> +    WebKit::WebPageProxy* page() const { return m_pageProxy.get(); }

this method returns mutable pointer, so should not be const
Comment 5 Andras Becsi 2013-01-03 02:16:53 PST
(In reply to comment #0)
> Curruntly, Coordinated Graphics manages a duplicated scale. Remove code related to a duplicated scale.
> 
> There are two major changes.
> 1. Don't keep scale. Instead query the scale from WebPage[Proxy].
> 2. CoordinatedLayerTreeHost::SetVisibleContentsRect message does not send a scale anymore. Instead reuse IPC communication of WebPage, and callback mechanism of GraphicsLayer.
> 
> FYI, note what is effectiveScaleFactor.
> effectiveScaleFactor is pageScaleFactor * deviceScaleFactor.
> platform webview and pageViewportController know both pageScaleFactor and deviceScaleFactor.
> But CoordinatedGraphicsLayer and TiledBackingStore need to know only effectiveScaleFactor.
> CoordinatedGraphicsLayer would snap pixel alignment using effectiveScaleFactor, and TiledBackingStore paints contents using effectiveScaleFactor.
> 
> However, GraphicsLayerCA snaps pixel alignment using pageScaleFactor. I think it is a potential bug, but apple uses only 2 as a deviceScaleFactor, so this potential bug does not appear yet.
> In conclusion, CoordinatedGraphicsLayer and TiledBackingStore need to know final scale for device, and it is effectiveScaleFactor.

I think this patch should be aligned with the work I do for Qt on retina display.
Since the coordinates in Qt are always logical and the device scale factor is applied on the backing store and during painting in QtWebPageSGNode, the device scale factor logic in PageViewportController is redundant and will be removed.
My patch for that also still needs testing, and I have to align the change with the way EFL is handling dpxr.

I'm back from vacation on monday, please do not land this before next week.
Comment 6 Mikhail Pozdnyakov 2013-01-03 04:44:05 PST
Comment on attachment 181120 [details]
WIP Patch

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

> Source/WebKit2/ChangeLog:8
> +        Curruntly, Coordinated Graphics manages a duplicated scale. Remove code about a

Spelling.

> Source/WebKit2/UIProcess/API/efl/EwkViewImpl.cpp:507
> +    return page()->pageScaleFactor() * page()->deviceScaleFactor();

Are you sure 'page()->pageScaleFactor()' is initialized properly here?

> Source/WebKit2/UIProcess/PageViewportController.cpp:50
> +    , m_effectiveScaleFactor(1)

maybe renaming worth creating a separate patch?

> Source/WebKit2/UIProcess/efl/PageViewportControllerClientEfl.h:55
> +    virtual void setContentsScale(float) { }

could this method be removed?
Comment 7 Dongseong Hwang 2013-01-03 16:39:53 PST
(In reply to comment #6)
> (From update of attachment 181120 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=181120&action=review

Thank Mikhail for review.
I rewrite the second WIP patch. I will apply your review :)

(In reply to comment #5)
> (In reply to comment #0)
> I think this patch should be aligned with the work I do for Qt on retina display.
What a coincidence! great.
> Since the coordinates in Qt are always logical and the device scale factor is applied on the backing store and during painting in QtWebPageSGNode, the device scale factor logic in PageViewportController is redundant and will be removed.
Do you means TiledBackingStore must know both pageScale and deviceScale? However TileCache in Apple knows only effectiveScale ( = pageScale * deviceScale). And I think PageViewportController needs to know effectiveScale because PageViewportController must adjust scroll position to avoid noncompositing layer blurring.

> My patch for that also still needs testing, and I have to align the change with the way EFL is handling dpxr.
> 
> I'm back from vacation on monday, please do not land this before next week.

Ok, I wait for you. I found a lot of problems as made this bug. I need to cowork and discuss in depth. :)


As I mentioned, I encountered many problems. I list some of them.
1. delegating scroll : we sometimes change scrollPosition of ScrollView  (i.e. select text using mouse) and sometimes ignore (most cases). However, changing scrollPosition of WebCore does not make anything except for calling PageDidRequestScroll callback.
I think we will remove scroll delegating so that WebCore draws scroll bar and we use Scrolling thread. Am I right?
Page::setPageScaleFactor(float scale, const IntPoint& origin) requires scroll position, but different scroll position arguments do not make difference on Coordinated Graphics.

2. page scale : Currently we never change page scale when using Minibrowser, but we can change via WK2 API. CoordiantedGraphicsLayer and TiledBackingStore seems to assume pageScale is always 1. We need to change CoordiantedGraphicsLayer and TiledBackingStore to get along with pageScale in WebCore.

3. There are complex and slow scroll position and scale communication routine related to PageViewportController.
For example, we need 4 times IPC communication to run wheel event.

      UI Process                      Message               Web Process
WebPageProxy                       (wheel ->)                WebPage
PageViewportController    (<- PageDidRequestScroll)   WebPage
PageViewportController    (setVisibleContentsRect ->) CoordinatedLayerTree
PageViewportController        (<- didRenderFrame)      CoordinatedLayerTree
LayerTreeRenderer::paint

Scale handling ditto.
We implemented PageViewportController runs drawing after receiving callback from Web Process. We need to change.
Comment 8 Mikhail Pozdnyakov 2013-01-07 08:39:44 PST
> 2. page scale : Currently we never change page scale when using Minibrowser, but we can change via WK2 API. CoordiantedGraphicsLayer and TiledBackingStore seems to assume pageScale is always 1. We need to change CoordiantedGraphicsLayer and TiledBackingStore to get along with pageScale in WebCore.
> 

https://bugs.webkit.org/show_bug.cgi?id=104374 is targeted to solve it.
However the solution would also require fix of the current bug.
Comment 9 Dongseong Hwang 2013-01-07 15:58:45 PST
(In reply to comment #8)
> https://bugs.webkit.org/show_bug.cgi?id=104374 is targeted to solve it.
> However the solution would also require fix of the current bug.

Aha, this bug was already created! I think both are duplicated.
Comment 10 Dongseong Hwang 2013-01-09 01:26:21 PST
*** Bug 104374 has been marked as a duplicate of this bug. ***
Comment 11 Dongseong Hwang 2013-01-09 01:30:18 PST
Created attachment 181873 [details]
WIP Patch2
Comment 12 Dongseong Hwang 2013-01-09 01:44:37 PST
Comment on attachment 181873 [details]
WIP Patch2

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

This is the first working patch, that means the first patch work abnormally :)

I post this patch to explain the direction.
We need prerequisite refactoring before this patch.
1. Decide whether transform include deviceScale as well as pageScale.
2. The size of WebPage (a.k.a nonCompositingLayer) is on viewport coordinated system, not contents coordinated system although the size of all GrahpicsLayer is on contents coordinated system.

TODO: As I mentioned, sending scroll and scall to Web Process is not efficient. We need to refactor further.

> Source/WebCore/css/StyleResolver.cpp:1456
> +//        documentStyle->setPageScaleTransform(frame ? frame->frameScaleFactor() : 1);

If we don't comment out, coordinated graphics draws with pageScale^2. It means if pageScale is 2, we can see 4 times scaled contents.

In addition, if we really want transform includes scale info, we should make transform include deviceRatioFactor and we should make GraphicsLayer not know about scale.
IMO current implementation is ambiguous.

> Source/WebKit2/UIProcess/PageViewportController.h:112
> +    float m_contentsScale; // Should always be cssScale * devicePixelRatio.

Define m_contentsScale that is cssScale * devicePixelRatio because GraphicsLayerCA and TiledBackingStore also use the contentsScale term.
Comment 13 Build Bot 2013-01-09 02:00:36 PST
Comment on attachment 181873 [details]
WIP Patch2

Attachment 181873 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15763457

New failing tests:
fast/frames/frame-set-scaling-hit.html
fast/dom/window-scroll-scaling.html
compositing/layer-creation/fixed-position-out-of-view-scaled.html
fast/dom/Range/scale-page-client-rects.html
fast/dom/elementFromPoint-scaled-scrolled.html
inspector/elements/highlight-node-scaled.html
fast/frames/frame-set-rotation-hit.html
compositing/layer-creation/fixed-position-out-of-view-scaled-scroll.html
fast/dom/Element/scale-page-bounding-client-rect.html
fast/dom/Element/scale-page-client-rects.html
fast/events/page-scaled-mouse-click-iframe.html
fast/dom/Range/scale-page-bounding-client-rect.html
fast/transforms/selection-bounds-in-transformed-view.html
svg/as-image/image-respects-pageScaleFactor.html
compositing/tiling/tile-cache-zoomed.html
Comment 14 Dominik Röttsches (drott) 2013-01-10 05:53:26 PST
(In reply to comment #12)

> In addition, if we really want transform includes scale info, we should make transform include deviceRatioFactor and we should make GraphicsLayer not know about scale.

Can you explain this a bit more - I don't really understand the comment. What I do know is that it's beneficial for the graphics context to know the scale factor.
Comment 15 Dongseong Hwang 2013-01-12 19:39:45 PST
(In reply to comment #14)
> (In reply to comment #12)
> 
> > In addition, if we really want transform includes scale info, we should make transform include deviceRatioFactor and we should make GraphicsLayer not know about scale.
> 
> Can you explain this a bit more - I don't really understand the comment. What I do know is that it's beneficial for the graphics context to know the scale factor.

That's good question! I should have explained well. Sorry for lazy comment.

GraphicsLayer should use final effective scale to draw contents on the device screen.
The effective scale is page scale * device scale.

Current WebCore slightly apply page scale to local transform of GraphicsLayer. But RenderLayerBacking notifies page scale and device scale to GraphicsLayer already. I think it is a bit confused that local transform include page scale. We must remember page scale and local transform are not orthogonal.
And if we really want local transform to include scale info, local transform should include device scale also.

I prefer local transform not to include scale info.

In addition, Page::pageScale() method causes style recalc. IMO it is only for applying page scale to local transform. If it is true, the style recalc can be avoidable as we don't change local transform.

And I don't fully understand why Page::pageScale() causes layout yet. Now, changing page scale causes layout after this patch. I'm not sure whether it is acceptable or not.
Comment 16 Kenneth Rohde Christiansen 2013-01-18 03:56:38 PST
Are you going to update this patch?
Comment 17 Dongseong Hwang 2013-01-23 03:42:38 PST
Created attachment 184193 [details]
Patch
Comment 18 Dongseong Hwang 2013-01-23 03:47:04 PST
After this patch, we need to reset the results of many layout tests because we now change a page scale factor in WebCore.

I tested pinch zoom using QtMiniBrowser and device scale factor using -r option of EFLMiniBrowser. I think this patch is fine.

However, I'm sure this patch makes new bugs that are not identified yet. We need to fix them gradually.
Comment 19 Kenneth Rohde Christiansen 2013-01-23 04:53:45 PST
Comment on attachment 184193 [details]
Patch

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

> Source/WebKit2/ChangeLog:11
> +        Currently, PageViewportController sends a page scale factor to Coordinated
> +        Graphics System regardless of the page scale factor in WebCore. This patch makes
> +        Coordinated Graphics System use the page scale factor in WebCore to match other
> +        ports.

Exactly what we want, to be more inline with the other ports

> Source/WebKit2/ChangeLog:15
> +        WebCore is changed, CoordinatedGraphicsLayer can be notified via

gets? instead of can be

> Source/WebKit2/ChangeLog:23
> +        We set true to applyDeviceScaleFactorInCompositor and
> +        ApplyPageScaleFactorInCompositor in Settings like chromium, because
> +        TiledBackingStore that is a backing store of each GraphicsLayer applies the
> +        scale to our raster graphics engines instead of applying the scale to the local
> +        transform of each render object.

that sounds right. Then you dont need to relayout after changing scale

> Source/WebKit2/ChangeLog:43
> +        * UIProcess/API/efl/EwkViewImpl.cpp:
> +        (EwkViewImpl::EwkViewImpl):
> +        * UIProcess/API/qt/qquickwebview.cpp:
> +        (QQuickWebViewPrivate::initialize):
> +        (QQuickWebViewLegacyPrivate::updateViewportSize):
> +        * UIProcess/API/qt/raw/qrawwebview.cpp:
> +        (QRawWebView::setSize):
> +        * UIProcess/CoordinatedGraphics/CoordinatedLayerTreeHostProxy.cpp:
> +        (WebKit::CoordinatedLayerTreeHostProxy::CoordinatedLayerTreeHostProxy):
> +        (WebKit::CoordinatedLayerTreeHostProxy::setVisibleContentsRect):
> +        * UIProcess/CoordinatedGraphics/CoordinatedLayerTreeHostProxy.h:
> +        (CoordinatedLayerTreeHostProxy):
> +        * UIProcess/DrawingAreaProxy.h:
> +        (WebKit::DrawingAreaProxy::setVisibleContentsRect):
> +        * UIProcess/DrawingAreaProxyImpl.cpp:

a bit of detailed info here would be useful

> Source/WebKit2/UIProcess/PageViewportController.cpp:202
> +        // FIXME : the contents position does not affect anything when delegates scrolling is enabled.
> +        m_webPageProxy->scalePage(m_pageScaleFactor, roundedIntPoint(m_contentsPosition));

Why is that a FIXME? Should it?

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1218
>  
> +#if USE(ACCELERATED_COMPOSITING)
> +    if (m_drawingArea->layerTreeHost())
> +        m_drawingArea->layerTreeHost()->deviceOrPageScaleFactorChanged();
> +#endif

smfr should probably look at this change

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1254
> +#if USE(ACCELERATED_COMPOSITING)
> +    if (m_drawingArea->layerTreeHost())
> +        m_drawingArea->layerTreeHost()->deviceOrPageScaleFactorChanged();
> +#endif

diddo

> Source/WebKit2/WebProcess/WebPage/mac/LayerTreeHostMac.mm:165
> -void LayerTreeHostMac::deviceScaleFactorDidChange()
> +void LayerTreeHostCA::deviceOrPageScaleFactorChanged()

diddo
Comment 20 EFL EWS Bot 2013-01-23 09:55:22 PST
Comment on attachment 184193 [details]
Patch

Attachment 184193 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/16080219
Comment 21 Early Warning System Bot 2013-01-23 10:24:36 PST
Comment on attachment 184193 [details]
Patch

Attachment 184193 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/16063687
Comment 22 Build Bot 2013-01-23 10:34:13 PST
Comment on attachment 184193 [details]
Patch

Attachment 184193 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/16065585
Comment 23 Build Bot 2013-01-23 12:18:17 PST
Comment on attachment 184193 [details]
Patch

Attachment 184193 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16067468
Comment 24 Dongseong Hwang 2013-01-23 14:52:48 PST
(In reply to comment #19)
> (From update of attachment 184193 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=184193&action=review
Thank you for review!

> > Source/WebKit2/ChangeLog:15
> > +        WebCore is changed, CoordinatedGraphicsLayer can be notified via
> 
> gets? instead of can be

yes

> > Source/WebKit2/ChangeLog:23
> > +        We set true to applyDeviceScaleFactorInCompositor and
> > +        ApplyPageScaleFactorInCompositor in Settings like chromium, because
> > +        TiledBackingStore that is a backing store of each GraphicsLayer applies the
> > +        scale to our raster graphics engines instead of applying the scale to the local
> > +        transform of each render object.
> 
> that sounds right. Then you dont need to relayout after changing scale

But we need to change Page::setPageScaleFactor to no relayout. I think putting "if (ApplyPageScaleFactorInCompositor)" in relevant place.

> > Source/WebKit2/ChangeLog:43
> > +        (WebKit::DrawingAreaProxy::setVisibleContentsRect):
> > +        * UIProcess/DrawingAreaProxyImpl.cpp:
> 
> a bit of detailed info here would be useful

I'll update.

> 
> > Source/WebKit2/UIProcess/PageViewportController.cpp:202
> > +        // FIXME : the contents position does not affect anything when delegates scrolling is enabled.
> > +        m_webPageProxy->scalePage(m_pageScaleFactor, roundedIntPoint(m_contentsPosition));
> 
> Why is that a FIXME? Should it?

I'll recomment without FIXME.
The comment is a bit irrelevant. What I really want to say is that Page::setPageScaleFactor is somewhat weird. I don't understand we really need updateLayout in  Page::setPageScaleFactor when delegatesScrolling is enabled.

void Page::setPageScaleFactor(float scale, const IntPoint& origin)
{
    ...
    if (scale == m_pageScaleFactor) {
        if (view && (view->scrollPosition() != origin || view->delegatesScrolling())) {
            document->updateLayoutIgnorePendingStylesheets();
            view->setScrollPosition(origin);
        }
        return;
    }
    ...
}

> > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1218
> >  
> > +#if USE(ACCELERATED_COMPOSITING)
> > +    if (m_drawingArea->layerTreeHost())
> > +        m_drawingArea->layerTreeHost()->deviceOrPageScaleFactorChanged();
> > +#endif
> 
> smfr should probably look at this change

I'll separate this part in another bug so that smfr review conveniently :)
Comment 25 Dongseong Hwang 2013-01-23 17:00:35 PST
Build fail due to Bug 107742.
I fixed in Bug 107760
Comment 26 Dongseong Hwang 2013-01-24 01:44:08 PST
Created attachment 184438 [details]
Patch
Comment 27 Early Warning System Bot 2013-01-24 01:53:34 PST
Comment on attachment 184438 [details]
Patch

Attachment 184438 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/16076681
Comment 28 Dongseong Hwang 2013-01-24 01:58:02 PST
Created attachment 184445 [details]
Patch
Comment 29 Dongseong Hwang 2013-01-30 02:47:50 PST
Created attachment 185449 [details]
Patch
Comment 30 Kenneth Rohde Christiansen 2013-01-30 02:55:10 PST
Comment on attachment 185449 [details]
Patch

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

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1276
>      m_page->settings()->setFixedPositionCreatesStackingContext(fixed);
> +    m_page->settings()->setApplyDeviceScaleFactorInCompositor(fixed);
> +    m_page->settings()->setApplyPageScaleFactorInCompositor(fixed);
>  #endif

I tihnk we are already setting these when creating our EwkView (or we should be)
Comment 31 Dongseong Hwang 2013-01-30 02:57:56 PST
(In reply to comment #30)
> (From update of attachment 185449 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=185449&action=review
> 
> > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1276
> >      m_page->settings()->setFixedPositionCreatesStackingContext(fixed);
> > +    m_page->settings()->setApplyDeviceScaleFactorInCompositor(fixed);
> > +    m_page->settings()->setApplyPageScaleFactorInCompositor(fixed);
> >  #endif
> 
> I tihnk we are already setting these when creating our EwkView (or we should be)

we can not set in UI process because these api are not open via WebPreference.
Comment 32 Kenneth Rohde Christiansen 2013-01-30 03:03:54 PST
LGTM, please have a WebKit2 owner review
Comment 33 Noam Rosenthal 2013-01-30 03:08:25 PST
(In reply to comment #32)
> LGTM, please have a WebKit2 owner review
LGTM as well. A step forward towards having less home-brew code around...
Comment 34 Simon Fraser (smfr) 2013-01-30 13:46:06 PST
Comment on attachment 185449 [details]
Patch

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

> Source/WebKit2/UIProcess/PageViewportController.cpp:201
> +        // FIXME : Page::setPageScaleFactor performs layout even if the scale is not changed.

Does that layout actually do any work though?
Comment 35 Dongseong Hwang 2013-01-30 13:52:26 PST
(In reply to comment #34)
> (From update of attachment 185449 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=185449&action=review
> 
> > Source/WebKit2/UIProcess/PageViewportController.cpp:201
> > +        // FIXME : Page::setPageScaleFactor performs layout even if the scale is not changed.
> 
> Does that layout actually do any work though?

Thank you for review!

after r141053 (Bug 107424) the comment became stale.
I mean that there is no layout with applyPageScaleInCompositor.

I'll get rid of the comment and the cq+.
Comment 36 Dongseong Hwang 2013-01-30 13:55:13 PST
Created attachment 185556 [details]
Patch for landing
Comment 37 Dongseong Hwang 2013-01-30 14:10:23 PST
Thank mikhail for cq+ :)
Comment 38 WebKit Review Bot 2013-01-30 14:29:25 PST
Comment on attachment 185556 [details]
Patch for landing

Clearing flags on attachment: 185556

Committed r141310: <http://trac.webkit.org/changeset/141310>
Comment 39 WebKit Review Bot 2013-01-30 14:29:34 PST
All reviewed patches have been landed.  Closing bug.