Commit 07631d12 authored by Thomas Anderson's avatar Thomas Anderson Committed by Commit Bot

Revert "Remove navigation start observing from DataUseAscriber"

This reverts commit 73722a14.

Reason for revert: Breaking the Linux x64 build
https://ci.chromium.org/buildbot/chromium/Linux%20x64/55031

Original change's description:
> Remove navigation start observing from DataUseAscriber
> 
> There's no reason to PostTask a bunch of data to the IO thread to do a
> no-op method.
> 
> This CL should change no behavior.
> 
> Bug: 792524
> Change-Id: Ia0f88da99c72fa3369b164a2fd25352f77d13544
> Reviewed-on: https://chromium-review.googlesource.com/827980
> Reviewed-by: rajendrant <rajendrant@chromium.org>
> Commit-Queue: rajendrant <rajendrant@chromium.org>
> Commit-Queue: Charlie Harrison <csharrison@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#524424}

TBR=csharrison@chromium.org,rajendrant@chromium.org

Change-Id: I00a16f8e2ee77b767f4289ad01ca4a05c711fdc9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 792524
Reviewed-on: https://chromium-review.googlesource.com/830473Reviewed-by: default avatarThomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524431}
parent 665f9dec
...@@ -299,6 +299,14 @@ void ChromeDataUseAscriber::RenderFrameDeleted(int render_process_id, ...@@ -299,6 +299,14 @@ void ChromeDataUseAscriber::RenderFrameDeleted(int render_process_id,
subframe_to_mainframe_map_.erase(key); subframe_to_mainframe_map_.erase(key);
} }
void ChromeDataUseAscriber::DidStartMainFrameNavigation(
const GURL& gurl,
int render_process_id,
int render_frame_id,
void* navigation_handle) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
}
void ChromeDataUseAscriber::ReadyToCommitMainFrameNavigation( void ChromeDataUseAscriber::ReadyToCommitMainFrameNavigation(
content::GlobalRequestID global_request_id, content::GlobalRequestID global_request_id,
int render_process_id, int render_process_id,
......
...@@ -79,6 +79,12 @@ class ChromeDataUseAscriber : public DataUseAscriber { ...@@ -79,6 +79,12 @@ class ChromeDataUseAscriber : public DataUseAscriber {
int main_render_process_id, int main_render_process_id,
int main_render_frame_id); int main_render_frame_id);
// Called when a main frame navigation is started.
void DidStartMainFrameNavigation(const GURL& gurl,
int render_process_id,
int render_frame_id,
void* navigation_handle);
// Called when a main frame navigation is ready to be committed in a // Called when a main frame navigation is ready to be committed in a
// renderer. // renderer.
void ReadyToCommitMainFrameNavigation( void ReadyToCommitMainFrameNavigation(
......
...@@ -123,6 +123,24 @@ void ChromeDataUseAscriberService::RenderFrameDeleted( ...@@ -123,6 +123,24 @@ void ChromeDataUseAscriberService::RenderFrameDeleted(
main_render_frame_id)); main_render_frame_id));
} }
void ChromeDataUseAscriberService::DidStartNavigation(
content::NavigationHandle* navigation_handle) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (!navigation_handle->IsInMainFrame())
return;
if (!ascriber_)
return;
content::WebContents* web_contents = navigation_handle->GetWebContents();
content::BrowserThread::PostTask(
content::BrowserThread::IO, FROM_HERE,
base::BindOnce(&ChromeDataUseAscriber::DidStartMainFrameNavigation,
base::Unretained(ascriber_), navigation_handle->GetURL(),
web_contents->GetMainFrame()->GetProcess()->GetID(),
web_contents->GetMainFrame()->GetRoutingID(),
navigation_handle));
}
void ChromeDataUseAscriberService::ReadyToCommitNavigation( void ChromeDataUseAscriberService::ReadyToCommitNavigation(
content::NavigationHandle* navigation_handle) { content::NavigationHandle* navigation_handle) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
......
...@@ -45,6 +45,11 @@ class ChromeDataUseAscriberService : public KeyedService { ...@@ -45,6 +45,11 @@ class ChromeDataUseAscriberService : public KeyedService {
// are propagated. // are propagated.
void RenderFrameDeleted(content::RenderFrameHost* render_frame_host); void RenderFrameDeleted(content::RenderFrameHost* render_frame_host);
// Called when a navigation is started. Propagates main frame navigation
// start to the |ascriber_| on the IO thread. NavigationHandle methods
// cannot be called on the IO thread, so the pointer is cast to void*.
void DidStartNavigation(content::NavigationHandle* navigation_handle);
// Called when the navigation is ready to be committed in a renderer. // Called when the navigation is ready to be committed in a renderer.
// Propagates the event to the |ascriber_| on the IO thread. NavigationHandle // Propagates the event to the |ascriber_| on the IO thread. NavigationHandle
// methods cannot be called on the IO thread, so the pointer is cast to void*. // methods cannot be called on the IO thread, so the pointer is cast to void*.
......
...@@ -220,7 +220,11 @@ TEST_F(ChromeDataUseAscriberTest, MainFrameNavigation) { ...@@ -220,7 +220,11 @@ TEST_F(ChromeDataUseAscriberTest, MainFrameNavigation) {
ascriber()->OnBeforeUrlRequest(request.get()); ascriber()->OnBeforeUrlRequest(request.get());
EXPECT_EQ(2u, recorders().size()); EXPECT_EQ(2u, recorders().size());
// Navigation commits. // Navigation starts.
ascriber()->DidStartMainFrameNavigation(GURL("http://test.com"),
kRenderProcessId, kRenderFrameId,
kNavigationHandle);
ascriber()->ReadyToCommitMainFrameNavigation( ascriber()->ReadyToCommitMainFrameNavigation(
content::GlobalRequestID(kRenderProcessId, 0), kRenderProcessId, content::GlobalRequestID(kRenderProcessId, 0), kRenderProcessId,
kRenderFrameId); kRenderFrameId);
...@@ -258,6 +262,9 @@ TEST_F(ChromeDataUseAscriberTest, SubResourceRequestsAttributed) { ...@@ -258,6 +262,9 @@ TEST_F(ChromeDataUseAscriberTest, SubResourceRequestsAttributed) {
ascriber()->OnBeforeUrlRequest(page_load_a_main_frame_request.get()); ascriber()->OnBeforeUrlRequest(page_load_a_main_frame_request.get());
// Commit the page load. // Commit the page load.
ascriber()->DidStartMainFrameNavigation(GURL("http://test.com"),
kRenderProcessId, kRenderFrameId,
kNavigationHandle);
ascriber()->ReadyToCommitMainFrameNavigation( ascriber()->ReadyToCommitMainFrameNavigation(
content::GlobalRequestID(kRenderProcessId, 0), kRenderProcessId, content::GlobalRequestID(kRenderProcessId, 0), kRenderProcessId,
kRenderFrameId); kRenderFrameId);
...@@ -276,6 +283,9 @@ TEST_F(ChromeDataUseAscriberTest, SubResourceRequestsAttributed) { ...@@ -276,6 +283,9 @@ TEST_F(ChromeDataUseAscriberTest, SubResourceRequestsAttributed) {
ascriber()->OnBeforeUrlRequest(page_load_b_main_frame_request.get()); ascriber()->OnBeforeUrlRequest(page_load_b_main_frame_request.get());
// Commit the second page load. // Commit the second page load.
ascriber()->DidStartMainFrameNavigation(GURL("http://test_2.com"),
kRenderProcessId, kRenderFrameId,
kNavigationHandle);
ascriber()->ReadyToCommitMainFrameNavigation( ascriber()->ReadyToCommitMainFrameNavigation(
content::GlobalRequestID(kRenderProcessId, 0), kRenderProcessId, content::GlobalRequestID(kRenderProcessId, 0), kRenderProcessId,
kRenderFrameId); kRenderFrameId);
...@@ -323,6 +333,9 @@ TEST_F(ChromeDataUseAscriberTest, SubResourceRequestsAfterNavigationFinish) { ...@@ -323,6 +333,9 @@ TEST_F(ChromeDataUseAscriberTest, SubResourceRequestsAfterNavigationFinish) {
// First page load 'a'. // First page load 'a'.
ascriber()->RenderFrameCreated(kRenderProcessId, kRenderFrameId, -1, -1); ascriber()->RenderFrameCreated(kRenderProcessId, kRenderFrameId, -1, -1);
ascriber()->OnBeforeUrlRequest(page_load_a_mainresource.get()); ascriber()->OnBeforeUrlRequest(page_load_a_mainresource.get());
ascriber()->DidStartMainFrameNavigation(GURL("http://test.com"),
kRenderProcessId, kRenderFrameId,
kNavigationHandle);
ascriber()->ReadyToCommitMainFrameNavigation( ascriber()->ReadyToCommitMainFrameNavigation(
content::GlobalRequestID(kRenderProcessId, 0), kRenderProcessId, content::GlobalRequestID(kRenderProcessId, 0), kRenderProcessId,
kRenderFrameId); kRenderFrameId);
...@@ -353,6 +366,9 @@ TEST_F(ChromeDataUseAscriberTest, SubResourceRequestsAfterNavigationFinish) { ...@@ -353,6 +366,9 @@ TEST_F(ChromeDataUseAscriberTest, SubResourceRequestsAfterNavigationFinish) {
// Second page load 'b' on the same main render frame. // Second page load 'b' on the same main render frame.
ascriber()->OnBeforeUrlRequest(page_load_b_mainresource.get()); ascriber()->OnBeforeUrlRequest(page_load_b_mainresource.get());
ascriber()->DidStartMainFrameNavigation(GURL("http://test_2.com"),
kRenderProcessId, kRenderFrameId,
kNavigationHandle);
ascriber()->ReadyToCommitMainFrameNavigation( ascriber()->ReadyToCommitMainFrameNavigation(
content::GlobalRequestID(kRenderProcessId, 0), kRenderProcessId, content::GlobalRequestID(kRenderProcessId, 0), kRenderProcessId,
kRenderFrameId); kRenderFrameId);
...@@ -390,6 +406,9 @@ TEST_F(ChromeDataUseAscriberTest, SubResourceRequestsAfterNavigationFinish) { ...@@ -390,6 +406,9 @@ TEST_F(ChromeDataUseAscriberTest, SubResourceRequestsAfterNavigationFinish) {
// Third page load 'c' on the same main render frame with // Third page load 'c' on the same main render frame with
// same_document_navigation set. // same_document_navigation set.
ascriber()->OnBeforeUrlRequest(page_load_c_mainresource.get()); ascriber()->OnBeforeUrlRequest(page_load_c_mainresource.get());
ascriber()->DidStartMainFrameNavigation(GURL("http://test_c.com"),
kRenderProcessId, kRenderFrameId,
kNavigationHandle);
ascriber()->ReadyToCommitMainFrameNavigation( ascriber()->ReadyToCommitMainFrameNavigation(
content::GlobalRequestID(kRenderProcessId, 0), kRenderProcessId, content::GlobalRequestID(kRenderProcessId, 0), kRenderProcessId,
kRenderFrameId); kRenderFrameId);
...@@ -450,7 +469,11 @@ TEST_F(ChromeDataUseAscriberTest, PageLoadObserverNotified) { ...@@ -450,7 +469,11 @@ TEST_F(ChromeDataUseAscriberTest, PageLoadObserverNotified) {
ascriber()->OnBeforeUrlRequest(request.get()); ascriber()->OnBeforeUrlRequest(request.get());
// Navigation starts and is ready to commit. // Navigation starts.
ascriber()->DidStartMainFrameNavigation(GURL("http://test.com"),
kRenderProcessId, kRenderFrameId,
kNavigationHandle);
ascriber()->ReadyToCommitMainFrameNavigation( ascriber()->ReadyToCommitMainFrameNavigation(
content::GlobalRequestID(kRenderProcessId, 0), kRenderProcessId, content::GlobalRequestID(kRenderProcessId, 0), kRenderProcessId,
kRenderFrameId); kRenderFrameId);
......
...@@ -64,6 +64,11 @@ void DataUseWebContentsObserver::RenderFrameDeleted( ...@@ -64,6 +64,11 @@ void DataUseWebContentsObserver::RenderFrameDeleted(
service_->RenderFrameDeleted(render_frame_host); service_->RenderFrameDeleted(render_frame_host);
} }
void DataUseWebContentsObserver::DidStartNavigation(
content::NavigationHandle* navigation_handle) {
service_->DidStartNavigation(navigation_handle);
}
void DataUseWebContentsObserver::ReadyToCommitNavigation( void DataUseWebContentsObserver::ReadyToCommitNavigation(
content::NavigationHandle* navigation_handle) { content::NavigationHandle* navigation_handle) {
service_->ReadyToCommitNavigation(navigation_handle); service_->ReadyToCommitNavigation(navigation_handle);
......
...@@ -35,6 +35,8 @@ class DataUseWebContentsObserver ...@@ -35,6 +35,8 @@ class DataUseWebContentsObserver
// WebContentsObserver implementation: // WebContentsObserver implementation:
void RenderFrameCreated(content::RenderFrameHost* render_frame_host) override; void RenderFrameCreated(content::RenderFrameHost* render_frame_host) override;
void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) override; void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) override;
void DidStartNavigation(
content::NavigationHandle* navigation_handle) override;
void ReadyToCommitNavigation( void ReadyToCommitNavigation(
content::NavigationHandle* navigation_handle) override; content::NavigationHandle* navigation_handle) override;
void WasShown() override; void WasShown() override;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment