Bug 114469 - Add contentAnchor to WKView
Summary: Add contentAnchor to WKView
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gavin Barraclough
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-11 15:37 PDT by Gavin Barraclough
Modified: 2013-04-12 18:38 PDT (History)
10 users (show)

See Also:


Attachments
Patch (15.34 KB, patch)
2013-04-11 16:13 PDT, Gavin Barraclough
simon.fraser: review-
eflews.bot: commit-queue-
Details | Formatted Diff | Diff
Reset layer origin on next setFrameSize with updates enabled (16.85 KB, patch)
2013-04-12 15:21 PDT, Gavin Barraclough
simon.fraser: review+
eflews.bot: commit-queue-
Details | Formatted Diff | Diff
use inline functions, more qt/efl build fixes (19.00 KB, patch)
2013-04-12 15:56 PDT, Gavin Barraclough
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gavin Barraclough 2013-04-11 15:37:52 PDT
Currently if you call setFrameSize: while frame size updates are disabled, or if the geometry update times out, then the previously rendered page content will be rendered anchored to the top left corner of the frame.

This is appropriate if the frame is being resized from the bottom right corner.  In order to achieve a more desirable appearance if the frame is being resized from another corner we should allow the corner at which the content anchors to be specified.
Comment 1 Gavin Barraclough 2013-04-11 16:13:25 PDT
Created attachment 197693 [details]
Patch
Comment 2 WebKit Commit Bot 2013-04-11 16:15:19 PDT
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 3 EFL EWS Bot 2013-04-11 16:16:51 PDT
Comment on attachment 197693 [details]
Patch

Attachment 197693 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/65015
Comment 4 Early Warning System Bot 2013-04-11 16:28:54 PDT
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 5 Simon Fraser (smfr) 2013-04-11 18:05:12 PDT
Comment on attachment 197693 [details]
Patch

Gavin and I discussed this and he's going to make a few more changes.
Comment 6 Gavin Barraclough 2013-04-12 15:21:21 PDT
Created attachment 197895 [details]
Reset layer origin on next setFrameSize with updates enabled
Comment 7 WebKit Commit Bot 2013-04-12 15:23:41 PDT
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 8 EFL EWS Bot 2013-04-12 15:29:32 PDT
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 9 Simon Fraser (smfr) 2013-04-12 15:38:27 PDT
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 10 Early Warning System Bot 2013-04-12 15:51:25 PDT
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
Comment 11 Gavin Barraclough 2013-04-12 15:56:56 PDT
Created attachment 197899 [details]
use inline functions, more qt/efl build fixes
Comment 12 WebKit Commit Bot 2013-04-12 15:59:20 PDT
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 13 Gavin Barraclough 2013-04-12 16:26:17 PDT
Fixed in http://trac.webkit.org/changeset/148311
Comment 14 Darin Adler 2013-04-12 16:56:04 PDT
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.
Comment 15 Ryosuke Niwa 2013-04-12 17:21:09 PDT
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) {
~~~~~~~ ^
Comment 16 Gavin Barraclough 2013-04-12 18:38:26 PDT
build fixes:
Changeset [148322] by barraclough@apple.com
Changeset [148324] by barraclough@apple.com
Changeset [148330] by jberlin@webkit.org