Replace docTop/Right/Bottom/Left/Width/Height with documentRect

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Emil A Eklund]

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.

[Emil A Eklund]

Created attachment 93392 [details]
Patch

[WebKit Review Bot]

Comment on attachment 93392 [details]
Patch

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

[Eric Seidel (no email)]

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.

[Simon Fraser (smfr)]

The Mac failure suggests that you need to fix the exports and WebKit.

[Emil A Eklund]

> The Mac failure suggests that you need to fix the exports and WebKit.
Yeah, working on that now. Thanks

[Emil A Eklund]

Created attachment 93673 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 93673 [details]
Patch

LGTM.

[Emil A Eklund]

> 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.

[Emil A Eklund]

Created attachment 93704 [details]
Patch for landing

[WebKit Commit Bot]

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

[WebKit Commit Bot]

Comment on attachment 93704 [details]
Patch for landing

Clearing flags on attachment: 93704

Committed r86632: <http://trac.webkit.org/changeset/86632>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.