Bug 60743 - Replace docTop/Right/Bottom/Left/Width/Height with documentRect
Summary: Replace docTop/Right/Bottom/Left/Width/Height with documentRect
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Emil A Eklund
URL:
Keywords:
Depends on:
Blocks: 60318
  Show dependency treegraph
 
Reported: 2011-05-12 18:39 PDT by Emil A Eklund
Modified: 2011-05-16 16:34 PDT (History)
4 users (show)

See Also:


Attachments
Patch (8.93 KB, patch)
2011-05-12 19:40 PDT, Emil A Eklund
no flags Details | Formatted Diff | Diff
Patch (11.33 KB, patch)
2011-05-16 11:17 PDT, Emil A Eklund
no flags Details | Formatted Diff | Diff
Patch for landing (11.33 KB, patch)
2011-05-16 15:18 PDT, Emil A Eklund
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Emil A Eklund 2011-05-12 18:39:45 PDT
RenderView has six methods for getting the document location and size, namely docTop, docRight, docBottom, docLeft, docWidth and docHeight. The current implementation of documentRect calls four of those, in effect computing the rect four times.
Comment 1 Emil A Eklund 2011-05-12 19:40:36 PDT
Created attachment 93392 [details]
Patch
Comment 2 WebKit Review Bot 2011-05-12 20:11:26 PDT
Comment on attachment 93392 [details]
Patch

Attachment 93392 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/8689911
Comment 3 Eric Seidel (no email) 2011-05-16 11:05:37 PDT
Comment on attachment 93392 [details]
Patch

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

Fantastic!

> Source/WebCore/page/FrameView.cpp:491
> +    ScrollView::setScrollOrigin(IntPoint(-rect.x(), -rect.y()), !m_frame->document()->printing(), size == contentsSize());

Is there a helper for producing -x, -y from x, y?  What about just IntSize() - rect.location()?  I'm not sure that's more clear though, so that may be a bad idea.... just brainstorming. :)

> Source/WebCore/rendering/RenderLayerCompositor.cpp:1143
> +        m_rootPlatformLayer->setSize(FloatSize(documentRect.size()));

Do you need the explicit FloatSize conversion here?  I doubt it.
Comment 4 Simon Fraser (smfr) 2011-05-16 11:09:07 PDT
The Mac failure suggests that you need to fix the exports and WebKit.
Comment 5 Emil A Eklund 2011-05-16 11:10:21 PDT
> The Mac failure suggests that you need to fix the exports and WebKit.
Yeah, working on that now. Thanks
Comment 6 Emil A Eklund 2011-05-16 11:17:42 PDT
Created attachment 93673 [details]
Patch
Comment 7 Eric Seidel (no email) 2011-05-16 11:18:56 PDT
Comment on attachment 93673 [details]
Patch

LGTM.
Comment 8 Emil A Eklund 2011-05-16 11:19:34 PDT
> Is there a helper for producing -x, -y from x, y? 
No but I could add one, it's something we do in quite a few places so a helper might be useful.


> Do you need the explicit FloatSize conversion here?  I doubt it.
Turns out I don't, good catch.
Comment 9 Emil A Eklund 2011-05-16 15:18:37 PDT
Created attachment 93704 [details]
Patch for landing
Comment 10 WebKit Commit Bot 2011-05-16 15:51:11 PDT
Comment on attachment 93704 [details]
Patch for landing

Rejecting attachment 93704 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-8', 'apply-..." exit_code: 1

Last 500 characters of output:
autoinstalled/mechanize/_urllib2_fork.py", line 332, in _call_chain
    result = func(*args)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_urllib2_fork.py", line 1170, in https_open
    return self.do_open(conn_factory, req)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_urllib2_fork.py", line 1118, in do_open
    raise URLError(err)
urllib2.URLError: <urlopen error [Errno 60] Operation timed out>

Full output: http://queues.webkit.org/results/8707083
Comment 11 WebKit Commit Bot 2011-05-16 16:34:03 PDT
Comment on attachment 93704 [details]
Patch for landing

Clearing flags on attachment: 93704

Committed r86632: <http://trac.webkit.org/changeset/86632>
Comment 12 WebKit Commit Bot 2011-05-16 16:34:08 PDT
All reviewed patches have been landed.  Closing bug.