Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1043)

Unified Diff: chrome/browser/page_load_metrics/page_load_tracker.cc

Issue 2859393002: Report page load timing information for child frames. (Closed)
Patch Set: add browsertests Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/page_load_metrics/page_load_tracker.cc
diff --git a/chrome/browser/page_load_metrics/page_load_tracker.cc b/chrome/browser/page_load_metrics/page_load_tracker.cc
index c735bb78cd41cbe810ef448a7a3c8edab70814b7..4a5d162fe644bcf3d5609ee32df36876181590d7 100644
--- a/chrome/browser/page_load_metrics/page_load_tracker.cc
+++ b/chrome/browser/page_load_metrics/page_load_tracker.cc
@@ -18,6 +18,7 @@
#include "chrome/common/page_load_metrics/page_load_timing.h"
#include "content/public/browser/navigation_details.h"
#include "content/public/browser/navigation_handle.h"
+#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_contents_observer.h"
#include "content/public/common/browser_side_navigation_policy.h"
@@ -511,6 +512,18 @@ void PageLoadTracker::DidFinishSubFrameNavigation(
content::NavigationHandle* navigation_handle) {
INVOKE_AND_PRUNE_OBSERVERS(observers_, OnDidFinishSubFrameNavigation,
navigation_handle);
+
+ // todo we don't want to erase if it's an aborted load (prev doc remains
+ // committed in that case)
+ child_frame_navigation_start_.erase(navigation_handle->GetFrameTreeNodeId());
+ if (navigation_handle->HasCommitted() &&
jkarlin 2017/05/05 19:19:25 Note that error pages count as commits.
Bryan McQuade 2017/05/07 19:32:11 good catch, thanks!. I decided that for tracking t
+ !navigation_handle->IsSameDocument()) {
+ DCHECK_GE(navigation_handle->NavigationStart(), navigation_start_);
+ base::TimeDelta navigation_delta =
+ navigation_handle->NavigationStart() - navigation_start_;
+ child_frame_navigation_start_.insert(std::make_pair(
+ navigation_handle->GetFrameTreeNodeId(), navigation_delta));
+ }
}
void PageLoadTracker::FailedProvisionalLoad(
@@ -561,7 +574,32 @@ void PageLoadTracker::NotifyClientRedirectTo(
}
}
-void PageLoadTracker::UpdateChildFrameMetadata(
+void PageLoadTracker::UpdateSubFrameTiming(
+ content::RenderFrameHost* render_frame_host,
+ const PageLoadTiming& new_timing,
+ const PageLoadMetadata& new_metadata) {
+ UpdateSubFrameMetadata(new_metadata);
+ const auto it = child_frame_navigation_start_.find(
+ render_frame_host->GetFrameTreeNodeId());
+ if (it == child_frame_navigation_start_.end()) {
+ // note that this can happen if a navigation starts but aborts and then the
+ // frame contents is updated via e.g. document.open/document.write from its
+ // parent. we probably want this to pass through to observers, but it's
jkarlin 2017/05/05 19:19:25 This can also happen if a navigation aborts and fa
Bryan McQuade 2017/05/07 19:32:10 Yes - I fixed our tracking of loads in DidFinishSu
+ // unclear what we should use for a nav start value (what is the nav start
+ // value in the render process? maybe we should be sending the nav start
+ // timeticks from the renderer?)
+ // todo log an error here to keep track of frequency of this case
+ return;
+ }
+ const PageLoadExtraInfo info = ComputePageLoadExtraInfo();
+ for (const auto& observer : observers_) {
+ observer->OnSubFrameTimingUpdate(render_frame_host->GetFrameTreeNodeId(),
+ it->second, new_timing, new_metadata,
+ info);
+ }
+}
+
+void PageLoadTracker::UpdateSubFrameMetadata(
const PageLoadMetadata& child_metadata) {
// Merge the child loading behavior flags with any we've already observed,
// possibly from other child frames.

Powered by Google App Engine
This is Rietveld 408576698