Summary: | Add contentAnchor to WKView | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gavin Barraclough <barraclough> | ||||||||
Component: | WebKit2 | Assignee: | Gavin Barraclough <barraclough> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abecsi, cmarcelo, commit-queue, gyuyoung.kim, menard, rakuco, rniwa, simon.fraser, thorton, webkit-ews | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Gavin Barraclough
2013-04-11 15:37:52 PDT
Created attachment 197693 [details]
Patch
Attachment 197693 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/UIProcess/API/mac/WKView.mm', u'Source/WebKit2/UIProcess/API/mac/WKViewPrivate.h', u'Source/WebKit2/UIProcess/DrawingAreaProxy.cpp', u'Source/WebKit2/UIProcess/DrawingAreaProxy.h', u'Source/WebKit2/UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.h', u'Source/WebKit2/UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.mm', u'Source/WebKit2/WebProcess/WebPage/DrawingArea.h', u'Source/WebKit2/WebProcess/WebPage/DrawingArea.messages.in', u'Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h', u'Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm']" exit_code: 1
Source/WebKit2/UIProcess/API/mac/WKViewPrivate.h:28: Place brace on its own line for function definitions. [whitespace/braces] [4]
Total errors found: 1 in 11 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 197693 [details] Patch Attachment 197693 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/65015 Comment on attachment 197693 [details] Patch Attachment 197693 [details] did not pass qt-wk2-ews (qt): Output: http://webkit-queues.appspot.com/results/101015 Comment on attachment 197693 [details]
Patch
Gavin and I discussed this and he's going to make a few more changes.
Created attachment 197895 [details]
Reset layer origin on next setFrameSize with updates enabled
Attachment 197895 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/UIProcess/API/mac/WKView.mm', u'Source/WebKit2/UIProcess/API/mac/WKViewPrivate.h', u'Source/WebKit2/UIProcess/API/qt/raw/qrawwebview.cpp', u'Source/WebKit2/UIProcess/DrawingAreaProxy.cpp', u'Source/WebKit2/UIProcess/DrawingAreaProxy.h', u'Source/WebKit2/UIProcess/PageViewportController.cpp', u'Source/WebKit2/UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.h', u'Source/WebKit2/UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.mm', u'Source/WebKit2/WebProcess/WebPage/DrawingArea.h', u'Source/WebKit2/WebProcess/WebPage/DrawingArea.messages.in', u'Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h', u'Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm']" exit_code: 1
Source/WebKit2/UIProcess/API/mac/WKViewPrivate.h:28: Place brace on its own line for function definitions. [whitespace/braces] [4]
Total errors found: 1 in 13 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 197895 [details] Reset layer origin on next setFrameSize with updates enabled Attachment 197895 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/48266 Comment on attachment 197895 [details] Reset layer origin on next setFrameSize with updates enabled View in context: https://bugs.webkit.org/attachment.cgi?id=197895&action=review > Source/WebKit2/UIProcess/API/mac/WKView.mm:98 > +#define WKContentAnchorRight(x) (!!((x) & WKContentAnchorTopRight)) > +#define WKContentAnchorBottom(x) (!!((x) & WKContentAnchorBottomLeft)) Wouldn't inline functions be preferable in C++? bool WKContentAnchorRight(unsigned x) { return x & WKContentAnchorTopRight; } But WKContentAnchor* are not bit masks, so this is vulnerable to someone re-ordering WKContentAnchor flags. I think an explicit x == WKContentAnchorTopRight || x == WKContentAnchorBottomRight would be better. Comment on attachment 197895 [details] Reset layer origin on next setFrameSize with updates enabled Attachment 197895 [details] did not pass qt-wk2-ews (qt): Output: http://webkit-queues.appspot.com/results/24012 Created attachment 197899 [details]
use inline functions, more qt/efl build fixes
Attachment 197899 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/UIProcess/API/mac/WKView.mm', u'Source/WebKit2/UIProcess/API/mac/WKViewPrivate.h', u'Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp', u'Source/WebKit2/UIProcess/API/qt/raw/qrawwebview.cpp', u'Source/WebKit2/UIProcess/DrawingAreaProxy.cpp', u'Source/WebKit2/UIProcess/DrawingAreaProxy.h', u'Source/WebKit2/UIProcess/PageViewportController.cpp', u'Source/WebKit2/UIProcess/efl/WebView.cpp', u'Source/WebKit2/UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.h', u'Source/WebKit2/UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.mm', u'Source/WebKit2/WebProcess/WebPage/DrawingArea.h', u'Source/WebKit2/WebProcess/WebPage/DrawingArea.messages.in', u'Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h', u'Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm']" exit_code: 1
Source/WebKit2/UIProcess/API/mac/WKViewPrivate.h:28: Place brace on its own line for function definitions. [whitespace/braces] [4]
Total errors found: 1 in 15 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 197895 [details] Reset layer origin on next setFrameSize with updates enabled View in context: https://bugs.webkit.org/attachment.cgi?id=197895&action=review >> Source/WebKit2/UIProcess/API/mac/WKView.mm:98 >> +#define WKContentAnchorBottom(x) (!!((x) & WKContentAnchorBottomLeft)) > > Wouldn't inline functions be preferable in C++? > > bool WKContentAnchorRight(unsigned x) { return x & WKContentAnchorTopRight; } > > But WKContentAnchor* are not bit masks, so this is vulnerable to someone re-ordering WKContentAnchor flags. I think an explicit x == WKContentAnchorTopRight || x == WKContentAnchorBottomRight would be better. You can use COMPILE_ASSERT to make a safe version that still uses & for efficiency. There's another build failure from this patch: http://build.webkit.org/builders/Apple%20Lion%20Debug%20%28Build%29/builds/15014 In file included from /Volumes/Data/slave/lion-debug/build/Source/WebKit2/UIProcess/API/mac/PDFViewController.mm:31: /Volumes/Data/slave/lion-debug/build/Source/WebKit2/UIProcess/API/mac/WKViewPrivate.h:28:29: error: C++ requires a type specifier for all declarations typedef NS_ENUM(NSUInteger, WKContentAnchor) { ^~~~~~~~~~~~~~~ /Volumes/Data/slave/lion-debug/build/Source/WebKit2/UIProcess/API/mac/WKViewPrivate.h:28:9: error: C++ requires a type specifier for all declarations typedef NS_ENUM(NSUInteger, WKContentAnchor) { ~~~~~~~ ^ build fixes: Changeset [148322] by barraclough@apple.com Changeset [148324] by barraclough@apple.com Changeset [148330] by jberlin@webkit.org |