Deferred frame loading stats v2

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 380 matching lines @@
 
 // This doesn't work with non-Document ExecutionContext.
 static void runAutofocusTask(ExecutionContext* context) {
   Document* document = toDocument(context);
   if (Element* element = document->autofocusElement()) {
     document->setAutofocusElement(0);
     element->focus();
   }
 }
 
-// These are logged to UMA, so don't re-arrange them without creating a new
-// histogram.
-enum DocumentVisibilityForDeferredLoading {
-  Created,
-  WouldLoadBecauseVisible,
-  // TODO(dgrogan): Add WouldLoadBecauseTopOrLeft, WouldLoadBecauseDisplayNone,
-  // etc
-
-  DocumentVisibilityForDeferredLoadingEnd
-};
-
-static void RecordStateToHistogram(DocumentVisibilityForDeferredLoading state) {
-  DEFINE_STATIC_LOCAL(EnumerationHistogram, unseenFrameHistogram,
-                      ("Navigation.DeferredDocumentLoading.StatesV1",
-                       DocumentVisibilityForDeferredLoadingEnd));
-  unseenFrameHistogram.count(state);
+static void recordLoadReasonToHistogram(WouldLoadReason reason) {
+  DEFINE_STATIC_LOCAL(
+      EnumerationHistogram, unseenFrameHistogram,
+      ("Navigation.DeferredDocumentLoading.StatesV2", WouldLoadReasonEnd));
+  unseenFrameHistogram.count(reason);
 }
 
 Document::Document(const DocumentInit& initializer,
                    DocumentClassFlags documentClasses)
     : ContainerNode(0, CreateDocument),
       TreeScope(*this),
       m_hasNodesWithPlaceholderStyle(false),
       m_evaluateMediaQueriesOnStyleRecalc(false),
       m_pendingSheetLayout(NoLayoutWithPendingSheets),
       m_frame(initializer.frame()),
@@ skipping 62 matching lines @@
       m_timeline(DocumentTimeline::create(this)),
       m_compositorPendingAnimations(new CompositorPendingAnimations()),
       m_templateDocumentHost(nullptr),
       m_didAssociateFormControlsTimer(
           this,
           &Document::didAssociateFormControlsTimerFired),
       m_timers(TaskRunnerHelper::get(TaskType::Timer, this)->clone()),
       m_hasViewportUnits(false),
       m_parserSyncPolicy(AllowAsynchronousParsing),
       m_nodeCount(0),
-      m_visibilityWasLogged(false) {
+      m_wouldLoadReason(Created) {
   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);
 
     CustomElementRegistry* registry =
         m_frame->localDOMWindow()
             ? m_frame->localDOMWindow()->maybeCustomElements()
@@ skipping 11 matching lines @@
   // We depend on the url getting immediately set in subframes, but we
   // also depend on the url NOT getting immediately set in opened windows.
   // See fast/dom/early-frame-url.html
   // and fast/dom/location-new-window-no-crash.html, respectively.
   // FIXME: Can/should we unify this behavior?
   if (initializer.shouldSetURL())
     setURL(initializer.url());
 
   initSecurityContext(initializer);
   DCHECK(getSecurityOrigin());
-  if (frame() && frame()->tree().top()->securityContext() &&
+  if (frame() &&
+      frame()->loader().stateMachine()->committedFirstRealDocumentLoad() &&
+      frame()->tree().top()->securityContext() &&

[dgrogan, 2016/10/18 22:11:45]

Even checking committedFirstRealDocumentLoad() first, changing this to
frame()->isCrossOriginSubframe() still dies with the same null pointer deref.

[dcheng, 2016/10/19 01:34:54]

OK, yeah, I think this logic should really just live in installNewDocument().

[dgrogan, 2016/10/19 18:59:54]

Done.

       !getSecurityOrigin()->canAccess(
-          frame()->tree().top()->securityContext()->getSecurityOrigin()))
-    RecordStateToHistogram(Created);
+          frame()->tree().top()->securityContext()->getSecurityOrigin())) {
+    recordLoadReasonToHistogram(Created);
+  }
 
   initDNSPrefetch();
 
   InstanceCounters::incrementCounter(InstanceCounters::DocumentCounter);
 
   m_lifecycle.advanceTo(DocumentLifecycle::Inactive);
 
   // Since CSSFontSelector requires Document::m_fetcher and StyleEngine owns
   // CSSFontSelector, need to initialize m_styleEngine after initializing
   // m_fetcher.
@@ skipping 5813 matching lines @@
   visitor->trace(m_resizeObserverController);
   visitor->trace(m_propertyRegistry);
   visitor->trace(m_nonAttachedStyle);
   Supplementable<Document>::trace(visitor);
   TreeScope::trace(visitor);
   ContainerNode::trace(visitor);
   ExecutionContext::trace(visitor);
   SecurityContext::trace(visitor);
 }
 
-void Document::onVisibilityMaybeChanged(bool visible) {
+void Document::wouldLoadBecause(WouldLoadReason reason) {
+  DCHECK(reason != Created);
   DCHECK(frame());
-  if (visible && !m_visibilityWasLogged && frame()->isCrossOriginSubframe()) {
-    m_visibilityWasLogged = true;
-    RecordStateToHistogram(WouldLoadBecauseVisible);
+  if (m_wouldLoadReason == Created && frame()->isCrossOriginSubframe() &&
+      frame()->loader().stateMachine()->committedFirstRealDocumentLoad()) {
+    recordLoadReasonToHistogram(reason);
   }
+  m_wouldLoadReason = reason;
 }
 
 DEFINE_TRACE_WRAPPERS(Document) {
   visitor->traceWrappers(m_importsController);
   visitor->traceWrappers(m_implementation);
   visitor->traceWrappers(m_styleSheetList);
   visitor->traceWrappers(m_styleEngine);
   visitor->traceWrappers(Supplementable<Document>::m_supplements.get(
       FontFaceSet::supplementName()));
   for (int i = 0; i < numNodeListInvalidationTypes; ++i) {
@@ skipping 17 matching lines @@
 }
 
 void showLiveDocumentInstances() {
   WeakDocumentSet& set = liveDocumentSet();
   fprintf(stderr, "There are %u documents currently alive:\n", set.size());
   for (Document* document : set)
     fprintf(stderr, "- Document %p URL: %s\n", document,
             document->url().getString().utf8().data());
 }
 #endif