Bug 82757 - [Chromium] defaultDeviceScaleFactor != 0 should set deviceScaleFactor = defaultDeviceScaleFactor in the absence of a viewport tag for debugging purposes
: [Chromium] defaultDeviceScaleFactor != 0 should set deviceScaleFactor = defau...
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To: Tim Dresser
:
Depends on:
Blocks: 70559
  Show dependency treegraph
 
Reported: 2012-03-30 11:12 PDT by Fady Samuel
Modified: 2012-04-10 15:38 PDT (History)
8 users (show)

See Also:


Attachments
Patch (5.72 KB, patch)
2012-04-03 10:26 PDT, Tim Dresser
no flags Details | Formatted Diff | Diff
Patch (5.34 KB, patch)
2012-04-10 07:20 PDT, Tim Dresser
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.