Bug 39366 - Move view-related functions from Frame to FrameView
Summary: Move view-related functions from Frame to FrameView
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Darin Adler
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-19 10:22 PDT by Darin Adler
Modified: 2010-05-25 09:49 PDT (History)
7 users (show)

See Also:


Attachments
Patch (48.71 KB, patch)
2010-05-19 10:40 PDT, Darin Adler
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2010-05-19 10:22:20 PDT
Move view-related functions from Frame to FrameView
Comment 1 Darin Adler 2010-05-19 10:40:14 PDT
Created attachment 56500 [details]
Patch
Comment 2 Early Warning System Bot 2010-05-19 10:49:14 PDT
Attachment 56500 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/2265334
Comment 3 WebKit Review Bot 2010-05-19 11:06:08 PDT
Attachment 56500 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/2284350
Comment 4 Eric Seidel (no email) 2010-05-20 00:51:52 PDT
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.
Comment 5 Darin Adler 2010-05-24 17:24:12 PDT
(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.
Comment 6 Darin Adler 2010-05-24 17:33:34 PDT
Committed r60104: <http://trac.webkit.org/changeset/60104>
Comment 7 WebKit Review Bot 2010-05-24 17:41:14 PDT
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
Comment 8 Julie Parent 2010-05-24 18:35:24 PDT
I think this change broke Chromium's compile on all platforms too.
Comment 9 Darin Adler 2010-05-25 09:47:10 PDT
(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.
Comment 10 Julie Parent 2010-05-25 09:49:23 PDT
Yup, tc fixed it last night in r60111.  Thanks for following up!