Enabled using viewport on desktop browsers behind experimental flag

Source/WebKit/chromium/src/WebViewImpl.cpp

Index: Source/WebKit/chromium/src/WebViewImpl.cpp
diff --git a/Source/WebKit/chromium/src/WebViewImpl.cpp b/Source/WebKit/chromium/src/WebViewImpl.cpp
index 2118aa9fbdebd914d0f525f53ae9fe752fa16b48..d84a1250c47795f2af9771ebaca04db86ac6ca10 100644
--- a/Source/WebKit/chromium/src/WebViewImpl.cpp
+++ b/Source/WebKit/chromium/src/WebViewImpl.cpp
@@ -1599,7 +1599,9 @@ void WebViewImpl::resize(const WebSize& newSize)
 
     m_size = newSize;
 
-    bool shouldAnchorAndRescaleViewport = settings()->viewportEnabled() && oldSize.width && oldContentsWidth;
+    bool shouldAnchorAndRescaleViewport = settings()->viewportEnabled()
+        && settings()->deviceSupportsRotation() && oldSize.width && oldContentsWidth;

[Rick Byers, 2013/06/04 00:25:30]

Note that ChromeOS does support rotation (Ctrl-Shift-F3).  From aelias's comment
in the bug I gather this code assumes that with viewport enabled, resizes ONLY
come from rotation.  Perhaps you want a different 'has resizable windows'
setting?  Also please add a comment describing what's going on here - I was
totally lost before reading the bug comments. 

[bokan, 2013/06/04 18:52:16]

Ah, resizableWindow makes more sense then - I'll change it. I assume in that
case we don't want to have Android-like behaviour when the screen's rotated on
CrOS? (i.e. rescale the viewport).

[Rick Byers, 2013/06/05 15:04:08]

Right.

+
     ViewportAnchor viewportAnchor(mainFrameImpl()->frame()->eventHandler());
     if (shouldAnchorAndRescaleViewport) {
         viewportAnchor.setAnchor(view->visibleContentRect(),
@@ -1609,6 +1611,9 @@ void WebViewImpl::resize(const WebSize& newSize)
     // Set the fixed layout size from the viewport constraints before resizing.
     updatePageDefinedPageScaleConstraints(mainFrameImpl()->frame()->document()->viewportArguments());
 
+    if (settings()->desktopViewportEnabled() && mainFrameImpl()->frame()->document()->viewportArguments().width == ViewportArguments::ValueAuto)
+        setFixedLayoutSize(newSize);

[Rick Byers, 2013/06/04 00:25:30]

Shouldn't we also be updating the size on resize under width=device-width (since
device-width means window width on desktop)?

[Rick Byers, 2013/06/04 00:25:30]

Shouldn't we also do this in the ValueDeviceWidth case (since width=device-width
is the same as width=auto on desktop)?

Do we really need a separate desktopViewPortEnabled setting, or could we
accomplish all the same things with 'viewportEnabled() &&
supportsResizableWindows()'?  The overall goal of this CL is to reduce the ways
the mobile web platform and desktop web platform have diverged since the line is
just becoming blurrier :-)

[bokan, 2013/06/04 18:52:16]

That happens in the method call above, along with other handling of the viewport
tag. If there was no viewport width set (which is ValueAuto) then we want to set
the layout size to the window so that we emulate the non-viewport case. 

[bokan, 2013/06/04 18:52:16]

Agreed, and I originally had everything behind viewportEnabled, but I don't know
how Android behaves in cases like these (e.g What would happen here when we set
layout size?) and I currently have no way to test it. How difficult/practical is
it to setup an Android build and device? 

[Rick Byers, 2013/06/05 15:04:08]

I think this logic belongs inside of updatePageDefinedPageScaleConstraints then.
 See crbug.com/229151 with some debate on exactly how this should happen, but
for now I think it's right to rely on a setting (just one named more precisely -
eg. layoutFallbackWidthIsWindowWidth or something). 

[Rick Byers, 2013/06/05 15:04:08]

You should be able to build content shell easily enough (which should be
sufficient here), see
https://code.google.com/p/chromium/wiki/AndroidBuildInstructions.

But being extra careful not to modify android behavior is a good idea.  Again
this is probably just about defining the settings to clearly indicate possible
differences between desktop and mobile (what the fallback width is and how
resize is handled).  In follow up work we can try to unify behavior and remove
the setting.

[bokan, 2013/06/06 19:43:47]

Done.

+
     WebDevToolsAgentPrivate* agentPrivate = devToolsAgentPrivate();
     if (agentPrivate)
         agentPrivate->webViewResized(newSize);
@@ -3002,7 +3007,7 @@ void WebViewImpl::updatePageDefinedPageScaleConstraints(const ViewportArguments&
     if (!settings()->viewportEnabled() || !isFixedLayoutModeEnabled() || !page() || !m_size.width || !m_size.height)
         return;
 
-    m_pageScaleConstraintsSet.updatePageDefinedConstraints(arguments, m_size, page()->settings()->layoutFallbackWidth());
+    m_pageScaleConstraintsSet.updatePageDefinedConstraints(arguments, m_size, settings()->desktopViewportEnabled() ? 0 : page()->settings()->layoutFallbackWidth());

[Rick Byers, 2013/06/04 00:25:30]

Rather than change the code here, can't we change the implementation of
layoutFallbackWidth?  Again it could be keyed off of whether we have resizable
windows or not...

[bokan, 2013/06/04 18:52:16]

Actually, I already set fallback width to 0 if the setting is on so this is
redundant. Removed.

 
     if (settingsImpl()->supportDeprecatedTargetDensityDPI())
         m_pageScaleConstraintsSet.adjustPageDefinedConstraintsForAndroidWebView(arguments, m_size, page()->settings()->layoutFallbackWidth(), deviceScaleFactor(), page()->settings()->useWideViewport(), page()->settings()->loadWithOverviewMode());
@@ -3589,7 +3594,7 @@ void WebViewImpl::didCommitLoad(bool* isNewNavigation, bool isNavigationWithinPa
     m_newNavigationLoader = 0;
 #endif
     m_observedNewNavigation = false;
-    if (*isNewNavigation && !isNavigationWithinPage)
+    if ((*isNewNavigation && !isNavigationWithinPage) || settings()->desktopViewportEnabled())

[Rick Byers, 2013/06/04 00:25:30]

Why is this different for android/desktop?

[bokan, 2013/06/04 18:52:16]

So I'm not sure why, but sometimes when I'd reload a page it's scale factor
would get reset to default, even though the page scale variable was still set so
the viewport would fill the window. Adding this makes it reload that scale. I'll
have to ask aelias@ about what's the appropriate thing to do here.

[Rick Byers, 2013/06/05 15:04:08]

Ok.  I'm OK with a temporary hack (eg. relying on a setting or perhaps even a
platform ifdef) in order to get this code landed since we don't even know yet if
it'll last.  Just be sure to call out the issue in a FIXME comment and reference
a bug to track resolving it.  

But yes, better yet, hopefully aelias can just help you figure out the right
thing to do. 

         m_pageScaleConstraintsSet.setNeedsReset(true);
 
     // Make sure link highlight from previous page is cleared.