Bug 96026 - Parcel up logic related to sticky positioning into a Constraints class that will later be used for threaded scrolling
Summary: Parcel up logic related to sticky positioning into a Constraints class that w...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-06 14:27 PDT by Simon Fraser (smfr)
Modified: 2012-09-06 16:20 PDT (History)
10 users (show)

See Also:

Attachments
Patch (25.83 KB, patch)
2012-09-06 15:17 PDT, Simon Fraser (smfr) 		no flags Details | Formatted Diff | Diff
Patch (27.28 KB, patch)
2012-09-06 16:03 PDT, Simon Fraser (smfr) 		no flags Details | Formatted Diff | Diff
Patch (27.24 KB, patch)
2012-09-06 16:06 PDT, Simon Fraser (smfr) 		simon.fraser: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2012-09-06 14:27:09 PDT 
Parcel up logic related to sticky positioning into a Constraints class that will later be used for threaded scrolling
Comment 1 Simon Fraser (smfr) 2012-09-06 15:17:26 PDT 
Created attachment 162602 [details]
Patch
Comment 2 James Robinson 2012-09-06 15:42:47 PDT 
Comment on attachment 162602 [details]
Patch

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

Looks pretty good.  Could you add the new files to the other build systems (evil cackle here).  For chromium it's pretty easy, just add them to the big-ass 'webcore_files' list in Source/WebCore/WebCore.gypi

> Source/WebCore/ChangeLog:22
> +        FixedPositionViewportConstraints is not yet used.

If I was writing this, I'd hold off on landing FixedPositionViewportConstraints until it was used and tested. Checked-in-but-dead code has a nasty habit of bitrotting.

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:25509
> +				0F605AED15F94848004DF0C0 /* ScrollingConstraints.h in Headers */,

should these be sorted?
Comment 3 Simon Fraser (smfr) 2012-09-06 15:56:30 PDT 
Comment on attachment 162602 [details]
Patch

r-, need to add the new files to other build systems
Comment 4 Simon Fraser (smfr) 2012-09-06 16:03:53 PDT 
Created attachment 162610 [details]
Patch
Comment 5 James Robinson 2012-09-06 16:04:42 PDT 
Comment on attachment 162610 [details]
Patch

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

> Source/WebCore/CMakeLists.txt:1735
> +    page/scrolling/ScrollingConstraints.cpp
> +    page/scrolling/ScrollingConstraints.cpp

don't need both - looks like convention here is not to list the headers so just nuke one line
Comment 6 James Robinson 2012-09-06 16:05:38 PDT 
Comment on attachment 162610 [details]
Patch

Otherwise looks good, R=me.  I'd probably let the EWS bots finish, or land and if you're in a hurry to land watch out for bustages.
Comment 7 Simon Fraser (smfr) 2012-09-06 16:06:08 PDT 
Created attachment 162611 [details]
Patch
Comment 8 Simon Fraser (smfr) 2012-09-06 16:08:35 PDT 
Comment on attachment 162611 [details]
Patch

Forward James's r+
Comment 9 Simon Fraser (smfr) 2012-09-06 16:20:50 PDT 
http://trac.webkit.org/changeset/127795