We should provide a way to create a view from a WKView which can borrow the WKView's live contents and render them in a second view. We can then provide optimizations, e.g. painting at a lower resolution, reusing existing tiles, etc. given knowledge that we're thumbnailing. The thumbnail view will not forward events, and WKView will disable event handling while thumbnailed. Also, changes to the size of the thumbnail view will not affect the layout of the content. <rdar://problem/15669655>
Created attachment 224246 [details] patch an initial cut. currently we don't re-use existing tiles when bringing in unparented views, because we can't tell if they've been purged without parenting them, and don't want to cause a sudden surge in backing store if many thumbnail views are made from unparented views at once, but this can be revisited.
Attachment 224246 [details] did not pass style-queue: ERROR: Source/WebKit2/UIProcess/API/mac/WKViewInternal.h:108: Missing spaces around = [whitespace/operators] [4] ERROR: Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h:41: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 2 in 17 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 224247 [details] rebase+style
Attachment 224247 [details] did not pass style-queue: ERROR: Source/WebKit2/UIProcess/API/mac/WKViewInternal.h:108: Missing spaces around = [whitespace/operators] [4] Total errors found: 1 in 17 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 224251 [details] fix ML build + offline review comments
Attachment 224251 [details] did not pass style-queue: ERROR: Source/WebKit2/UIProcess/API/mac/WKViewInternal.h:108: Missing spaces around = [whitespace/operators] [4] Total errors found: 1 in 17 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 224251 [details] fix ML build + offline review comments View in context: https://bugs.webkit.org/attachment.cgi?id=224251&action=review > Source/WebKit2/UIProcess/API/Cocoa/WKThumbnailView.h:30 > +#import <Cocoa/Cocoa.h> We shouldn’t import the Core Data headers here. AppKit will do. > Source/WebKit2/UIProcess/API/Cocoa/WKThumbnailView.h:31 > +#import <WebKit2/WKDeclarationSpecifiers.h> I don’t think this is necessary anymore. > Source/WebKit2/UIProcess/API/Cocoa/WKThumbnailView.h:35 > +WK_API_CLASS @interface WKThumbnailView : NSView WK_API_CLASS goes on its own line before the @interface line.
Created attachment 224400 [details] patch
Created attachment 224402 [details] rebase
Attachment 224402 [details] did not pass style-queue: ERROR: Source/WebKit2/UIProcess/API/mac/WKViewInternal.h:108: Missing spaces around = [whitespace/operators] [4] Total errors found: 1 in 17 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 224412 [details] build fix
Attachment 224412 [details] did not pass style-queue: ERROR: Source/WebKit2/UIProcess/API/mac/WKViewInternal.h:108: Missing spaces around = [whitespace/operators] [4] Total errors found: 1 in 17 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 224412 [details] build fix View in context: https://bugs.webkit.org/attachment.cgi?id=224412&action=review > Source/WebKit2/UIProcess/API/Cocoa/WKThumbnailView.h:38 > +- (void)setScale:(double)scale; This should be a property so that it’s possible to get it, not just set it. It should be a CGFloat. > Source/WebKit2/UIProcess/API/Cocoa/WKThumbnailView.mm:46 > +@implementation WKThumbnailView > +{ The brace normally goes on the same line with the class name. > Source/WebKit2/UIProcess/API/Cocoa/WKThumbnailView.mm:49 > + double _thumbnailScale; If scale were a property, you’d get a _scale ivar for free. > Source/WebKit2/UIProcess/API/Cocoa/WKThumbnailView.mm:54 > + bool _originalMayStartMediaWhenInWindow; > + bool _originalSourceViewIsInWindow; > + > + bool _shouldApplyThumbnailScale; Objective-C booleans are normally BOOL. > Source/WebKit2/UIProcess/API/Cocoa/WKThumbnailView.mm:60 > + self = [super initWithFrame:frame]; > + if (self) { Please use the usual early return pattern if (!(self = [super initWithFrame:frame]))\n return nil; > Source/WebKit2/UIProcess/API/Cocoa/WKThumbnailView.mm:114 > + self.layer.sublayers = layer ? @[layer] : @[]; Please add spaces inside the @[]s. > Source/WebKit2/UIProcess/API/Cocoa/WKThumbnailView.mm:122 > + return [self.layer.sublayers objectAtIndex:0]; You can use -firstObject. > Source/WebKit2/UIProcess/API/mac/PageClientImpl.mm:189 > + NSView *activeView = m_wkView._thumbnailView ? m_wkView._thumbnailView : m_wkView; > + NSWindow *activeViewWindow = activeView.window; I’d factor this out into a helper function. > Source/WebKit2/UIProcess/API/mac/WKView.mm:2562 > + return hostView.layer.sublayers[0]; Can use -firstObject. > Source/WebKit2/UIProcess/API/mac/WKViewInternal.h:32 > #import <wtf/Vector.h> > +#import <wtf/RetainPtr.h> R < V > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:295 > + , m_preThumbnailPageScale(1) Terrible name. > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:4190 > +{ Should we ASSERT_ARG(destinationScale, destinationScale > 0) here? Also, I’m not sure what the word destination signifies here. Normally the parameter name in a setter should be the property being set or some abbreviation thereof. > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:4227 > +} This whole business of special-casing 1 seems highly dubious to me. > Source/WebKit2/WebProcess/WebPage/WebPage.h:1100 > + WebCore::IntPoint m_preThumbnailScrollPosition; Also terrible name. > Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:574 > + [m_hostingLayer setSublayers:layer ? [NSArray arrayWithObject:layer] : [NSArray array]]; Can’t use array literals here?
Comment on attachment 224412 [details] build fix View in context: https://bugs.webkit.org/attachment.cgi?id=224412&action=review > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:4191 > + if (destinationScale == m_thumbnailScale) Dan wants some fixmees here (or fixes) about what happens if the page navigates or does layout. >> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:4227 >> +} > > This whole business of special-casing 1 seems highly dubious to me. Yup, we've sorted out that this is unnecessary.
http://trac.webkit.org/changeset/164337