Make sure StyleEngine uses the same CSSFontSelector in its lifetime

StyleEngine shouldn't switch CSSFontSelector during its lifetime. It's
almost ensured, except that popups may change it during initialization.

This patch rewrites CSSFontSelector initialization in popup documents,
so that the correct CSSFontSelector is used from the beginning, and does
not need to be overridden later.

This is preparation for ensuring no FontSelector switching for Font and
FontFallbackList.

Bug: 1049295
Change-Id: Iaee3121ef06e4d2d6ac357bfa61332eee1da0bbf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2099297
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#749502}

third_party/blink/renderer/core/css/style_engine.cc

@@ -77,6 +77,7 @@
 #include "third_party/blink/renderer/core/layout/layout_object.h"
 #include "third_party/blink/renderer/core/layout/layout_view.h"
 #include "third_party/blink/renderer/core/page/page.h"
+#include "third_party/blink/renderer/core/page/page_popup_controller.h"
 #include "third_party/blink/renderer/core/paint/paint_layer.h"
 #include "third_party/blink/renderer/core/probe/core_probes.h"
 #include "third_party/blink/renderer/core/style/computed_style.h"
@@ -93,6 +94,18 @@
 
 namespace blink {
 
+namespace {
+
+CSSFontSelector* CreateCSSFontSelectorFor(Document& document) {
+  DCHECK(document.GetFrame());
+  if (UNLIKELY(document.GetFrame()->PagePopupOwner())) {
+    return PagePopupController::CreateCSSFontSelector(document);
+  }
+  return MakeGarbageCollected<CSSFontSelector>(&document);
+}
+
+}  // namespace
+
 StyleEngine::StyleEngine(Document& document)
     : document_(&document),
       is_master_(!document.IsHTMLImport()),
@@ -101,7 +114,7 @@ StyleEngine::StyleEngine(Document& document)
   if (document.GetFrame()) {
     // We don't need to create CSSFontSelector for imported document or
     // HTMLTemplateElement's document, because those documents have no frame.
-    font_selector_ = MakeGarbageCollected<CSSFontSelector>(&document);
+    font_selector_ = CreateCSSFontSelectorFor(document);
     font_selector_->RegisterForInvalidationCallbacks(this);
   }
   if (document.IsInMainFrame())
@@ -879,14 +892,6 @@ void StyleEngine::FontsNeedUpdate(FontSelector*) {
                       nullptr);
 }
 
-void StyleEngine::SetFontSelector(CSSFontSelector* font_selector) {
-  if (font_selector_)
-    font_selector_->UnregisterForInvalidationCallbacks(this);
-  font_selector_ = font_selector;
-  if (font_selector_)
-    font_selector_->RegisterForInvalidationCallbacks(this);
-}
-
 void StyleEngine::PlatformColorsChanged() {
   if (resolver_)
     resolver_->InvalidateMatchedPropertiesCache();

third_party/blink/renderer/core/css/style_engine.h

@@ -245,7 +245,6 @@ class CORE_EXPORT StyleEngine final : public GarbageCollected<StyleEngine>,
   void UpdateLayoutTreeRebuildRoot(ContainerNode* ancestor, Node* dirty_node);
 
   CSSFontSelector* GetFontSelector() { return font_selector_; }
-  void SetFontSelector(CSSFontSelector*);
 
   void RemoveFontFaceRules(const HeapVector<Member<const StyleRuleFontFace>>&);
   // updateGenericFontFamilySettings is used from WebSettingsImpl.

third_party/blink/renderer/core/exported/web_page_popup_impl.cc

@@ -303,12 +303,12 @@ void WebPagePopupImpl::Initialize(WebViewImpl* web_view,
                                        /* InterfaceRegistry* */ nullptr);
   frame->SetPagePopupOwner(popup_client_->OwnerElement());
   frame->SetView(MakeGarbageCollected<LocalFrameView>(*frame));
+  PagePopupSupplement::Install(*frame, *this, popup_client_);
   frame->Init();
   frame->View()->SetParentVisible(true);
   frame->View()->SetSelfVisible(true);
 
   DCHECK(frame->DomWindow());
-  PagePopupSupplement::Install(*frame, *this, popup_client_);
   DCHECK_EQ(popup_client_->OwnerElement().GetDocument().ExistingAXObjectCache(),
             frame->GetDocument()->ExistingAXObjectCache());
   if (AXObjectCache* cache = frame->GetDocument()->ExistingAXObjectCache()) {

third_party/blink/renderer/core/html/forms/color_chooser_popup_ui_controller.h

@@ -55,7 +55,6 @@ class CORE_EXPORT ColorChooserPopupUIController final
 
   // PagePopupClient functions:
   void WriteDocument(SharedBuffer*) override;
-  void SelectFontsFromOwnerDocument(Document&) override {}
   Locale& GetLocale() override;
   void SetValueAndClosePopup(int, const String&) override;
   void SetValue(const String&) override;

third_party/blink/renderer/core/html/forms/date_time_chooser_impl.h

@@ -60,7 +60,6 @@ class CORE_EXPORT DateTimeChooserImpl final : public DateTimeChooser,
  private:
   // PagePopupClient functions:
   void WriteDocument(SharedBuffer*) override;
-  void SelectFontsFromOwnerDocument(Document&) override {}
   Locale& GetLocale() override;
   void SetValueAndClosePopup(int, const String&) override;
   void SetValue(const String&) override;

third_party/blink/renderer/core/html/forms/internal_popup_menu.cc

@@ -414,11 +414,11 @@ void InternalPopupMenu::AddSeparator(ItemIterationContext& context,
   PagePopupClient::AddString("},\n", data);
 }
 
-void InternalPopupMenu::SelectFontsFromOwnerDocument(Document& document) {
+CSSFontSelector* InternalPopupMenu::CreateCSSFontSelector(
+    Document& popup_document) {
   Document& owner_document = OwnerElement().GetDocument();
-  document.GetStyleEngine().SetFontSelector(
-      MakeGarbageCollected<PopupMenuCSSFontSelector>(
-          &document, owner_document.GetStyleEngine().GetFontSelector()));
+  return MakeGarbageCollected<PopupMenuCSSFontSelector>(
+      &popup_document, owner_document.GetStyleEngine().GetFontSelector());
 }
 
 void InternalPopupMenu::SetValueAndClosePopup(int num_value,

third_party/blink/renderer/core/html/forms/internal_popup_menu.h

@@ -12,6 +12,7 @@
 namespace blink {
 
 class ChromeClient;
+class CSSFontSelector;
 class PagePopup;
 class HTMLElement;
 class HTMLHRElement;
@@ -49,7 +50,7 @@ class CORE_EXPORT InternalPopupMenu final : public PopupMenu,
 
   // PagePopupClient functions:
   void WriteDocument(SharedBuffer*) override;
-  void SelectFontsFromOwnerDocument(Document&) override;
+  CSSFontSelector* CreateCSSFontSelector(Document& popup_document) override;
   void SetValueAndClosePopup(int, const String&) override;
   void SetValue(const String&) override;
   void CancelPopup() override;

third_party/blink/renderer/core/html/forms/resources/listPicker.js

@@ -40,7 +40,6 @@ function handleArgumentsTimeout() {
  */
 function ListPicker(element, config) {
   Picker.call(this, element, config);
-  window.pagePopupController.selectFontsFromOwnerDocument(document);
   this._selectElement = createElement('select');
   this._selectElement.size = 20;
   this._element.appendChild(this._selectElement);

third_party/blink/renderer/core/page/page_popup_client.cc

@@ -30,6 +30,7 @@
 
 #include "third_party/blink/renderer/core/page/page_popup_client.h"
 
+#include "third_party/blink/renderer/core/css/css_font_selector.h"
 #include "third_party/blink/renderer/core/dom/node_computed_style.h"
 #include "third_party/blink/renderer/core/frame/local_frame.h"
 #include "third_party/blink/renderer/core/page/chrome_client.h"
@@ -155,4 +156,9 @@ void PagePopupClient::AddProperty(const char* name,
   addLiteral("},\n", data);
 }
 
+CSSFontSelector* PagePopupClient::CreateCSSFontSelector(
+    Document& popup_document) {
+  return MakeGarbageCollected<CSSFontSelector>(&popup_document);
+}
+
 }  // namespace blink

third_party/blink/renderer/core/page/page_popup_client.h

@@ -39,6 +39,7 @@
 
 namespace blink {
 
+class CSSFontSelector;
 class ChromeClient;
 class Document;
 class Element;
@@ -53,13 +54,12 @@ class CORE_EXPORT PagePopupClient {
   //  - window.setValueAndClosePopup(number, string).
   virtual void WriteDocument(SharedBuffer*) = 0;
 
-  // This is called after the document is ready to do additionary setup.
-  virtual void SelectFontsFromOwnerDocument(Document&) = 0;
-
   virtual Element& OwnerElement() = 0;
 
   virtual ChromeClient& GetChromeClient() = 0;
 
+  virtual CSSFontSelector* CreateCSSFontSelector(Document& popup_document);
+
   // Returns effective zoom factor of ownerElement, or the page zoom factor if
   // the effective zoom factor is not available.
   virtual float ZoomFactor();

third_party/blink/renderer/core/page/page_popup_controller.cc

@@ -34,6 +34,7 @@
 #include "third_party/blink/public/strings/grit/blink_strings.h"
 #include "third_party/blink/renderer/core/page/page_popup.h"
 #include "third_party/blink/renderer/core/page/page_popup_client.h"
+#include "third_party/blink/renderer/core/page/page_popup_supplement.h"
 #include "third_party/blink/renderer/platform/text/platform_locale.h"
 
 namespace blink {
@@ -60,13 +61,6 @@ void PagePopupController::closePopup() {
     popup_client_->CancelPopup();
 }
 
-void PagePopupController::selectFontsFromOwnerDocument(
-    Document* target_document) {
-  DCHECK(target_document);
-  if (popup_client_)
-    popup_client_->SelectFontsFromOwnerDocument(*target_document);
-}
-
 String PagePopupController::localizeNumberString(const String& number_string) {
   if (popup_client_)
     return popup_client_->GetLocale().ConvertToLocalizedNumber(number_string);
@@ -112,4 +106,17 @@ void PagePopupController::setWindowRect(int x, int y, int width, int height) {
   popup_.SetWindowRect(IntRect(x, y, width, height));
 }
 
+// static
+CSSFontSelector* PagePopupController::CreateCSSFontSelector(
+    Document& popup_document) {
+  LocalFrame* frame = popup_document.GetFrame();
+  DCHECK(frame);
+  DCHECK(frame->PagePopupOwner());
+
+  auto* controller = PagePopupSupplement::From(*frame).GetPagePopupController();
+
+  DCHECK(controller->popup_client_);
+  return controller->popup_client_->CreateCSSFontSelector(popup_document);
+}
+
 }  // namespace blink

third_party/blink/renderer/core/page/page_popup_controller.h

@@ -37,6 +37,7 @@
 
 namespace blink {
 
+class CSSFontSelector;
 class Document;
 class PagePopup;
 class PagePopupClient;
@@ -50,7 +51,6 @@ class PagePopupController final : public ScriptWrappable {
   void setValueAndClosePopup(int num_value, const String& string_value);
   void setValue(const String&);
   void closePopup();
-  void selectFontsFromOwnerDocument(Document* target_document);
   String localizeNumberString(const String&);
   String formatMonth(int year, int zero_base_month);
   String formatShortMonth(int year, int zero_base_month);
@@ -60,6 +60,8 @@ class PagePopupController final : public ScriptWrappable {
   void ClearPagePopupClient();
   void setWindowRect(int x, int y, int width, int height);
 
+  static CSSFontSelector* CreateCSSFontSelector(Document& popup_document);
+
  private:
   PagePopup& popup_;
   PagePopupClient* popup_client_;

third_party/blink/renderer/core/page/page_popup_controller.idl

@@ -38,7 +38,6 @@
     void setValueAndClosePopup(long numberValue, DOMString stringValue);
     void setValue(DOMString value);
     void closePopup();
-    void selectFontsFromOwnerDocument(Document targetDocument);
     DOMString localizeNumberString(DOMString numberString);
     DOMString formatMonth(long year, long zeroBaseMonth);
     DOMString formatShortMonth(long year, long zeroBaseMonth);