Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(300)

Issue 26251010: Reset to initial page scale in additional scenarios. (Closed)

Created:
7 years, 2 months ago by aelias_OOO_until_Jul13
Modified:
7 years, 1 month ago
CC:
blink-reviews
Visibility:
Public.

Description

Reset to initial page scale in additional scenarios. This patch causes page scale to reset to initial scale in two more cases: 1) When the page-defined initial scale goes from -1 to a specific value, or changes value. This fixes a race where if page scale was reset before the viewport tag was processed, then we would never go to initial scale. 2) When the initial scale was set to a value less than the final minimum scale and the content width expands. In this case, the content width was previously preventing us from going to the desired scale so we should try again now that it's expanded. Also no-op cleanup of some surrounding tests. NOTRY=true BUG= internal b/10935045, b/10838508 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=161255

Patch Set 1 #

Patch Set 2 : Merge both setNeedsResets into a single one in WebViewImpl #

Patch Set 3 : Add test coverage and extra setNeedsLayout() call #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+56 lines, -30 lines) Patch
M Source/web/PageScaleConstraintsSet.h View 1 chunk +2 lines, -0 lines 0 comments Download
M Source/web/PageScaleConstraintsSet.cpp View 1 3 chunks +13 lines, -10 lines 0 comments Download
M Source/web/WebViewImpl.cpp View 1 2 1 chunk +7 lines, -0 lines 1 comment Download
M Source/web/tests/WebFrameTest.cpp View 1 2 4 chunks +34 lines, -20 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
aelias_OOO_until_Jul13
Hi Mikhail, PTAL. I got around to writing tests. While writing the test for condition ...
7 years, 1 month ago (2013-11-04 09:28:33 UTC) #1
mnaganov (inactive)
LGTM, thanks! https://codereview.chromium.org/26251010/diff/6001/Source/web/WebViewImpl.cpp File Source/web/WebViewImpl.cpp (right): https://codereview.chromium.org/26251010/diff/6001/Source/web/WebViewImpl.cpp#newcode3012 Source/web/WebViewImpl.cpp:3012: if (mainFrameImpl() && mainFrameImpl()->frameView()) I guess, we ...
7 years, 1 month ago (2013-11-04 11:44:09 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/aelias@chromium.org/26251010/6001
7 years, 1 month ago (2013-11-04 17:24:08 UTC) #3
commit-bot: I haz the power
7 years, 1 month ago (2013-11-04 17:29:51 UTC) #4
Message was sent while issue was closed.
Change committed as 161255

Powered by Google App Engine
This is Rietveld 408576698