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

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

Index: third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
diff --git a/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp b/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
index da7d65d99f6abaa313a92c605a8a13a52b99c444..6f0f46397d605674f2279d5e17aefd9fb41e1264 100644
--- a/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
+++ b/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
@@ -653,6 +653,10 @@ void ResourceFetcher::initializeResourceRequest(
       (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) {
@@ -1114,10 +1118,6 @@ ArchiveResource* ResourceFetcher::createArchive(Resource* resource) {
   return m_archive ? m_archive->mainResource() : nullptr;
 }
 
-ResourceTimingInfo* ResourceFetcher::getNavigationTimingInfo() {
-  return m_navigationTimingInfo.get();
-}
-
 void ResourceFetcher::handleLoadCompletion(Resource* resource) {
   context().didLoadResource(resource);