Skip to content

Commit

Permalink
Disable data use ascriber if data saver is not enabled.
Browse files Browse the repository at this point in the history
This is guarded behind a field trial, and is the last killswitch option.

Bug: 781189
Change-Id: I22a25ff6a2629ca8b59aef727c54e2f491cb4771
Reviewed-on: https://chromium-review.googlesource.com/872074
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530449}
  • Loading branch information
tarunban authored and Commit Bot committed Jan 19, 2018
1 parent 020e72c commit 0dc72f9
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 0 deletions.
38 changes: 38 additions & 0 deletions chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@

namespace data_use_measurement {

const base::Feature kDisableAscriberIfDataSaverDisabled{
"DisableAscriberIfDataSaverDisabled", base::FEATURE_DISABLED_BY_DEFAULT};

// static
const void* const ChromeDataUseAscriber::DataUseRecorderEntryAsUserData::
kDataUseAscriberUserDataKey =
Expand Down Expand Up @@ -194,6 +197,9 @@ void ChromeDataUseAscriber::OnUrlRequestCompleted(net::URLRequest* request,
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
DCHECK(request);

if (IsDisabled())
return;

const DataUseRecorderEntry entry = GetDataUseRecorderEntry(request);

if (entry == data_use_recorders_.end())
Expand All @@ -214,6 +220,11 @@ void ChromeDataUseAscriber::OnUrlRequestCompletedOrDestroyed(
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
DCHECK(request);

if (IsDisabled())
return;

DCHECK(request);

const DataUseRecorderEntry entry = GetDataUseRecorderEntry(request);

if (entry == data_use_recorders_.end())
Expand Down Expand Up @@ -275,6 +286,8 @@ void ChromeDataUseAscriber::RenderFrameCreated(int render_process_id,
int main_render_process_id,
int main_render_frame_id) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
if (IsDisabled())
return;

const auto render_frame =
RenderFrameHostID(render_process_id, render_frame_id);
Expand Down Expand Up @@ -305,6 +318,9 @@ void ChromeDataUseAscriber::RenderFrameDeleted(int render_process_id,
int main_render_frame_id) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);

if (IsDisabled())
return;

RenderFrameHostID key(render_process_id, render_frame_id);

if (main_render_process_id == -1 && main_render_frame_id == -1) {
Expand All @@ -327,6 +343,8 @@ void ChromeDataUseAscriber::ReadyToCommitMainFrameNavigation(
int render_process_id,
int render_frame_id) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
if (IsDisabled())
return;

main_render_frame_entry_map_
.find(RenderFrameHostID(render_process_id, render_frame_id))
Expand All @@ -341,6 +359,8 @@ void ChromeDataUseAscriber::DidFinishMainFrameNavigation(
uint32_t page_transition,
base::TimeTicks time) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
if (IsDisabled())
return;

RenderFrameHostID main_frame(render_process_id, render_frame_id);

Expand Down Expand Up @@ -528,6 +548,8 @@ void ChromeDataUseAscriber::WasShownOrHidden(int main_render_process_id,
int main_render_frame_id,
bool visible) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
if (IsDisabled())
return;

auto main_frame_it = main_render_frame_entry_map_.find(
RenderFrameHostID(main_render_process_id, main_render_frame_id));
Expand All @@ -543,6 +565,8 @@ void ChromeDataUseAscriber::RenderFrameHostChanged(int old_render_process_id,
int new_render_process_id,
int new_render_frame_id) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
if (IsDisabled())
return;

auto old_frame_iter = main_render_frame_entry_map_.find(
RenderFrameHostID(old_render_process_id, old_render_frame_id));
Expand All @@ -563,4 +587,18 @@ void ChromeDataUseAscriber::RenderFrameHostChanged(int old_render_process_id,
}
}

bool ChromeDataUseAscriber::IsDisabled() const {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);

// TODO(rajendrant): https://crbug.com/753559. Fix platform specific race
// conditions and re-enable.
return base::FeatureList::IsEnabled(kDisableAscriberIfDataSaverDisabled) &&
disable_ascriber_;
}

void ChromeDataUseAscriber::DisableAscriber() {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
disable_ascriber_ = true;
}

} // namespace data_use_measurement
13 changes: 13 additions & 0 deletions chrome/browser/data_use_measurement/chrome_data_use_ascriber.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <tuple>
#include <utility>

#include "base/feature_list.h"
#include "base/hash.h"
#include "base/macros.h"
#include "base/supports_user_data.h"
Expand All @@ -27,6 +28,9 @@ class RenderFrameHost;

namespace data_use_measurement {

// Disables data use ascriber if data saver is disabled.
extern const base::Feature kDisableAscriberIfDataSaverDisabled;

class URLRequestClassifier;

// Browser implementation of DataUseAscriber. Maintains a list of
Expand Down Expand Up @@ -181,6 +185,11 @@ class ChromeDataUseAscriber : public DataUseAscriber {
void AscribeRecorderWithRequest(net::URLRequest* request,
DataUseRecorderEntry entry);

void DisableAscriber() override;

// Returns true if data use ascriber is disabled.
bool IsDisabled() const;

// Owner for all instances of DataUseRecorder. An instance is kept in this
// list if any entity (render frame hosts, URLRequests, pending navigations)
// that ascribe data use to the instance exists, and deleted when all
Expand All @@ -206,6 +215,10 @@ class ChromeDataUseAscriber : public DataUseAscriber {
// Detects heavy pages. Can be null when the feature is disabled.
std::unique_ptr<DataUseAscriber::PageLoadObserver> page_capping_observer_;

// True if the dtaa use ascriber should be disabled. The ascriber is enabled
// by default.
bool disable_ascriber_ = false;

DISALLOW_COPY_AND_ASSIGN(ChromeDataUseAscriber);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,11 @@ void DataReductionProxyIOData::SetDataUseAscriber(
DCHECK(data_use_ascriber);
data_use_observer_.reset(
new DataReductionProxyDataUseObserver(this, data_use_ascriber));

// Disable data use ascriber when data saver is not enabled.
if (!IsEnabled()) {
data_use_ascriber->DisableAscriber();
}
}

void DataReductionProxyIOData::SetPreviewsDecider(
Expand Down
2 changes: 2 additions & 0 deletions components/data_use_measurement/core/data_use_ascriber.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,6 @@ void DataUseAscriber::OnUrlRequestDestroyed(net::URLRequest* request) {
recorder->OnUrlRequestDestroyed(request);
}

void DataUseAscriber::DisableAscriber() {}

} // namespace data_use_measurement
3 changes: 3 additions & 0 deletions components/data_use_measurement/core/data_use_ascriber.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ class DataUseAscriber {
virtual void OnUrlRequestCompleted(net::URLRequest* request, bool started);
virtual void OnUrlRequestDestroyed(net::URLRequest* request);

// Disables data use ascriber.
virtual void DisableAscriber();

protected:
base::ObserverList<PageLoadObserver> observers_;

Expand Down

0 comments on commit 0dc72f9

Please sign in to comment.