Settings should not call into inspector

Source/core/inspector/InspectorPageAgent.cpp

Index: Source/core/inspector/InspectorPageAgent.cpp
diff --git a/Source/core/inspector/InspectorPageAgent.cpp b/Source/core/inspector/InspectorPageAgent.cpp
index c1d6981e267a15ad5347ee3cea975d821f76439b..0bffef1de2b9a951fccef500cddb0a183c71dcf0 100644
--- a/Source/core/inspector/InspectorPageAgent.cpp
+++ b/Source/core/inspector/InspectorPageAgent.cpp
@@ -331,6 +331,8 @@ InspectorPageAgent::InspectorPageAgent(Page* page, InjectedScriptManager* inject
     , m_ignoreScriptsEnabledNotification(false)
     , m_deviceMetricsOverridden(false)
     , m_emulateViewportEnabled(false)
+    , m_settingsCacheTextAutosizingEnabled(false)
+    , m_settingsCacheFontScaleFactor(1.0)
 {
 }
 
@@ -780,7 +782,7 @@ void InspectorPageAgent::getScriptExecutionStatus(ErrorString*, PageCommandHandl
     if (frame) {
         disabledByScriptController = !frame->script().canExecuteScripts(NotAboutToExecuteScript);
         if (frame->settings())
-            disabledInSettings = !frame->settings()->isScriptEnabled();
+            disabledInSettings = !frame->settings()->scriptEnabled();
     }
 
     if (!disabledByScriptController) {
@@ -1046,12 +1048,28 @@ void InspectorPageAgent::didRecalculateStyle()
         m_overlay->update();
 }
 
-void InspectorPageAgent::scriptsEnabled(bool isEnabled)
+void InspectorPageAgent::settingsChanged()
 {
-    if (m_ignoreScriptsEnabledNotification)
+    Settings* settings = mainFrame()->settings();
+    if (!settings)
         return;
 
-    m_frontend->scriptsEnabled(isEnabled);
+    if (!m_ignoreScriptsEnabledNotification)
+        m_frontend->scriptsEnabled(settings->scriptEnabled());

[pfeldman, 2014/01/30 16:34:08]

We don't want to call scriptsEnabled with the same value upon every setting
change.

+
+    if (!m_deviceMetricsOverridden) {
+        m_settingsCacheTextAutosizingEnabled = settings->textAutosizingEnabled();

[pfeldman, 2014/01/30 16:34:08]

You want to store these unconditionally so that you could restore proper ones.

+        m_settingsCacheFontScaleFactor = settings->deviceScaleAdjustment();
+    } else {
+        if (settings->textAutosizingEnabled() != overrideTextAutosizing()) {
+            m_settingsCacheTextAutosizingEnabled = settings->textAutosizingEnabled();
+            settings->setTextAutosizingEnabled(overrideTextAutosizing());

[pfeldman, 2014/01/30 16:34:08]

So upon setTextAutosizingEnabled("A"), you will actually bring it back to "B"?
This looks extremely fragile to me.

It seems like instead, you want all call sites to respect the override. Either
we need another setting "textAutosizingOverride" that call sites respect or we
need to use InspectorInstrumentation::overrideTextAutosizing that was used in
settings, but place it into the actual call sites instead. We tend to do the
latter.

[gnana, 2014/01/31 14:21:09]

After Placing InspectorInstrumentation::overrideTextAutosizing &
overrideFontScaleFactor in actual call sites, we dont need to cache the settings
anymore. 

+        }
+        if (settings->deviceScaleAdjustment() != overrideFontScaleFactor()) {
+            m_settingsCacheFontScaleFactor = settings->deviceScaleAdjustment();
+            settings->setDeviceScaleAdjustment(overrideFontScaleFactor());
+        }
+    }
 }
 
 PassRefPtr<TypeBuilder::Page::Frame> InspectorPageAgent::buildObjectForFrame(Frame* frame)
@@ -1124,6 +1142,15 @@ void InspectorPageAgent::updateViewMetrics(int width, int height, double deviceS
         return;
 
     m_deviceMetricsOverridden = width && height;
+    Settings& settings = m_page->settings();
+    if (m_deviceMetricsOverridden) {
+        settings.setTextAutosizingEnabled(overrideTextAutosizing());
+        settings.setDeviceScaleAdjustment(overrideFontScaleFactor());
+    } else {
+        settings.setTextAutosizingEnabled(m_settingsCacheTextAutosizingEnabled);
+        settings.setDeviceScaleAdjustment(m_settingsCacheFontScaleFactor);
+    }
+
     m_emulateViewportEnabled = emulateViewport;
     m_client->overrideDeviceMetrics(width, height, static_cast<float>(deviceScaleFactor), emulateViewport, fitWindow);
 
@@ -1162,17 +1189,13 @@ void InspectorPageAgent::clearDeviceOrientationOverride(ErrorString* error)
     setDeviceOrientationOverride(error, 0, 0, 0);
 }
 
-bool InspectorPageAgent::overrideTextAutosizing(bool textAutosizing)
+bool InspectorPageAgent::overrideTextAutosizing()
 {
-    if (!m_deviceMetricsOverridden)
-        return textAutosizing;
     return m_state->getBoolean(PageAgentState::pageAgentTextAutosizingOverride);
 }
 
-float InspectorPageAgent::overrideFontScaleFactor(float fontScaleFactor)
+float InspectorPageAgent::overrideFontScaleFactor()
 {
-    if (!m_deviceMetricsOverridden)
-        return fontScaleFactor;
     return static_cast<float>(m_state->getDouble(PageAgentState::fontScaleFactor));
 }