Title: [117170] trunk/Source
- Revision
- 117170
- Author
- [email protected]
- Date
- 2012-05-15 15:34:05 -0700 (Tue, 15 May 2012)
Log Message
[chromium] Fix unsafe viewport tag dispatch
https://bugs.webkit.org/show_bug.cgi?id=80554
Patch by Alexandre Elias <[email protected]> on 2012-05-15
Reviewed by Adam Barth.
In some uncommon situations (such as window.open() new tab on
Android), dispatchViewportPropertiesDidChange may early-return without
setting the page scale because the window size is not yet
available from the embedder. At that point, the previous behavior was
to call it again in layoutUpdated(), but this is unsafe since it
leaves a pending needsLayout.
I moved the dispatch call into WebViewImpl::didChangeContentsSize and
WebViewImpl::resize() instead -- these represent more explicitly the
situations where the inputs to the viewport tag calculation change. I
removed the other dispatch call from setFrameRect as it's now
redundant.
Covered by FixedLayoutInitializeAtMinimumPageScale test introduced
in http://webk.it/82949 (an assertion will fire there without
this fix).
Source/WebCore:
* page/FrameView.cpp:
(WebCore::FrameView::setFrameRect):
Source/WebKit/chromium:
* src/ChromeClientImpl.cpp:
(WebKit::ChromeClientImpl::layoutUpdated):
(WebKit::ChromeClientImpl::dispatchViewportPropertiesDidChange):
* src/WebViewImpl.cpp:
(WebKit::WebViewImpl::resize):
(WebKit::WebViewImpl::didChangeContentsSize):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (117169 => 117170)
--- trunk/Source/WebCore/ChangeLog 2012-05-15 22:33:19 UTC (rev 117169)
+++ trunk/Source/WebCore/ChangeLog 2012-05-15 22:34:05 UTC (rev 117170)
@@ -1,3 +1,30 @@
+2012-05-15 Alexandre Elias <[email protected]>
+
+ [chromium] Fix unsafe viewport tag dispatch
+ https://bugs.webkit.org/show_bug.cgi?id=80554
+
+ Reviewed by Adam Barth.
+
+ In some uncommon situations (such as window.open() new tab on
+ Android), dispatchViewportPropertiesDidChange may early-return without
+ setting the page scale because the window size is not yet
+ available from the embedder. At that point, the previous behavior was
+ to call it again in layoutUpdated(), but this is unsafe since it
+ leaves a pending needsLayout.
+
+ I moved the dispatch call into WebViewImpl::didChangeContentsSize and
+ WebViewImpl::resize() instead -- these represent more explicitly the
+ situations where the inputs to the viewport tag calculation change. I
+ removed the other dispatch call from setFrameRect as it's now
+ redundant.
+
+ Covered by FixedLayoutInitializeAtMinimumPageScale test introduced
+ in http://webk.it/82949 (an assertion will fire there without
+ this fix).
+
+ * page/FrameView.cpp:
+ (WebCore::FrameView::setFrameRect):
+
2012-05-15 Jer Noble <[email protected]>
r117147 causes a null-deref crash in DOMImplementation::createDocument()
Modified: trunk/Source/WebCore/page/FrameView.cpp (117169 => 117170)
--- trunk/Source/WebCore/page/FrameView.cpp 2012-05-15 22:33:19 UTC (rev 117169)
+++ trunk/Source/WebCore/page/FrameView.cpp 2012-05-15 22:34:05 UTC (rev 117170)
@@ -395,16 +395,6 @@
if (newRect == oldRect)
return;
-#if ENABLE(VIEWPORT)
- if (useFixedLayout()) {
- Document* document = m_frame->document();
- ViewportArguments viewport = document->viewportArguments();
- Page* page = frame() ? frame()->page() : 0;
- if (page)
- page->chrome()->client()->dispatchViewportPropertiesDidChange(viewport);
- }
-#endif
-
ScrollView::setFrameRect(newRect);
updateScrollableAreaSet();
Modified: trunk/Source/WebKit/chromium/ChangeLog (117169 => 117170)
--- trunk/Source/WebKit/chromium/ChangeLog 2012-05-15 22:33:19 UTC (rev 117169)
+++ trunk/Source/WebKit/chromium/ChangeLog 2012-05-15 22:34:05 UTC (rev 117170)
@@ -1,3 +1,34 @@
+2012-05-15 Alexandre Elias <[email protected]>
+
+ [chromium] Fix unsafe viewport tag dispatch
+ https://bugs.webkit.org/show_bug.cgi?id=80554
+
+ Reviewed by Adam Barth.
+
+ In some uncommon situations (such as window.open() new tab on
+ Android), dispatchViewportPropertiesDidChange may early-return without
+ setting the page scale because the window size is not yet
+ available from the embedder. At that point, the previous behavior was
+ to call it again in layoutUpdated(), but this is unsafe since it
+ leaves a pending needsLayout.
+
+ I moved the dispatch call into WebViewImpl::didChangeContentsSize and
+ WebViewImpl::resize() instead -- these represent more explicitly the
+ situations where the inputs to the viewport tag calculation change. I
+ removed the other dispatch call from setFrameRect as it's now
+ redundant.
+
+ Covered by FixedLayoutInitializeAtMinimumPageScale test introduced
+ in http://webk.it/82949 (an assertion will fire there without
+ this fix).
+
+ * src/ChromeClientImpl.cpp:
+ (WebKit::ChromeClientImpl::layoutUpdated):
+ (WebKit::ChromeClientImpl::dispatchViewportPropertiesDidChange):
+ * src/WebViewImpl.cpp:
+ (WebKit::WebViewImpl::resize):
+ (WebKit::WebViewImpl::didChangeContentsSize):
+
2012-05-15 James Robinson <[email protected]>
[chromium] Move createOffscreenGraphicsContext3D() from WebKitPlatformSupport to Platform
Modified: trunk/Source/WebKit/chromium/src/ChromeClientImpl.cpp (117169 => 117170)
--- trunk/Source/WebKit/chromium/src/ChromeClientImpl.cpp 2012-05-15 22:33:19 UTC (rev 117169)
+++ trunk/Source/WebKit/chromium/src/ChromeClientImpl.cpp 2012-05-15 22:34:05 UTC (rev 117170)
@@ -586,15 +586,6 @@
void ChromeClientImpl::layoutUpdated(Frame* frame) const
{
-#if ENABLE(VIEWPORT)
- if (!m_webView->isPageScaleFactorSet() && frame == frame->page()->mainFrame()) {
- // If the page does not have a viewport tag, then compute a scale
- // factor to make the page width fit the device width based on the
- // default viewport parameters.
- ViewportArguments viewport = frame->document()->viewportArguments();
- dispatchViewportPropertiesDidChange(viewport);
- }
-#endif
m_webView->layoutUpdated(WebFrameImpl::fromFrame(frame));
}
Modified: trunk/Source/WebKit/chromium/src/WebViewImpl.cpp (117169 => 117170)
--- trunk/Source/WebKit/chromium/src/WebViewImpl.cpp 2012-05-15 22:33:19 UTC (rev 117169)
+++ trunk/Source/WebKit/chromium/src/WebViewImpl.cpp 2012-05-15 22:34:05 UTC (rev 117170)
@@ -1311,6 +1311,13 @@
return;
m_size = newSize;
+#if ENABLE(VIEWPORT)
+ if (settings()->viewportEnabled()) {
+ ViewportArguments viewportArguments = mainFrameImpl()->frame()->document()->viewportArguments();
+ m_page->chrome()->client()->dispatchViewportPropertiesDidChange(viewportArguments);
+ }
+#endif
+
WebDevToolsAgentPrivate* agentPrivate = devToolsAgentPrivate();
if (agentPrivate && agentPrivate->metricsOverridden())
agentPrivate->webViewResized();
@@ -3001,9 +3008,21 @@
void WebViewImpl::didChangeContentsSize()
{
- bool didClampScale = computePageScaleFactorLimits();
+#if ENABLE(VIEWPORT)
+ if (!settings()->viewportEnabled())
+ return;
- if (!didClampScale)
+ bool didChangeScale = false;
+ if (!isPageScaleFactorSet()) {
+ // If the viewport tag failed to be processed earlier, we need
+ // to recompute it now.
+ ViewportArguments viewportArguments = mainFrameImpl()->frame()->document()->viewportArguments();
+ m_page->chrome()->client()->dispatchViewportPropertiesDidChange(viewportArguments);
+ didChangeScale = true;
+ } else
+ didChangeScale = computePageScaleFactorLimits();
+
+ if (!didChangeScale)
return;
if (!mainFrameImpl())
@@ -3012,6 +3031,7 @@
FrameView* view = mainFrameImpl()->frameView();
if (view && view->needsLayout())
view->layout();
+#endif
}
bool WebViewImpl::useExternalPopupMenus()
Modified: trunk/Source/WebKit/chromium/tests/WebFrameTest.cpp (117169 => 117170)
--- trunk/Source/WebKit/chromium/tests/WebFrameTest.cpp 2012-05-15 22:33:19 UTC (rev 117169)
+++ trunk/Source/WebKit/chromium/tests/WebFrameTest.cpp 2012-05-15 22:34:05 UTC (rev 117170)
@@ -226,10 +226,10 @@
WebView* webView = static_cast<WebView*>(FrameTestHelpers::createWebViewAndLoad(m_baseURL + "no_viewport_tag.html", true, 0, &client));
- webView->resize(WebSize(viewportWidth, viewportHeight));
webView->settings()->setViewportEnabled(true);
webView->settings()->setDefaultDeviceScaleFactor(2);
webView->enableFixedLayoutMode(true);
+ webView->resize(WebSize(viewportWidth, viewportHeight));
webView->layout();
EXPECT_EQ(2, webView->deviceScaleFactor());
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
