Bug 82757

Summary: [Chromium] defaultDeviceScaleFactor != 0 should set deviceScaleFactor = defaultDeviceScaleFactor in the absence of a viewport tag for debugging purposes
Product: WebKit Reporter: Fady Samuel <fsamuel>
Component: Layout and RenderingAssignee: Tim Dresser <tdresser>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, aelias, fishd, fsamuel, mbolohan, rjkroege, tdresser, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 70559    
Attachments:
Description Flags
Patch
none
Patch none

Description Fady Samuel 2012-03-30 11:12:38 PDT
[Chromium] defaultDeviceScaleFactor != 0 should set deviceScaleFactor = defaultDeviceScaleFactor in the absense of a viewport tag for debugging purposes
Comment 2 Tim Dresser 2012-04-03 10:26:53 PDT
Created attachment 135356 [details]
Patch
Comment 3 Fady Samuel 2012-04-03 10:40:02 PDT
This looks good to me. Thanks. Added abarth@ for review.
Comment 4 Alexandre Elias 2012-04-03 11:07:30 PDT
Looks like this should be a no-op patch if the setting isn't changed, so it's fine by me.  I'd be curious to know what kind of debugging you have in mind though.
Comment 5 Fady Samuel 2012-04-03 11:20:15 PDT
(In reply to comment #4)
> Looks like this should be a no-op patch if the setting isn't changed, so it's fine by me.  I'd be curious to know what kind of debugging you have in mind though.

Manually passing --default-device-scale-factor=2 in Chromium and browsing.
Comment 6 Darin Fisher (:fishd, Google) 2012-04-05 15:44:27 PDT
Comment on attachment 135356 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=135356&action=review

> Source/WebKit/chromium/tests/WebFrameTest.cpp:34
> +#include "FrameView.h"

please avoid reaching into WebCore if you can help it.

> Source/WebKit/chromium/tests/WebFrameTest.cpp:181
> +    webViewImpl->resize(WebSize(viewportWidth, viewportHeight));

Why do you need to use WebViewImpl here?

> Source/WebKit/chromium/tests/WebFrameTest.cpp:185
> +    webViewImpl->mainFrameImpl()->frameView()->layout();

just call WebWidget::layout()?  WebView inherits from WebWidget
Comment 7 Tim Dresser 2012-04-10 07:20:17 PDT
Created attachment 136452 [details]
Patch
Comment 8 Tim Dresser 2012-04-10 07:21:46 PDT
(In reply to comment #6)
> (From update of attachment 135356 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=135356&action=review
> 
> > Source/WebKit/chromium/tests/WebFrameTest.cpp:34
> > +#include "FrameView.h"
> 
> please avoid reaching into WebCore if you can help it.
> 
> > Source/WebKit/chromium/tests/WebFrameTest.cpp:181
> > +    webViewImpl->resize(WebSize(viewportWidth, viewportHeight));
> 
> Why do you need to use WebViewImpl here?
> 
> > Source/WebKit/chromium/tests/WebFrameTest.cpp:185
> > +    webViewImpl->mainFrameImpl()->frameView()->layout();
> 
> just call WebWidget::layout()?  WebView inherits from WebWidget

Done.
Comment 9 WebKit Review Bot 2012-04-10 15:38:18 PDT
Comment on attachment 136452 [details]
Patch

Clearing flags on attachment: 136452

Committed r113782: <http://trac.webkit.org/changeset/113782>
Comment 10 WebKit Review Bot 2012-04-10 15:38:23 PDT
All reviewed patches have been landed.  Closing bug.