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

Unified Diff: chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc

Issue 2413663003: Expose GlobalRequestID in NavigationHandle and ResourceRequestInfo. (Closed)
Patch Set: Addressed comments 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 side-by-side diff with in-line comments
Download patch
Index: chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc
diff --git a/chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc b/chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc
index 1a745b2501a511c0222bf81e89164910f8bf6da1..0667709d048d2520126d4c0698f51e0161cd88a0 100644
--- a/chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc
+++ b/chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc
@@ -4,18 +4,37 @@
#include "chrome/browser/data_use_measurement/chrome_data_use_ascriber.h"
+#include "base/memory/ptr_util.h"
+#include "components/data_use_measurement/core/data_use_recorder.h"
+#include "components/data_use_measurement/core/data_use_user_data.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/resource_request_info.h"
+#include "net/url_request/url_request.h"
namespace data_use_measurement {
+// static
+const void* ChromeDataUseAscriber::DataUseRecorderEntryAsUserData::
+ kUserDataKey = static_cast<void*>(
+ &ChromeDataUseAscriber::DataUseRecorderEntryAsUserData::kUserDataKey);
+
+ChromeDataUseAscriber::DataUseRecorderEntryAsUserData::
+ DataUseRecorderEntryAsUserData(DataUseRecorderEntry entry)
+ : entry_(entry) {}
+
+ChromeDataUseAscriber::DataUseRecorderEntryAsUserData::
+ ~DataUseRecorderEntryAsUserData() {}
+
ChromeDataUseAscriber::ChromeDataUseAscriber() {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
}
ChromeDataUseAscriber::~ChromeDataUseAscriber() {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+ DCHECK_EQ(0u, data_use_recorders_.size());
}
DataUseRecorder* ChromeDataUseAscriber::GetDataUseRecorder(
@@ -24,11 +43,87 @@ DataUseRecorder* ChromeDataUseAscriber::GetDataUseRecorder(
return nullptr;
}
+void ChromeDataUseAscriber::OnBeforeUrlRequest(net::URLRequest* request) {
+ DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+ if (!request->url().SchemeIsHTTPOrHTTPS())
+ return;
+
+ auto service = static_cast<DataUseUserData*>(
+ request->GetUserData(DataUseUserData::kUserDataKey));
+ if (service)
+ return;
+
+ const content::ResourceRequestInfo* request_info =
+ content::ResourceRequestInfo::ForRequest(request);
+ content::ResourceType resource_type = request_info
+ ? request_info->GetResourceType()
+ : content::RESOURCE_TYPE_LAST_TYPE;
+
+ if (resource_type == content::RESOURCE_TYPE_MAIN_FRAME) {
RyanSturm 2016/11/02 21:41:58 nit: if (resource_type != content::RESOURCE_TYPE_M
Not at Google. Contact bengr 2016/11/07 22:17:17 Done.
+ int render_process_id = -1;
+ int render_frame_id = -1;
+ bool has_valid_render_frame_id_ =
+ content::ResourceRequestInfo::GetRenderFrameForRequest(
+ request, &render_process_id, &render_frame_id);
+ DCHECK(has_valid_render_frame_id_);
+ // Browser tests may not set up DataUseWebContentsObservers in which case
+ // this class never sees navigation and frame events so DataUseRecorders
+ // will never be destroyed. To avoid this, we ignore requests whose
+ // render frames don't have a record. However, this can also be caused by
+ // URLRequests racing the frame create events.
+ // TODO(kundaji): Add UMA.
+ if (render_frame_data_use_map_.find(
+ RenderFrameHostID(render_process_id, render_frame_id)) ==
+ render_frame_data_use_map_.end()) {
+ return;
+ }
+
+ auto user_data = static_cast<DataUseRecorderEntryAsUserData*>(
+ request->GetUserData(DataUseRecorderEntryAsUserData::kUserDataKey));
+ if (user_data)
+ return;
+
+ DataUseRecorderEntry entry = data_use_recorders_.insert(
+ data_use_recorders_.end(), base::MakeUnique<DataUseRecorder>());
+ request->SetUserData(DataUseRecorderEntryAsUserData::kUserDataKey,
+ new DataUseRecorderEntryAsUserData(entry));
+
+ pending_navigation_data_use_map_.insert(
+ std::make_pair(request_info->GetGlobalRequestID(), entry));
+ }
+}
+
+void ChromeDataUseAscriber::OnUrlRequestCompleted(net::URLRequest* request,
+ bool started) {
+ const content::ResourceRequestInfo* request_info =
+ content::ResourceRequestInfo::ForRequest(request);
+ content::ResourceType resource_type = request_info
+ ? request_info->GetResourceType()
+ : content::RESOURCE_TYPE_LAST_TYPE;
+
+ if (resource_type == content::RESOURCE_TYPE_MAIN_FRAME) {
RyanSturm 2016/11/02 21:41:58 nit: if (resource_type != content::RESOURCE_TYPE_M
Not at Google. Contact bengr 2016/11/07 22:17:17 In the current cl there is nothing being done for
+ // If request was not successful, then NavigationHandle in
+ // DidFinishMainFrameNavigation will not have GlobalRequestID. So we erase
+ // the DataUseRecorderEntry here.
+ if (!request->status().is_success()) {
+ DeletePendingNavigationEntry(request_info->GetGlobalRequestID());
+ }
+ }
+}
+
void ChromeDataUseAscriber::RenderFrameCreated(int render_process_id,
int render_frame_id,
int parent_render_process_id,
int parent_render_frame_id) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+ // TODO(kundaji): Point child render frames to the same DataUseRecorder as
+ // parent render frame.
+ DataUseRecorderEntry entry = data_use_recorders_.insert(
+ data_use_recorders_.end(), base::MakeUnique<DataUseRecorder>());
+ render_frame_data_use_map_.insert(std::make_pair(
+ RenderFrameHostID(render_process_id, render_frame_id), entry));
}
void ChromeDataUseAscriber::RenderFrameDeleted(int render_process_id,
@@ -36,6 +131,12 @@ void ChromeDataUseAscriber::RenderFrameDeleted(int render_process_id,
int parent_render_process_id,
int parent_render_frame_id) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+ RenderFrameHostID key(render_process_id, render_frame_id);
+ auto frame_iter = render_frame_data_use_map_.find(key);
+ DCHECK(frame_iter != render_frame_data_use_map_.end());
+ DataUseRecorderEntry entry = frame_iter->second;
+ render_frame_data_use_map_.erase(frame_iter);
+ data_use_recorders_.erase(entry);
}
void ChromeDataUseAscriber::DidStartMainFrameNavigation(
@@ -48,11 +149,14 @@ void ChromeDataUseAscriber::DidStartMainFrameNavigation(
void ChromeDataUseAscriber::DidFinishMainFrameNavigation(
GURL gurl,
+ content::GlobalRequestID global_request_id,
int render_process_id,
int render_frame_id,
bool is_same_page_navigation,
void* navigation_handle) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+ DeletePendingNavigationEntry(global_request_id);
}
void ChromeDataUseAscriber::DidRedirectMainFrameNavigation(
@@ -63,4 +167,17 @@ void ChromeDataUseAscriber::DidRedirectMainFrameNavigation(
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
}
+void ChromeDataUseAscriber::DeletePendingNavigationEntry(
+ content::GlobalRequestID global_request_id) {
+ auto navigation_iter =
+ pending_navigation_data_use_map_.find(global_request_id);
+ // Pending navigation entry will not be found if finish navigation
+ // raced the URLRequest.
+ if (navigation_iter != pending_navigation_data_use_map_.end()) {
+ auto entry = navigation_iter->second;
+ pending_navigation_data_use_map_.erase(navigation_iter);
+ data_use_recorders_.erase(entry);
+ }
+}
+
} // namespace data_use_measurement

Powered by Google App Engine
This is Rietveld 408576698