Report nav timing 2 instance as soon as it's requested.

third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp

 /*
     Copyright (C) 1998 Lars Knoll (knoll@mpi-hd.mpg.de)
     Copyright (C) 2001 Dirk Mueller (mueller@kde.org)
     Copyright (C) 2002 Waldo Bastian (bastian@kde.org)
     Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All
     rights reserved.
     Copyright (C) 2009 Torch Mobile Inc. http://www.torchmobile.com/
 
     This library is free software; you can redistribute it and/or
     modify it under the terms of the GNU Library General Public
@@ skipping 635 matching lines @@
   if (request.requestContext() == WebURLRequest::RequestContextUnspecified)
     determineRequestContext(request, type);
   if (type == Resource::LinkPrefetch)
     request.setHTTPHeaderField(HTTPNames::Purpose, "prefetch");
 
   context().addAdditionalRequestHeaders(
       request,
       (type == Resource::MainResource) ? FetchMainResource : FetchSubresource);
 }
 
+std::unique_ptr<ResourceTimingInfo> ResourceFetcher::getNavigationTimingInfo() {
+  return std::move(m_navigationTimingInfo);

[Kunihiko Sakamoto, 2017/02/03 01:28:07]

Scripts can be executed from partially loaded main document, so if we release
m_navigationTimingInfo while the main resource is still loading,
ResourceFetcher::handleLoaderFinish() may crash.

[sunjian, 2017/02/03 20:15:21]

Added an extra field which is raw pointer of m_navigationTimingInfo. I think it
will fix this problem.

[panicker, 2017/02/03 23:27:43]

This is strange. I think your intention is shared_ptr i.e. fetcher and
NavigationTiming both share ownership of this ResourceTimingInfo. Probably makes
sense to use scoped_refptr.
Note that shared_ptr is not allowed in chromium:
https://chromium-cpp.appspot.com/

[sunjian, 2017/02/04 00:08:36]

A shared smart pointer probably makes things easier. But i don't think we can
convert a std::unique_ptr into a scoped_refptr (correct me if otherwise). And
ResourceTimingInfo only allows single ownership (its create method returns a
std::unique_ptr object).
https://www.chromium.org/developers/smart-pointer-guidelines. This doc mentions
that we should try to avoid using scoped_refptr if possible. I don't like having
a raw pointer either because it may end up getting orphaned and pointing to
something that doesn't exist anymore. However, this worry kind of goes away when
knowing this situation is unlikely to happen cause PerformanceNavigationTiming
instance, if gets created, will always outlive ResourceFetcher instance, which
means that the raw pointer reference will never get orphaned. Correct me if i'm
missing something.

[panicker, 2017/02/07 00:45:05]

I think shared_ptr is the right thing here. We want to clean this up when ref
count goes to zero,
vs. making a manual raw pointer in ResourceFetcher and making assumptions about
its lifetime compared that of a PerformanceEntry.

+haraken could you chime in here?
before this patch: ResourceTimingInfo instance was completely owned by
platform/loader/fetch/ResourceFetcher.h and kept in ResourceFetcher as a
unique_ptr
With this patch, we want to share the ownership of this instance with
PerformanceNavigationTiming. This seems like a reasonable use of shared_ptr?

Also I noticed that core/ uses std::shared_ptr and rest of Source/ uses
scoped_refptr. What's the official recommendation for core/?

+}
+
 void ResourceFetcher::initializeRevalidation(
     ResourceRequest& revalidatingRequest,
     Resource* resource) {
   DCHECK(resource);
   DCHECK(memoryCache()->contains(resource));
   DCHECK(resource->isLoaded());
   DCHECK(resource->canUseCacheValidator());
   DCHECK(!resource->isCacheValidator());
   DCHECK(!context().isControlledByServiceWorker());
 
@@ skipping 441 matching lines @@
 }
 
 ArchiveResource* ResourceFetcher::createArchive(Resource* resource) {
   // Only the top-frame can load MHTML.
   if (!context().isMainFrame())
     return nullptr;
   m_archive = MHTMLArchive::create(resource->url(), resource->resourceBuffer());
   return m_archive ? m_archive->mainResource() : nullptr;
 }
 
-ResourceTimingInfo* ResourceFetcher::getNavigationTimingInfo() {
-  return m_navigationTimingInfo.get();
-}
-
 void ResourceFetcher::handleLoadCompletion(Resource* resource) {
   context().didLoadResource(resource);
 
   resource->reloadIfLoFiOrPlaceholderImage(this, Resource::kReloadIfNeeded);
 }
 
 void ResourceFetcher::handleLoaderFinish(Resource* resource,
                                          double finishTime,
                                          LoaderFinishType type) {
   DCHECK(resource);
@@ skipping 394 matching lines @@
   visitor->trace(m_context);
   visitor->trace(m_archive);
   visitor->trace(m_loaders);
   visitor->trace(m_nonBlockingLoaders);
   visitor->trace(m_documentResources);
   visitor->trace(m_preloads);
   visitor->trace(m_resourceTimingInfoMap);
 }
 
 }  // namespace blink