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

Side by Side Diff: chrome/browser/android/offline_pages/offline_page_tab_helper.cc

Issue 2452593003: Fix incorrectly reported UMA OfflinePages.AggregatedRequestResult (Closed)
Patch Set: Fix tests Created 4 years, 1 month 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 2016 The Chromium Authors. All rights reserved. 1 // Copyright 2016 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/android/offline_pages/offline_page_tab_helper.h" 5 #include "chrome/browser/android/offline_pages/offline_page_tab_helper.h"
6 6
7 #include "base/bind.h" 7 #include "base/bind.h"
8 #include "base/logging.h" 8 #include "base/logging.h"
9 #include "base/memory/ptr_util.h" 9 #include "base/memory/ptr_util.h"
10 #include "chrome/browser/android/offline_pages/offline_page_request_job.h" 10 #include "chrome/browser/android/offline_pages/offline_page_request_job.h"
(...skipping 74 matching lines...) Expand 10 before | Expand all | Expand 10 after
85 navigated_url, 85 navigated_url,
86 provisional_offline_info_.offline_page->url)); 86 provisional_offline_info_.offline_page->url));
87 offline_info_.offline_page = 87 offline_info_.offline_page =
88 std::move(provisional_offline_info_.offline_page); 88 std::move(provisional_offline_info_.offline_page);
89 offline_info_.offline_header = provisional_offline_info_.offline_header; 89 offline_info_.offline_header = provisional_offline_info_.offline_header;
90 offline_info_.is_offline_preview = 90 offline_info_.is_offline_preview =
91 provisional_offline_info_.is_offline_preview; 91 provisional_offline_info_.is_offline_preview;
92 } 92 }
93 provisional_offline_info_.Clear(); 93 provisional_offline_info_.Clear();
94 94
95 // If the offline page has been loaded successfully, nothing more to do.
96 net::Error error_code = navigation_handle->GetNetErrorCode();
97 if (error_code == net::OK)
98 return;
99
95 // We might be reloading the URL in order to fetch the offline page. 100 // We might be reloading the URL in order to fetch the offline page.
96 // * If successful, nothing to do. 101 // * If successful, nothing to do.
97 // * Otherwise, we're hitting error again. Bail out to avoid loop. 102 // * Otherwise, we're hitting error again. Bail out to avoid loop.
98 if (reloading_url_on_net_error_) 103 if (reloading_url_on_net_error_)
99 return; 104 return;
100 105
101 // When the navigation starts, the request might be intercepted to serve the 106 // When the navigation starts, the request might be intercepted to serve the
102 // offline content if the network is detected to be in disconnected or poor 107 // offline content if the network is detected to be in disconnected or poor
103 // conditions. This detection might not work for some cases, i.e., connected 108 // conditions. This detection might not work for some cases, i.e., connected
104 // to a hotspot or proxy that does not have network, and the navigation will 109 // to a hotspot or proxy that does not have network, and the navigation will
105 // eventually fail. To handle this, we will reload the page to force the 110 // eventually fail. To handle this, we will reload the page to force the
106 // offline interception if the error code matches the following list. 111 // offline interception if the error code matches the following list.
107 // Otherwise, the error page will be shown. 112 // Otherwise, the error page will be shown.
108 net::Error error_code = navigation_handle->GetNetErrorCode();
109 if (error_code != net::ERR_INTERNET_DISCONNECTED && 113 if (error_code != net::ERR_INTERNET_DISCONNECTED &&
110 error_code != net::ERR_NAME_NOT_RESOLVED && 114 error_code != net::ERR_NAME_NOT_RESOLVED &&
111 error_code != net::ERR_ADDRESS_UNREACHABLE && 115 error_code != net::ERR_ADDRESS_UNREACHABLE &&
112 error_code != net::ERR_PROXY_CONNECTION_FAILED) { 116 error_code != net::ERR_PROXY_CONNECTION_FAILED) {
113 // Do not report aborted error since the error page is not shown on this 117 // Do not report aborted error since the error page is not shown on this
114 // error. 118 // error.
115 if (error_code != net::ERR_ABORTED) { 119 if (error_code != net::ERR_ABORTED) {
116 OfflinePageRequestJob::ReportAggregatedRequestResult( 120 OfflinePageRequestJob::ReportAggregatedRequestResult(
117 OfflinePageRequestJob::AggregatedRequestResult::SHOW_NET_ERROR_PAGE); 121 OfflinePageRequestJob::AggregatedRequestResult::SHOW_NET_ERROR_PAGE);
118 } 122 }
(...skipping 48 matching lines...) Expand 10 before | Expand all | Expand 10 after
167 base::MakeUnique<OfflinePageItem>(offline_page); 171 base::MakeUnique<OfflinePageItem>(offline_page);
168 provisional_offline_info_.offline_header = offline_header; 172 provisional_offline_info_.offline_header = offline_header;
169 provisional_offline_info_.is_offline_preview = is_offline_preview; 173 provisional_offline_info_.is_offline_preview = is_offline_preview;
170 } 174 }
171 175
172 const OfflinePageItem* OfflinePageTabHelper::GetOfflinePageForTest() const { 176 const OfflinePageItem* OfflinePageTabHelper::GetOfflinePageForTest() const {
173 return provisional_offline_info_.offline_page.get(); 177 return provisional_offline_info_.offline_page.get();
174 } 178 }
175 179
176 } // namespace offline_pages 180 } // namespace offline_pages
OLDNEW
« no previous file with comments | « chrome/browser/android/offline_pages/offline_page_request_job_unittest.cc ('k') | tools/metrics/histograms/histograms.xml » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698