Settings should not call into inspector

Source/core/inspector/InspectorPageAgent.cpp

 /*
  * Copyright (C) 2011 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 313 matching lines @@
     , m_page(page)
     , m_injectedScriptManager(injectedScriptManager)
     , m_client(client)
     , m_frontend(0)
     , m_overlay(overlay)
     , m_lastScriptIdentifier(0)
     , m_enabled(false)
     , m_ignoreScriptsEnabledNotification(false)
     , m_deviceMetricsOverridden(false)
     , m_emulateViewportEnabled(false)
+    , m_settingsCacheTextAutosizingEnabled(false)
+    , m_settingsCacheFontScaleFactor(1.0)
 {
 }
 
 void InspectorPageAgent::setFrontend(InspectorFrontend* frontend)
 {
     m_frontend = frontend->page();
 }
 
 void InspectorPageAgent::clearFrontend()
 {
@@ skipping 429 matching lines @@
 }
 
 void InspectorPageAgent::getScriptExecutionStatus(ErrorString*, PageCommandHandler::Result::Enum* status)
 {
     bool disabledByScriptController = false;
     bool disabledInSettings = false;
     Frame* frame = mainFrame();
     if (frame) {
         disabledByScriptController = !frame->script().canExecuteScripts(NotAboutToExecuteScript);
         if (frame->settings())
-            disabledInSettings = !frame->settings()->isScriptEnabled();
+            disabledInSettings = !frame->settings()->scriptEnabled();
     }
 
     if (!disabledByScriptController) {
         *status = PageCommandHandler::Result::Allowed;
         return;
     }
 
     if (disabledInSettings)
         *status = PageCommandHandler::Result::Disabled;
     else
@@ skipping 245 matching lines @@
 #endif
     m_frontend->frameResized();
 }
 
 void InspectorPageAgent::didRecalculateStyle()
 {
     if (m_enabled)
         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)
 {
     RefPtr<TypeBuilder::Page::Frame> frameObject = TypeBuilder::Page::Frame::create()
         .setId(frameId(frame))
         .setLoaderId(loaderId(frame->loader().documentLoader()))
         .setUrl(urlWithoutFragment(frame->document()->url()).string())
         .setMimeType(frame->loader().documentLoader()->responseMIMEType())
         .setSecurityOrigin(frame->document()->securityOrigin()->toRawString());
@@ skipping 52 matching lines @@
     }
     return result;
 }
 
 void InspectorPageAgent::updateViewMetrics(int width, int height, double deviceScaleFactor, bool emulateViewport, bool fitWindow)
 {
     if (width && height && !m_page->settings().acceleratedCompositingEnabled())
         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);
 
     Document* document = mainFrame()->document();
     if (document) {
         document->styleResolverChanged(RecalcStyleImmediately);
         document->mediaQueryAffectingValueChanged();
     }
     InspectorInstrumentation::mediaQueryResultChanged(document);
 
@@ skipping 18 matching lines @@
     }
 
     controller->didChangeDeviceOrientation(DeviceOrientationData::create(true, alpha, true, beta, true, gamma).get());
 }
 
 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));
 }
 
 void InspectorPageAgent::setTouchEmulationEnabled(ErrorString*, bool enabled)
 {
     if (m_state->getBoolean(PageAgentState::touchEventEmulationEnabled) == enabled)
         return;
     updateTouchEventEmulationInPage(enabled);
 }
 
@@ skipping 82 matching lines @@
 }
 
 void InspectorPageAgent::setShowViewportSizeOnResize(ErrorString*, bool show, const bool* showGrid)
 {
     m_state->setBoolean(PageAgentState::showSizeOnResize, show);
     m_state->setBoolean(PageAgentState::showGridOnResize, showGrid && *showGrid);
 }
 
 } // namespace WebCore