Don't attach NetworkStateObserver until Document is initialized/attached

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.
@@ skipping 392 matching lines @@
       EnumerationHistogram, unseenFrameHistogram,
       ("Navigation.DeferredDocumentLoading.StatesV3", WouldLoadReasonEnd));
   unseenFrameHistogram.count(reason);
 }
 
 class Document::NetworkStateObserver final
     : public GarbageCollectedFinalized<Document::NetworkStateObserver>,
       public NetworkStateNotifier::NetworkStateObserver,
       public ContextLifecycleObserver {
   USING_GARBAGE_COLLECTED_MIXIN(Document::NetworkStateObserver);
-  EAGERLY_FINALIZE();
 
  public:
   explicit NetworkStateObserver(Document& document)
       : ContextLifecycleObserver(&document) {
     networkStateNotifier().addOnLineObserver(
         this,
         TaskRunnerHelper::get(TaskType::Networking, getExecutionContext()));
   }
 
-  // We eagerly finalize, so it's safe to touch getExecutionContext() here.
-  ~NetworkStateObserver() { unregisterAsObserver(getExecutionContext()); }
-
   void onLineStateChange(bool onLine) override {
     AtomicString eventName =
         onLine ? EventTypeNames::online : EventTypeNames::offline;
     Document* document = toDocument(getExecutionContext());
     if (!document->domWindow())
       return;
     document->domWindow()->dispatchEvent(Event::create(eventName));
     InspectorInstrumentation::networkStateChanged(document->frame(), onLine);
   }
 
   void contextDestroyed(ExecutionContext* context) override {
     unregisterAsObserver(context);
   }
 
   void unregisterAsObserver(ExecutionContext* context) {
-    if (!context)
-      return;
+    DCHECK(context);
     networkStateNotifier().removeOnLineObserver(
         this, TaskRunnerHelper::get(TaskType::Networking, context));
   }
 
   DEFINE_INLINE_VIRTUAL_TRACE() { ContextLifecycleObserver::trace(visitor); }
 };
 
 Document::Document(const DocumentInit& initializer,
                    DocumentClassFlags documentClasses)
     : ContainerNode(0, CreateDocument),
@@ skipping 79 matching lines @@
       m_didAssociateFormControlsTimer(
           TaskRunnerHelper::get(TaskType::UnspecedLoading, this),
           this,
           &Document::didAssociateFormControlsTimerFired),
       m_timers(TaskRunnerHelper::get(TaskType::Timer, this)),
       m_hasViewportUnits(false),
       m_parserSyncPolicy(AllowAsynchronousParsing),
       m_nodeCount(0),
       m_wouldLoadReason(Created),
       m_passwordCount(0),
-      m_engagementLevel(mojom::blink::EngagementLevel::NONE),
-      m_networkStateObserver(new NetworkStateObserver(*this)) {
+      m_engagementLevel(mojom::blink::EngagementLevel::NONE) {
   if (m_frame) {
     DCHECK(m_frame->page());
     provideContextFeaturesToDocumentFrom(*this, *m_frame->page());
 
     m_fetcher = m_frame->loader().documentLoader()->fetcher();
     FrameFetchContext::provideDocumentToContext(m_fetcher->context(), this);
 
     // TODO(dcheng): Why does this need to check that DOMWindow is non-null?
     CustomElementRegistry* registry =
         m_frame->domWindow() ? m_frame->domWindow()->maybeCustomElements()
@@ skipping 1889 matching lines @@
   // The TextAutosizer can't update layout view info while the Document is
   // detached, so update now in case anything changed.
   if (TextAutosizer* autosizer = textAutosizer())
     autosizer->updatePageInfo();
 
   m_frame->documentAttached();
   m_lifecycle.advanceTo(DocumentLifecycle::StyleClean);
 
   if (view())
     view()->didAttachDocument();
+
+  m_networkStateObserver = new NetworkStateObserver(*this);

[sof, 2017/02/27 07:54:33]

Could you add a short explanatory comment why this is done here?

[kinuko, 2017/02/27 09:15:05]

Done.

 }
 
 void Document::shutdown() {
   TRACE_EVENT0("blink", "Document::shutdown");
   RELEASE_ASSERT(!m_frame || m_frame->tree().childCount() == 0);
   if (!isActive())
     return;
 
   // Frame navigation can cause a new Document to be attached. Don't allow that,
   // since that will cause a situation where LocalFrame still has a Document
@@ skipping 4199 matching lines @@
 }
 
 void showLiveDocumentInstances() {
   WeakDocumentSet& set = liveDocumentSet();
   fprintf(stderr, "There are %u documents currently alive:\n", set.size());
   for (blink::Document* document : set)
     fprintf(stderr, "- Document %p URL: %s\n", document,
             document->url().getString().utf8().data());
 }
 #endif