Bug 87061 – WKPageGetScaleFactor can return 0.0 after a session is restored

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Brady Eidson]

WKPageGetScaleFactor can return 0.0 after a session is restored

This is because http://trac.webkit.org/changeset/116617 made "0.0" the new default scale factor to indicate that the scale factor has never actually been set.

When restoring sessions in WK2 we propagate that 0.0 up to the UIProcess as the "actual" scale factor which then confuses clients.

Patch and API test coming.

In radar as <rdar://problem/11460336>

[Brady Eidson]

Created attachment 143150 [details]
Patch v1 - Fix + API test

[WebKit Review Bot]

Attachment 143150 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1
Tools/TestWebKitAPI/Tests/WebKit2/WKPageGetScaleFactorNotZero.cpp:76:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:1133:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 2 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[John Sullivan]

Comment on attachment 143150 [details]
Patch v1 - Fix + API test

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

> Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:1135
>      m_frame->page()->send(Messages::WebPageProxy::PageScaleFactorDidChange(scaleFactor));

It seems cleaner to bail out without calling PageScaleFactorDidChange() at all if we're just trying to use "no particular value". Would that approach not work for some reason? It doesn't seem right to set the scaleFactor to 1 if we have no information about the actual scaleFactor.

[Brady Eidson]

(In reply to comment #3)
> (From update of attachment 143150 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=143150&action=review
> 
> > Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:1135
> >      m_frame->page()->send(Messages::WebPageProxy::PageScaleFactorDidChange(scaleFactor));
> 
> It seems cleaner to bail out without calling PageScaleFactorDidChange() at all if we're just trying to use "no particular value". Would that approach not work for some reason? It doesn't seem right to set the scaleFactor to 1 if we have no information about the actual scaleFactor.

Previous behavior was to notify the UI process of the scale factor always, even if it was the default value of 1.0

This is important - for example - if the page already had content in it and the scale factor had been set to a non-default value.  Restoring a session in to that page with the default scale factor needs to also update the UI process's copy of the scale factor.

[Beth Dakin]

Comment on attachment 143150 [details]
Patch v1 - Fix + API test

You should fix the style errors, but r=me

[Brady Eidson]

(In reply to comment #5)
> (From update of attachment 143150 [details])
> You should fix the style errors, but r=me

Yah, misguided attempt to be more precise with floating points than with other numbers, based on "== 0.0" being scattered throughout WebCore.

Will fix.

Thanks for the review!

[Brady Eidson]

http://trac.webkit.org/changeset/117870