Avoid clobbering state in viewport tag initialization.

third_party/WebKit/Source/core/dom/Document.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  *           (C) 2006 Alexey Proskuryakov (ap@webkit.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2011, 2012 Apple Inc. All rights reserved.
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (C) 2008, 2009, 2011, 2012 Google Inc. All rights reserved.
  * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) Research In Motion Limited 2010-2011. All rights reserved.
@@ skipping 3132 matching lines @@
     }
 
     if (httpRefreshType == HttpRefreshFromMetaTag && isSandboxed(SandboxAutomaticFeatures)) {
         String message = "Refused to execute the redirect specified via '<meta http-equiv='refresh' content='...'>'. The document is sandboxed, and the 'allow-scripts' keyword is not set.";
         addConsoleMessage(ConsoleMessage::create(SecurityMessageSource, ErrorMessageLevel, message));
         return;
     }
     m_frame->navigationScheduler().scheduleRedirect(delay, refreshURL);
 }
 
-bool Document::shouldMergeWithLegacyDescription(ViewportDescription::Type origin)
+bool Document::shouldMergeWithLegacyDescription(ViewportDescription::Type origin) const
 {
     return settings() && settings()->viewportMetaMergeContentQuirk() && m_legacyViewportDescription.isMetaViewportType() && m_legacyViewportDescription.type == origin;
 }
 
 void Document::setViewportDescription(const ViewportDescription& viewportDescription)
 {
     if (viewportDescription == m_viewportDescription)

[dgozman, 2016/05/09 17:36:19]

This check looks strange now (and was before). Why do we only compare with
|m_viewportDescription| if we can set |m_legacyViewportDescription| instead? I
think it's safe to remove it.

[aelias_OOO_until_Jul13, 2016/05/09 23:56:02]

Agreed it's weird now.  I'd like to preserve a safeguard against unnecessary
calls to updateViewportDescription() though which could do expensive stuff like
layouts, so I split this into two early-returns, one for legacy and one for not.

         return;
 
-    if (settings() && !settings()->viewportMetaEnabled() && viewportDescription.isLegacyViewportType())
-        return;
-
     // The UA-defined min-width is used by the processing of legacy meta tags.
     if (!viewportDescription.isSpecifiedByAuthor())
         m_viewportDefaultMinWidth = viewportDescription.minWidth;

[dgozman, 2016/05/09 17:36:19]

Should we do something similar to |m_viewportDefaultMinWidth|? It could possibly
mess up "request desktop" scenario, couldn't it?

[aelias_OOO_until_Jul13, 2016/05/09 23:56:02]

I don't think there's a risk of bugs with this one since it's basically just a
fixed constant of 980 in all cases, but anyway I moved it within the non-legacy
block which is the only place where it may appear.

 
     if (viewportDescription.isLegacyViewportType()) {
         m_legacyViewportDescription = viewportDescription;
-
-        // When no author style for @viewport is present, and a meta tag for defining
-        // the viewport is, apply the meta tag viewport instead of the UA styles.
-        if (m_viewportDescription.type == ViewportDescription::AuthorStyleSheet)
-            return;
+    } else {
         m_viewportDescription = viewportDescription;
-    } else {
-        // If the legacy viewport tag has higher priority than the cascaded @viewport
-        // descriptors, use the values from the legacy tag.
-        if (!shouldOverrideLegacyDescription(viewportDescription.type))
-            m_viewportDescription = m_legacyViewportDescription;
-        else
-            m_viewportDescription = viewportDescription;
     }
 
     updateViewportDescription();
 }
 
+ViewportDescription Document::viewportDescription() const
+{
+    ViewportDescription appliedViewportDescription = m_viewportDescription;
+    bool viewportMetaEnabled = settings() && settings()->viewportMetaEnabled();
+    if (m_legacyViewportDescription.type != ViewportDescription::UserAgentStyleSheet && viewportMetaEnabled)
+        appliedViewportDescription = m_legacyViewportDescription;
+    if (shouldOverrideLegacyDescription(m_viewportDescription.type))
+        appliedViewportDescription = m_viewportDescription;
+
+    return appliedViewportDescription;
+}
+
 void Document::updateViewportDescription()
 {
     if (frame() && frame()->isMainFrame()) {
-        frameHost()->chromeClient().dispatchViewportPropertiesDidChange(m_viewportDescription);
+        frameHost()->chromeClient().dispatchViewportPropertiesDidChange(viewportDescription());
     }
 }
 
 void Document::processReferrerPolicy(const String& policy)
 {
     ReferrerPolicy referrerPolicy;
     if (!SecurityPolicy::referrerPolicyFromString(policy, &referrerPolicy)) {
         addConsoleMessage(ConsoleMessage::create(RenderingMessageSource, ErrorMessageLevel, "Failed to set referrer policy: The value '" + policy + "' is not one of 'always', 'default', 'never', 'no-referrer', 'no-referrer-when-downgrade', 'origin', 'origin-when-crossorigin', or 'unsafe-url'. This document's referrer policy has been left unchanged."));
         return;
     }
@@ skipping 2824 matching lines @@
 #ifndef NDEBUG
 using namespace blink;
 void showLiveDocumentInstances()
 {
     Document::WeakDocumentSet& set = Document::liveDocumentSet();
     fprintf(stderr, "There are %u documents currently alive:\n", set.size());
     for (Document* document : set)
         fprintf(stderr, "- Document %p URL: %s\n", document, document->url().getString().utf8().data());
 }
 #endif