Bug 170913

Summary: Plumb all four obscured insets to WebCore, instead of just top/left
Product: WebKit Reporter: Tim Horton <thorton>
Component: New BugsAssignee: Tim Horton <thorton>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, bdakin, buildbot, cdumez, commit-queue, dbates, japhet, mitz, ryanhaddad, sam, simon.fraser, wenson_hsieh
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Tim Horton
Reported 2017-04-17 13:52:25 PDT
Plumb all four obscured insets to WebCore, instead of just top/left
Attachments
Patch (41.91 KB, patch)
2017-04-17 13:52 PDT, Tim Horton 		no flags
DetailsFormatted DiffDiff
Patch (33.75 KB, patch)
2017-04-17 15:33 PDT, Tim Horton 		no flags
DetailsFormatted DiffDiff
Patch (33.96 KB, patch)
2017-04-17 16:32 PDT, Tim Horton 		no flags
DetailsFormatted DiffDiff
Patch (33.94 KB, patch)
2017-04-17 18:02 PDT, Tim Horton 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Tim Horton
Comment 1 2017-04-17 13:52:59 PDT
Created attachment 307293 [details] Patch
Tim Horton
Comment 2 2017-04-17 13:58:57 PDT
Maybe this should use FloatBoxExtent?
Simon Fraser (smfr)
Comment 3 2017-04-17 13:59:07 PDT
Comment on attachment 307293 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=307293&action=review > Source/WebCore/platform/graphics/FloatRectInsets.h:32 > +class FloatRectInsets { Can this be a BoxExtent<float>?
Build Bot
Comment 4 2017-04-17 13:59:18 PDT
Attachment 307293 [details] did not pass style-queue: ERROR: Source/WebKit2/Shared/VisibleContentRectUpdateInfo.h:57: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] Total errors found: 1 in 25 files If any of these errors are false positives, please file a bug against check-webkit-style.
Tim Horton
Comment 5 2017-04-17 13:59:39 PDT
lol, I beat you by 10 seconds.
Wenson Hsieh
Comment 6 2017-04-17 14:03:32 PDT
Comment on attachment 307293 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=307293&action=review > Source/WebCore/ChangeLog:4 > + https://bugs.webkit.org/show_bug.cgi?id=170913 Maybe add a "(Work towards )<rdar://problem/________>" here? > Source/WebKit2/ChangeLog:4 > + https://bugs.webkit.org/show_bug.cgi?id=170913 Maybe add a "(Work towards )<rdar://problem/________>" here? > Source/WebCore/platform/graphics/FloatRectInsets.cpp:33 > +TextStream& operator<<(TextStream& ts, const FloatRectInsets &insets) "FloatRectInsets &" => "FloatRectInsets& " > Source/WebCore/platform/graphics/FloatRectInsets.h:58 > + Should this know how to convert to UIEdgeInsets via operator UIEdgeInsets()? (like how FloatRect can convert to a CGRect).
Tim Horton
Comment 7 2017-04-17 15:33:53 PDT
Created attachment 307306 [details] Patch
Build Bot
Comment 8 2017-04-17 15:36:02 PDT
Attachment 307306 [details] did not pass style-queue: ERROR: Source/WebKit2/Shared/VisibleContentRectUpdateInfo.h:57: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] Total errors found: 1 in 25 files If any of these errors are false positives, please file a bug against check-webkit-style.
Tim Horton
Comment 9 2017-04-17 16:32:16 PDT
Created attachment 307318 [details] Patch
Build Bot
Comment 10 2017-04-17 16:37:51 PDT
Attachment 307318 [details] did not pass style-queue: ERROR: Source/WebKit2/Shared/VisibleContentRectUpdateInfo.h:57: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] Total errors found: 1 in 25 files If any of these errors are false positives, please file a bug against check-webkit-style.
Tim Horton
Comment 11 2017-04-17 18:02:40 PDT
Created attachment 307327 [details] Patch
Build Bot
Comment 12 2017-04-17 18:04:49 PDT
Attachment 307327 [details] did not pass style-queue: ERROR: Source/WebKit2/Shared/VisibleContentRectUpdateInfo.h:57: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] Total errors found: 1 in 25 files If any of these errors are false positives, please file a bug against check-webkit-style.
WebKit Commit Bot
Comment 13 2017-04-17 19:37:22 PDT
Comment on attachment 307327 [details] Patch Clearing flags on attachment: 307327 Committed r215446: <http://trac.webkit.org/changeset/215446>
WebKit Commit Bot
Comment 14 2017-04-17 19:37:24 PDT
All reviewed patches have been landed. Closing bug.
Ryan Haddad
Comment 15 2017-04-18 09:36:33 PDT
This change broke the iOS debug build: https://build.webkit.org/builders/Apple%20iOS%2010%20Simulator%20Debug%20%28Build%29/builds/843
Sam Weinig
Comment 16 2017-04-18 18:49:51 PDT
Is this for Mac and iOS, or just iOS?
Tim Horton
Comment 17 2017-04-18 19:03:49 PDT
(In reply to Sam Weinig from comment #16) > Is this for Mac and iOS, or just iOS? Just iOS. Mac sends even less! Only top. But we should use the same mechanism (the WebCore parts are not iOS specific) in the future.
Sam Weinig
Comment 18 2017-04-18 19:16:43 PDT
(In reply to Tim Horton from comment #17) > (In reply to Sam Weinig from comment #16) > > Is this for Mac and iOS, or just iOS? > > Just iOS. Mac sends even less! Only top. But we should use the same > mechanism (the WebCore parts are not iOS specific) in the future. Yay-boo.
Sam Weinig
Comment 19 2017-04-18 19:17:37 PDT
Also, was this really not testable? You even updated TextStream to support it :|
Tim Horton
Comment 20 2017-04-18 19:29:01 PDT
Settle down and wait for the next patch :)
Sam Weinig
Comment 21 2017-04-18 19:37:54 PDT
(In reply to Tim Horton from comment #20) > Settle down and wait for the next patch :) Oh, so now I have to trust that "tests are coming"? I've been spun that tale before young man.
Note You need to log in before you can comment on or make changes to this bug.