Bug 170913 - Plumb all four obscured insets to WebCore, instead of just top/left
Summary: Plumb all four obscured insets to WebCore, instead of just top/left
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-17 13:52 PDT by Tim Horton
Modified: 2017-04-18 19:37 PDT (History)
12 users (show)

See Also:


Attachments
Patch (41.91 KB, patch)
2017-04-17 13:52 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (33.75 KB, patch)
2017-04-17 15:33 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (33.96 KB, patch)
2017-04-17 16:32 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (33.94 KB, patch)
2017-04-17 18:02 PDT, Tim Horton
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2017-04-17 13:52:25 PDT
Plumb all four obscured insets to WebCore, instead of just top/left
Comment 1 Tim Horton 2017-04-17 13:52:59 PDT
Created attachment 307293 [details]
Patch
Comment 2 Tim Horton 2017-04-17 13:58:57 PDT
Maybe this should use FloatBoxExtent?
Comment 3 Simon Fraser (smfr) 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>?
Comment 4 Build Bot 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.
Comment 5 Tim Horton 2017-04-17 13:59:39 PDT
lol, I beat you by 10 seconds.
Comment 6 Wenson Hsieh 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).
Comment 7 Tim Horton 2017-04-17 15:33:53 PDT
Created attachment 307306 [details]
Patch
Comment 8 Build Bot 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.
Comment 9 Tim Horton 2017-04-17 16:32:16 PDT
Created attachment 307318 [details]
Patch
Comment 10 Build Bot 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.
Comment 11 Tim Horton 2017-04-17 18:02:40 PDT
Created attachment 307327 [details]
Patch
Comment 12 Build Bot 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.
Comment 13 WebKit Commit Bot 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>
Comment 14 WebKit Commit Bot 2017-04-17 19:37:24 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 Ryan Haddad 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
Comment 16 Sam Weinig 2017-04-18 18:49:51 PDT
Is this for Mac and iOS, or just iOS?
Comment 17 Tim Horton 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.
Comment 18 Sam Weinig 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.
Comment 19 Sam Weinig 2017-04-18 19:17:37 PDT
Also, was this really not testable? You even updated TextStream to support it :|
Comment 20 Tim Horton 2017-04-18 19:29:01 PDT
Settle down and wait for the next patch :)
Comment 21 Sam Weinig 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.