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

Side by Side Diff: chrome/browser/page_load_metrics/metrics_web_contents_observer.cc

Issue 2380773002: Prevent tracking metrics for cases such as 204s and downloads. (Closed)
Patch Set: use existing test file Created 4 years, 2 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 unified diff | Download patch
OLDNEW
1 // Copyright 2015 The Chromium Authors. All rights reserved. 1 // Copyright 2015 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "chrome/browser/page_load_metrics/metrics_web_contents_observer.h" 5 #include "chrome/browser/page_load_metrics/metrics_web_contents_observer.h"
6 6
7 #include <algorithm> 7 #include <algorithm>
8 #include <ostream> 8 #include <ostream>
9 #include <string> 9 #include <string>
10 #include <utility> 10 #include <utility>
(...skipping 832 matching lines...) Expand 10 before | Expand all | Expand 10 after
843 std::move(provisional_loads_[navigation_handle])); 843 std::move(provisional_loads_[navigation_handle]));
844 provisional_loads_.erase(navigation_handle); 844 provisional_loads_.erase(navigation_handle);
845 845
846 // Ignore same-page navigations. 846 // Ignore same-page navigations.
847 if (navigation_handle->HasCommitted() && navigation_handle->IsSamePage()) { 847 if (navigation_handle->HasCommitted() && navigation_handle->IsSamePage()) {
848 if (finished_nav) 848 if (finished_nav)
849 finished_nav->StopTracking(); 849 finished_nav->StopTracking();
850 return; 850 return;
851 } 851 }
852 852
853 // Ignore internally generated aborts for navigations with HTTP responses that
854 // don't commit, such as HTTP 204 responses and downloads.
855 if (!navigation_handle->HasCommitted() &&
856 navigation_handle->GetNetErrorCode() == net::ERR_ABORTED &&
857 navigation_handle->GetResponseHeaders()) {
858 if (finished_nav)
859 finished_nav->StopTracking();
860 return;
861 }
862
853 const bool should_track = 863 const bool should_track =
854 finished_nav && ShouldTrackNavigation(navigation_handle); 864 finished_nav && ShouldTrackNavigation(navigation_handle);
855 865
856 if (finished_nav && !should_track) 866 if (finished_nav && !should_track)
857 finished_nav->StopTracking(); 867 finished_nav->StopTracking();
858 868
859 if (navigation_handle->HasCommitted()) { 869 if (navigation_handle->HasCommitted()) {
860 // Notify other loads that they may have been aborted by this committed 870 // Notify other loads that they may have been aborted by this committed
861 // load. is_certainly_browser_timestamp is set to false because 871 // load. is_certainly_browser_timestamp is set to false because
862 // NavigationStart() could be set in either the renderer or browser process. 872 // NavigationStart() could be set in either the renderer or browser process.
863 NotifyAbortAllLoadsWithTimestamp( 873 NotifyAbortAllLoadsWithTimestamp(
864 AbortTypeForPageTransition(navigation_handle->GetPageTransition()), 874 AbortTypeForPageTransition(navigation_handle->GetPageTransition()),
865 IsNavigationUserInitiated(navigation_handle), 875 IsNavigationUserInitiated(navigation_handle),
866 navigation_handle->NavigationStart(), false); 876 navigation_handle->NavigationStart(), false);
867 877
868 if (should_track) { 878 if (should_track) {
869 HandleCommittedNavigationForTrackedLoad(navigation_handle, 879 HandleCommittedNavigationForTrackedLoad(navigation_handle,
870 std::move(finished_nav)); 880 std::move(finished_nav));
871 } else { 881 } else {
872 committed_load_.reset(); 882 committed_load_.reset();
873 } 883 }
874 } else if (should_track) { 884 } else if (should_track) {
875 HandleFailedNavigationForTrackedLoad(navigation_handle, 885 HandleFailedNavigationForTrackedLoad(navigation_handle,
876 std::move(finished_nav)); 886 std::move(finished_nav));
877 } 887 }
878 } 888 }
879 889
880 // Handle a pre-commit error. Navigations that result in an error page will be 890 // Handle a pre-commit error. Navigations that result in an error page will be
881 // ignored. Note that downloads/204s will result in HasCommitted() returning 891 // ignored.
882 // false.
883 // TODO(csharrison): Track changes to NavigationHandle for signals when this is
884 // the case (HTTP response headers).
885 void MetricsWebContentsObserver::HandleFailedNavigationForTrackedLoad( 892 void MetricsWebContentsObserver::HandleFailedNavigationForTrackedLoad(
886 content::NavigationHandle* navigation_handle, 893 content::NavigationHandle* navigation_handle,
887 std::unique_ptr<PageLoadTracker> tracker) { 894 std::unique_ptr<PageLoadTracker> tracker) {
888 tracker->FailedProvisionalLoad(navigation_handle); 895 tracker->FailedProvisionalLoad(navigation_handle);
889 896
890 net::Error error = navigation_handle->GetNetErrorCode(); 897 net::Error error = navigation_handle->GetNetErrorCode();
891 898
892 // net::OK: This case occurs when the NavigationHandle finishes and reports 899 // net::OK: This case occurs when the NavigationHandle finishes and reports
893 // !HasCommitted(), but reports no net::Error. This should not occur 900 // !HasCommitted(), but reports no net::Error. This should not occur
894 // pre-PlzNavigate, but afterwards it should represent the navigation stopped 901 // pre-PlzNavigate, but afterwards it should represent the navigation stopped
895 // by the user before it was ready to commit. 902 // by the user before it was ready to commit.
896 // net::ERR_ABORTED: An aborted provisional load has error 903 // net::ERR_ABORTED: An aborted provisional load has error
897 // net::ERR_ABORTED. Note that this can come from some non user-initiated 904 // net::ERR_ABORTED.
898 // errors, such as downloads, or 204 responses. See crbug.com/542369.
899 if ((error == net::OK) || (error == net::ERR_ABORTED)) { 905 if ((error == net::OK) || (error == net::ERR_ABORTED)) {
900 tracker->NotifyAbort(ABORT_OTHER, false, base::TimeTicks::Now(), true); 906 tracker->NotifyAbort(ABORT_OTHER, false, base::TimeTicks::Now(), true);
901 aborted_provisional_loads_.push_back(std::move(tracker)); 907 aborted_provisional_loads_.push_back(std::move(tracker));
902 } 908 }
903 } 909 }
904 910
905 void MetricsWebContentsObserver::HandleCommittedNavigationForTrackedLoad( 911 void MetricsWebContentsObserver::HandleCommittedNavigationForTrackedLoad(
906 content::NavigationHandle* navigation_handle, 912 content::NavigationHandle* navigation_handle,
907 std::unique_ptr<PageLoadTracker> tracker) { 913 std::unique_ptr<PageLoadTracker> tracker) {
908 if (!navigation_handle->HasUserGesture() && 914 if (!navigation_handle->HasUserGesture() &&
(...skipping 185 matching lines...) Expand 10 before | Expand all | Expand 10 after
1094 content::NavigationHandle* navigation_handle) const { 1100 content::NavigationHandle* navigation_handle) const {
1095 DCHECK(navigation_handle->IsInMainFrame()); 1101 DCHECK(navigation_handle->IsInMainFrame());
1096 DCHECK(!navigation_handle->HasCommitted() || 1102 DCHECK(!navigation_handle->HasCommitted() ||
1097 !navigation_handle->IsSamePage()); 1103 !navigation_handle->IsSamePage());
1098 1104
1099 return BrowserPageTrackDecider(embedder_interface_.get(), web_contents(), 1105 return BrowserPageTrackDecider(embedder_interface_.get(), web_contents(),
1100 navigation_handle).ShouldTrack(); 1106 navigation_handle).ShouldTrack();
1101 } 1107 }
1102 1108
1103 } // namespace page_load_metrics 1109 } // namespace page_load_metrics
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698