Bug 39366 – Move view-related functions from Frame to FrameView

• Component: New Bugs
• Status: RESOLVED FIXED

[Darin Adler]

Move view-related functions from Frame to FrameView

[Darin Adler]

Created attachment 56500 [details]
Patch

[Early Warning System Bot]

Attachment 56500 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/2265334

[WebKit Review Bot]

Attachment 56500 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/2284350

[Eric Seidel (no email)]

Comment on attachment 56500 [details]
Patch

WebCore/css/CSSStyleSelector.cpp:4043
 +                          multiplier *= view->textZoomFactor();
much cleaner, thanks.

WebCore/dom/MouseRelatedEvent.cpp:137
 +      float zoomFactor = pageZoomFactor(this);
MUCH cleaner.  Thanks again!

WebCore/html/HTMLBodyElement.cpp:268
 +      document->updateLayoutIgnorePendingStylesheets();
This is a behavior change.  We didn't use to bother to layout when the view didn't exist.  Is this intentional?

WebCore/page/FrameView.h:297
 +      int m_borderX;
This is destined to be an IntSize in a later patch. :)

WebKit/efl/ewk/ewk_frame.cpp:974
 +          return -1;
-1?

LGTM.

[Darin Adler]

(In reply to comment #4)
> WebCore/html/HTMLBodyElement.cpp:268
>  +      document->updateLayoutIgnorePendingStylesheets();
> This is a behavior change.  We didn't use to bother to layout when the view didn't exist.  Is this intentional?

It was intentional. I think it's safer to call the layout update function before looking at anything, including the view's existence.

> WebCore/page/FrameView.h:297
>  +      int m_borderX;
> This is destined to be an IntSize in a later patch. :)

I thought the same thing.

> WebKit/efl/ewk/ewk_frame.cpp:974
>  +          return -1;
> -1?

Just following the design of this EFL function. They made it return -1 when there is no frame. I don't understand it -- that's the downside of each port having its own API. Different quirks.

[Darin Adler]

Committed r60104: <http://trac.webkit.org/changeset/60104>

[WebKit Review Bot]

http://trac.webkit.org/changeset/60104 might have broken Qt Linux ARMv5 Release
The following changes are on the blame list:
http://trac.webkit.org/changeset/60104
http://trac.webkit.org/changeset/60101
http://trac.webkit.org/changeset/60102
http://trac.webkit.org/changeset/60103

[Julie Parent]

I think this change broke Chromium's compile on all platforms too.

[Darin Adler]

(In reply to comment #8)
> I think this change broke Chromium's compile on all platforms too.

Has it been fixed yet? Everything looks OK on <http://build.webkit.org/waterfall> at this point, so maybe I fixed it or someone else did. The fix should be simple if it hasn't been done yet.

[Julie Parent]

Yup, tc fixed it last night in r60111.  Thanks for following up!