Should check whether document.settings() is available or not when updating GenericFontFamilySetting…

Source/core/dom/StyleEngine.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 513 matching lines @@
 {
     // We should not recreate FontSelector. Instead, clear fontFaceCache.
     if (m_fontSelector)
         m_fontSelector->fontFaceCache()->clear();
     if (m_resolver)
         m_resolver->invalidateMatchedPropertiesCache();
 }
 
 void StyleEngine::updateGenericFontFamilySettings()
 {
+    if (!document().isActive() || !document().settings())

[dglazkov, 2014/05/29 22:24:28]

Just checking: is this a symptom of a larger problem? Is it ok for
WebSettingsImpl to call this on a frameless doc?

[tasak, 2014/05/30 02:12:05]

As far as I investigated,

WebSettingsImpl::setStandardFontFamily (i.e. m_settings != null)
 ==> Settings::notifyGenericFontFamilyChange()
 ==> Page::settingsChanged()
    (obtain StyleEngine instance by using mainFrame() and frame->document() and
document()->styleEngine())
 ==> StyleEngine::updateGenericFontFamilySettings()

Looking at Document::settings(),
 - m_frame->settings() used in Document::settings
 - m_host->settings() used in Frame::settings()
 - m_page->settings() used in FrameHost::settings()
 - Page has OwnPtr<Settings> (strictly speaking, SettingsDelegate owns.)

I think,
  - Page has non-null m_settings, because Page creates Settings in its
constructor (i.e. SettingsDelegate(Settings::create()) )
  - FrameHost also has non-null m_page, because Page creates FrameHost in its
constructor. (i.e. m_frameHost(FrameHost::create(*this)) )
  - Document has non-null m_frame, because frame->document() seems to work.

So LocalFrame seems not to have m_host.

Talking about "it's ok or not", each settings() says "settings() can return
null". LocalFrame's constructor takes "FrameHost*" (not FrameHost&).

So this "no FrameHost localframe" would be expected (by someone who added the
comment).

+        return;
     if (!m_fontSelector)
         return;
 
     m_fontSelector->updateGenericFontFamilySettings(*m_document);
     if (m_resolver)
         m_resolver->invalidateMatchedPropertiesCache();
 }
 
 void StyleEngine::removeFontFaceRules(const WillBeHeapVector<RawPtrWillBeMember<const StyleRuleFontFace> >& fontFaceRules)
 {
@@ skipping 97 matching lines @@
     visitor->trace(m_authorStyleSheets);
     visitor->trace(m_documentStyleSheetCollection);
     visitor->trace(m_styleSheetCollectionMap);
     visitor->trace(m_resolver);
     visitor->trace(m_fontSelector);
     visitor->trace(m_textToSheetCache);
     visitor->trace(m_sheetToTextCache);
 }
 
 }