New issue
Advanced search Search tips

Issue 868021 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 20
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug

Blocking:
issue 821009



Sign in to add a comment

Migrate components/web_resource/resource_request_allowed_notifier.h to NetworkConnectionTracker

Project Member Reported by rmcelrath@chromium.org, Jul 26

Issue description

ResourceRequestAllowedNotifier currently uses net::NetworkChangeNotifier to receive network changes. 

With network service, that will need to be converted to using NetworkConnectionTracker's observer APIs.
 
Labels: OS-Windows
Owner: rmcelrath@chromium.org
Status: Assigned (was: Available)
Owner: ----
Status: Available (was: Assigned)
You can look at https://crrev.com/c/1171208 for an example of how to do this.
Owner: cduvall@chromium.org
Status: Started (was: Available)
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c

commit a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c
Author: Clark DuVall <cduvall@chromium.org>
Date: Tue Aug 21 18:28:02 2018

Migrate ResourceRequestAllowedNotifier to NetworkConnectionTracker

A getter is used for NetworkConnectionTracker because some services that
use ResourceRequestAllowedNotifier are initialized early in browser
startup (e.g. VariationsService), and only perform the initialization
of ResourceRequestAllowedNotifier later on the UI thread. The getter
allows us to run get the connection tracker at that point so we don't get
DCHECKs about being on the UI thread when running
content::GetNetworkConnectionTracker().

This also moves the NetworkConnectionTracker in ios/ from BrowserState to
ApplicationContext, which is available everywhere. It also matches non-IOS
usage more closely, since we have it as a global there.

Bug:  868021 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I130c6b47feb90f0f7f0776ccc65666414a1ae802
Reviewed-on: https://chromium-review.googlesource.com/1180360
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584849}
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/chrome/browser/chrome_browser_main_browsertest.cc
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/chrome/browser/metrics/chrome_metrics_services_manager_client.cc
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/chrome/browser/plugins/plugins_resource_service.cc
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/chrome/browser/translate/translate_manager_render_view_host_unittest.cc
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/chrome/browser/translate/translate_service.cc
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/chrome/browser/translate/translate_service.h
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/chrome/browser/translate/translate_service_unittest.cc
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/components/variations/service/variations_service.cc
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/components/variations/service/variations_service.h
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/components/variations/service/variations_service_unittest.cc
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/components/web_resource/BUILD.gn
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/components/web_resource/resource_request_allowed_notifier.cc
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/components/web_resource/resource_request_allowed_notifier.h
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/components/web_resource/resource_request_allowed_notifier_test_util.cc
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/components/web_resource/resource_request_allowed_notifier_test_util.h
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/components/web_resource/resource_request_allowed_notifier_unittest.cc
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/components/web_resource/web_resource_service.cc
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/components/web_resource/web_resource_service.h
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/components/web_resource/web_resource_service_unittest.cc
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/ios/chrome/browser/BUILD.gn
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/ios/chrome/browser/DEPS
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/ios/chrome/browser/application_context.h
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/ios/chrome/browser/application_context_impl.cc
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/ios/chrome/browser/application_context_impl.h
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/ios/chrome/browser/google/BUILD.gn
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/ios/chrome/browser/google/google_url_tracker_factory.cc
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/ios/chrome/browser/metrics/ios_chrome_metrics_services_manager_client.mm
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/ios/chrome/browser/translate/translate_service_ios.cc
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/ios/chrome/test/testing_application_context.h
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/ios/chrome/test/testing_application_context.mm
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/ios/web/DEPS
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/ios/web/browser_state.mm
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/ios/web/public/browser_state.h
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/ios/web_view/BUILD.gn
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/ios/web_view/internal/DEPS
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/ios/web_view/internal/app/application_context.cc
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/ios/web_view/internal/app/application_context.h
[modify] https://crrev.com/a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c/ios/web_view/internal/translate/web_view_translate_service.cc

Status: Fixed (was: Started)
Status: Assigned (was: Fixed)
Labels: -Proj-Servicification-Canary
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 22

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f31df567c2acf03257ba60ff5ebb49ed43150ae5

commit f31df567c2acf03257ba60ff5ebb49ed43150ae5
Author: Sky Malice <skym@chromium.org>
Date: Wed Aug 22 22:57:27 2018

Revert "Migrate ResourceRequestAllowedNotifier to NetworkConnectionTracker"

This reverts commit a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c.

Reason for revert: ChromeBrowserMainBrowserTest.VariationsServiceStartsRequestOnNetworkChange started flaking after this CL.

BUG:  876861 

Original change's description:
> Migrate ResourceRequestAllowedNotifier to NetworkConnectionTracker
> 
> A getter is used for NetworkConnectionTracker because some services that
> use ResourceRequestAllowedNotifier are initialized early in browser
> startup (e.g. VariationsService), and only perform the initialization
> of ResourceRequestAllowedNotifier later on the UI thread. The getter
> allows us to run get the connection tracker at that point so we don't get
> DCHECKs about being on the UI thread when running
> content::GetNetworkConnectionTracker().
> 
> This also moves the NetworkConnectionTracker in ios/ from BrowserState to
> ApplicationContext, which is available everywhere. It also matches non-IOS
> usage more closely, since we have it as a global there.
> 
> Bug:  868021 
> Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
> Change-Id: I130c6b47feb90f0f7f0776ccc65666414a1ae802
> Reviewed-on: https://chromium-review.googlesource.com/1180360
> Reviewed-by: Eugene But <eugenebut@chromium.org>
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
> Commit-Queue: Clark DuVall <cduvall@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#584849}

TBR=jam@chromium.org,eugenebut@chromium.org,rsesek@chromium.org,rmcelrath@chromium.org,cduvall@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  868021 
Change-Id: I62cf40b6b4661d1d93b385dd6502402f547aacc5
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Reviewed-on: https://chromium-review.googlesource.com/1185685
Reviewed-by: Sky Malice <skym@chromium.org>
Commit-Queue: Sky Malice <skym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585293}
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/chrome/browser/chrome_browser_main_browsertest.cc
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/chrome/browser/metrics/chrome_metrics_services_manager_client.cc
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/chrome/browser/plugins/plugins_resource_service.cc
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/chrome/browser/translate/translate_manager_render_view_host_unittest.cc
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/chrome/browser/translate/translate_service.cc
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/chrome/browser/translate/translate_service.h
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/chrome/browser/translate/translate_service_unittest.cc
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/components/variations/service/variations_service.cc
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/components/variations/service/variations_service.h
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/components/variations/service/variations_service_unittest.cc
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/components/web_resource/BUILD.gn
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/components/web_resource/resource_request_allowed_notifier.cc
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/components/web_resource/resource_request_allowed_notifier.h
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/components/web_resource/resource_request_allowed_notifier_test_util.cc
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/components/web_resource/resource_request_allowed_notifier_test_util.h
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/components/web_resource/resource_request_allowed_notifier_unittest.cc
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/components/web_resource/web_resource_service.cc
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/components/web_resource/web_resource_service.h
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/components/web_resource/web_resource_service_unittest.cc
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/ios/chrome/browser/BUILD.gn
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/ios/chrome/browser/DEPS
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/ios/chrome/browser/application_context.h
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/ios/chrome/browser/application_context_impl.cc
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/ios/chrome/browser/application_context_impl.h
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/ios/chrome/browser/google/BUILD.gn
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/ios/chrome/browser/google/google_url_tracker_factory.cc
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/ios/chrome/browser/metrics/ios_chrome_metrics_services_manager_client.mm
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/ios/chrome/browser/translate/translate_service_ios.cc
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/ios/chrome/test/testing_application_context.h
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/ios/chrome/test/testing_application_context.mm
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/ios/web/DEPS
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/ios/web/browser_state.mm
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/ios/web/public/browser_state.h
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/ios/web_view/BUILD.gn
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/ios/web_view/internal/DEPS
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/ios/web_view/internal/app/application_context.cc
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/ios/web_view/internal/app/application_context.h
[modify] https://crrev.com/f31df567c2acf03257ba60ff5ebb49ed43150ae5/ios/web_view/internal/translate/web_view_translate_service.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e00fddec92e49b1b57f5de2f1134697fd6b1c703

commit e00fddec92e49b1b57f5de2f1134697fd6b1c703
Author: Clark DuVall <cduvall@chromium.org>
Date: Thu Aug 23 03:50:06 2018

Reland "Migrate ResourceRequestAllowedNotifier to NetworkConnectionTracker"

This is a reland of a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c

ChromeBrowserMainBrowserTest.VariationsServiceStartsRequestOnNetworkChange
was flaky in the original change. Added logic in the test to wait for the
connection type change.

Only diffs from original are in chrome/browser/chrome_browser_main_browsertest.cc

Original change's description:
> Migrate ResourceRequestAllowedNotifier to NetworkConnectionTracker
>
> A getter is used for NetworkConnectionTracker because some services that
> use ResourceRequestAllowedNotifier are initialized early in browser
> startup (e.g. VariationsService), and only perform the initialization
> of ResourceRequestAllowedNotifier later on the UI thread. The getter
> allows us to run get the connection tracker at that point so we don't get
> DCHECKs about being on the UI thread when running
> content::GetNetworkConnectionTracker().
>
> This also moves the NetworkConnectionTracker in ios/ from BrowserState to
> ApplicationContext, which is available everywhere. It also matches non-IOS
> usage more closely, since we have it as a global there.
>
> Bug:  868021 
> Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
> Change-Id: I130c6b47feb90f0f7f0776ccc65666414a1ae802
> Reviewed-on: https://chromium-review.googlesource.com/1180360
> Reviewed-by: Eugene But <eugenebut@chromium.org>
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
> Commit-Queue: Clark DuVall <cduvall@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#584849}

TBR=eugenebut@chromium.org,rsesek@chromium.org,rmcelrath@chromium.org,jam@chromium.org

Bug:  868021 
Change-Id: I5941b72474657159f0d4a1e6667fd77a3c475887
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Reviewed-on: https://chromium-review.googlesource.com/1185602
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585387}
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/chrome/browser/chrome_browser_main_browsertest.cc
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/chrome/browser/metrics/chrome_metrics_services_manager_client.cc
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/chrome/browser/plugins/plugins_resource_service.cc
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/chrome/browser/translate/translate_manager_render_view_host_unittest.cc
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/chrome/browser/translate/translate_service.cc
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/chrome/browser/translate/translate_service.h
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/chrome/browser/translate/translate_service_unittest.cc
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/components/variations/service/variations_service.cc
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/components/variations/service/variations_service.h
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/components/variations/service/variations_service_unittest.cc
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/components/web_resource/BUILD.gn
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/components/web_resource/resource_request_allowed_notifier.cc
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/components/web_resource/resource_request_allowed_notifier.h
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/components/web_resource/resource_request_allowed_notifier_test_util.cc
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/components/web_resource/resource_request_allowed_notifier_test_util.h
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/components/web_resource/resource_request_allowed_notifier_unittest.cc
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/components/web_resource/web_resource_service.cc
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/components/web_resource/web_resource_service.h
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/components/web_resource/web_resource_service_unittest.cc
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/ios/chrome/browser/BUILD.gn
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/ios/chrome/browser/DEPS
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/ios/chrome/browser/application_context.h
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/ios/chrome/browser/application_context_impl.cc
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/ios/chrome/browser/application_context_impl.h
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/ios/chrome/browser/google/BUILD.gn
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/ios/chrome/browser/google/google_url_tracker_factory.cc
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/ios/chrome/browser/metrics/ios_chrome_metrics_services_manager_client.mm
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/ios/chrome/browser/translate/translate_service_ios.cc
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/ios/chrome/test/testing_application_context.h
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/ios/chrome/test/testing_application_context.mm
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/ios/web/DEPS
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/ios/web/browser_state.mm
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/ios/web/public/browser_state.h
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/ios/web_view/BUILD.gn
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/ios/web_view/internal/DEPS
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/ios/web_view/internal/app/application_context.cc
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/ios/web_view/internal/app/application_context.h
[modify] https://crrev.com/e00fddec92e49b1b57f5de2f1134697fd6b1c703/ios/web_view/internal/translate/web_view_translate_service.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Aug 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6

commit 8185f7dd60785a837ace8b35446cfe4e9d5a1bf6
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Thu Aug 23 11:47:23 2018

Revert "Reland "Migrate ResourceRequestAllowedNotifier to NetworkConnectionTracker""

This reverts commit e00fddec92e49b1b57f5de2f1134697fd6b1c703.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 585387 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2UwMGZkZGVjOTJlNDliMWI1N2Y1ZGUyZjExMzQ2OTdmZDZiMWM3MDMM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.memory/Linux%20ChromiumOS%20MSan%20Tests/8297

Sample Failed Step: browser_tests

Original change's description:
> Reland "Migrate ResourceRequestAllowedNotifier to NetworkConnectionTracker"
> 
> This is a reland of a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c
> 
> ChromeBrowserMainBrowserTest.VariationsServiceStartsRequestOnNetworkChange
> was flaky in the original change. Added logic in the test to wait for the
> connection type change.
> 
> Only diffs from original are in chrome/browser/chrome_browser_main_browsertest.cc
> 
> Original change's description:
> > Migrate ResourceRequestAllowedNotifier to NetworkConnectionTracker
> >
> > A getter is used for NetworkConnectionTracker because some services that
> > use ResourceRequestAllowedNotifier are initialized early in browser
> > startup (e.g. VariationsService), and only perform the initialization
> > of ResourceRequestAllowedNotifier later on the UI thread. The getter
> > allows us to run get the connection tracker at that point so we don't get
> > DCHECKs about being on the UI thread when running
> > content::GetNetworkConnectionTracker().
> >
> > This also moves the NetworkConnectionTracker in ios/ from BrowserState to
> > ApplicationContext, which is available everywhere. It also matches non-IOS
> > usage more closely, since we have it as a global there.
> >
> > Bug:  868021 
> > Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
> > Change-Id: I130c6b47feb90f0f7f0776ccc65666414a1ae802
> > Reviewed-on: https://chromium-review.googlesource.com/1180360
> > Reviewed-by: Eugene But <eugenebut@chromium.org>
> > Reviewed-by: Robert Sesek <rsesek@chromium.org>
> > Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> > Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
> > Commit-Queue: Clark DuVall <cduvall@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#584849}
> 
> TBR=eugenebut@chromium.org,rsesek@chromium.org,rmcelrath@chromium.org,jam@chromium.org
> 
> Bug:  868021 
> Change-Id: I5941b72474657159f0d4a1e6667fd77a3c475887
> Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
> Reviewed-on: https://chromium-review.googlesource.com/1185602
> Reviewed-by: Clark DuVall <cduvall@chromium.org>
> Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
> Commit-Queue: Clark DuVall <cduvall@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#585387}

Change-Id: Ie348785d70d6c9b7242a51f0370ee471f37c036d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  868021 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Reviewed-on: https://chromium-review.googlesource.com/1185922
Cr-Commit-Position: refs/heads/master@{#585452}
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/chrome/browser/chrome_browser_main_browsertest.cc
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/chrome/browser/metrics/chrome_metrics_services_manager_client.cc
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/chrome/browser/plugins/plugins_resource_service.cc
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/chrome/browser/translate/translate_manager_render_view_host_unittest.cc
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/chrome/browser/translate/translate_service.cc
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/chrome/browser/translate/translate_service.h
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/chrome/browser/translate/translate_service_unittest.cc
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/components/variations/service/variations_service.cc
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/components/variations/service/variations_service.h
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/components/variations/service/variations_service_unittest.cc
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/components/web_resource/BUILD.gn
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/components/web_resource/resource_request_allowed_notifier.cc
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/components/web_resource/resource_request_allowed_notifier.h
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/components/web_resource/resource_request_allowed_notifier_test_util.cc
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/components/web_resource/resource_request_allowed_notifier_test_util.h
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/components/web_resource/resource_request_allowed_notifier_unittest.cc
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/components/web_resource/web_resource_service.cc
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/components/web_resource/web_resource_service.h
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/components/web_resource/web_resource_service_unittest.cc
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/ios/chrome/browser/BUILD.gn
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/ios/chrome/browser/DEPS
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/ios/chrome/browser/application_context.h
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/ios/chrome/browser/application_context_impl.cc
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/ios/chrome/browser/application_context_impl.h
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/ios/chrome/browser/google/BUILD.gn
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/ios/chrome/browser/google/google_url_tracker_factory.cc
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/ios/chrome/browser/metrics/ios_chrome_metrics_services_manager_client.mm
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/ios/chrome/browser/translate/translate_service_ios.cc
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/ios/chrome/test/testing_application_context.h
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/ios/chrome/test/testing_application_context.mm
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/ios/web/DEPS
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/ios/web/browser_state.mm
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/ios/web/public/browser_state.h
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/ios/web_view/BUILD.gn
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/ios/web_view/internal/DEPS
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/ios/web_view/internal/app/application_context.cc
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/ios/web_view/internal/app/application_context.h
[modify] https://crrev.com/8185f7dd60785a837ace8b35446cfe4e9d5a1bf6/ios/web_view/internal/translate/web_view_translate_service.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Aug 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/34141782bc8b1e459167cbdafa9a18ef0c6294e2

commit 34141782bc8b1e459167cbdafa9a18ef0c6294e2
Author: Clark DuVall <cduvall@chromium.org>
Date: Thu Aug 23 18:58:58 2018

Reland "Reland "Migrate ResourceRequestAllowedNotifier to NetworkConnectionTracker""

This is a reland of e00fddec92e49b1b57f5de2f1134697fd6b1c703

Needed to initialize variables in ChromeBrowserMainBrowserTest for asan/msan.
Verified this works with msan build.

Original change's description:
> Reland "Migrate ResourceRequestAllowedNotifier to NetworkConnectionTracker"
>
> This is a reland of a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c
>
> ChromeBrowserMainBrowserTest.VariationsServiceStartsRequestOnNetworkChange
> was flaky in the original change. Added logic in the test to wait for the
> connection type change.
>
> Only diffs from original are in chrome/browser/chrome_browser_main_browsertest.cc
>
> Original change's description:
> > Migrate ResourceRequestAllowedNotifier to NetworkConnectionTracker
> >
> > A getter is used for NetworkConnectionTracker because some services that
> > use ResourceRequestAllowedNotifier are initialized early in browser
> > startup (e.g. VariationsService), and only perform the initialization
> > of ResourceRequestAllowedNotifier later on the UI thread. The getter
> > allows us to run get the connection tracker at that point so we don't get
> > DCHECKs about being on the UI thread when running
> > content::GetNetworkConnectionTracker().
> >
> > This also moves the NetworkConnectionTracker in ios/ from BrowserState to
> > ApplicationContext, which is available everywhere. It also matches non-IOS
> > usage more closely, since we have it as a global there.
> >
> > Bug:  868021 
> > Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
> > Change-Id: I130c6b47feb90f0f7f0776ccc65666414a1ae802
> > Reviewed-on: https://chromium-review.googlesource.com/1180360
> > Reviewed-by: Eugene But <eugenebut@chromium.org>
> > Reviewed-by: Robert Sesek <rsesek@chromium.org>
> > Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> > Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
> > Commit-Queue: Clark DuVall <cduvall@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#584849}
>
> TBR=eugenebut@chromium.org,rsesek@chromium.org,rmcelrath@chromium.org,jam@chromium.org
>
> Bug:  868021 
> Change-Id: I5941b72474657159f0d4a1e6667fd77a3c475887
> Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
> Reviewed-on: https://chromium-review.googlesource.com/1185602
> Reviewed-by: Clark DuVall <cduvall@chromium.org>
> Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
> Commit-Queue: Clark DuVall <cduvall@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#585387}

TBR=eugenebut@chromium.org,rsesek@chromium.org,rmcelrath@chromium.org,jam@chromium.org

Bug:  868021 ,  876861 
Change-Id: I46fccf072d0b3080603e97c73ff055ac7c45e723
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Reviewed-on: https://chromium-review.googlesource.com/1187081
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585565}
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/chrome/browser/chrome_browser_main_browsertest.cc
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/chrome/browser/metrics/chrome_metrics_services_manager_client.cc
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/chrome/browser/plugins/plugins_resource_service.cc
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/chrome/browser/translate/translate_manager_render_view_host_unittest.cc
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/chrome/browser/translate/translate_service.cc
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/chrome/browser/translate/translate_service.h
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/chrome/browser/translate/translate_service_unittest.cc
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/components/variations/service/variations_service.cc
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/components/variations/service/variations_service.h
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/components/variations/service/variations_service_unittest.cc
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/components/web_resource/BUILD.gn
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/components/web_resource/resource_request_allowed_notifier.cc
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/components/web_resource/resource_request_allowed_notifier.h
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/components/web_resource/resource_request_allowed_notifier_test_util.cc
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/components/web_resource/resource_request_allowed_notifier_test_util.h
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/components/web_resource/resource_request_allowed_notifier_unittest.cc
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/components/web_resource/web_resource_service.cc
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/components/web_resource/web_resource_service.h
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/components/web_resource/web_resource_service_unittest.cc
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/ios/chrome/browser/BUILD.gn
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/ios/chrome/browser/DEPS
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/ios/chrome/browser/application_context.h
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/ios/chrome/browser/application_context_impl.cc
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/ios/chrome/browser/application_context_impl.h
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/ios/chrome/browser/google/BUILD.gn
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/ios/chrome/browser/google/google_url_tracker_factory.cc
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/ios/chrome/browser/metrics/ios_chrome_metrics_services_manager_client.mm
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/ios/chrome/browser/translate/translate_service_ios.cc
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/ios/chrome/test/testing_application_context.h
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/ios/chrome/test/testing_application_context.mm
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/ios/web/DEPS
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/ios/web/browser_state.mm
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/ios/web/public/browser_state.h
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/ios/web_view/BUILD.gn
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/ios/web_view/internal/DEPS
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/ios/web_view/internal/app/application_context.cc
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/ios/web_view/internal/app/application_context.h
[modify] https://crrev.com/34141782bc8b1e459167cbdafa9a18ef0c6294e2/ios/web_view/internal/translate/web_view_translate_service.cc

Status: Fixed (was: Assigned)
Project Member

Comment 15 by bugdroid1@chromium.org, Aug 30

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/22b92b162796a4d67497685889b836faa9fba994

commit 22b92b162796a4d67497685889b836faa9fba994
Author: Clark DuVall <cduvall@chromium.org>
Date: Thu Aug 30 19:18:40 2018

Revert "Reland "Reland "Migrate ResourceRequestAllowedNotifier to NetworkConnectionTracker"""

This reverts commit 34141782bc8b1e459167cbdafa9a18ef0c6294e2.

Reason for revert: Caused Finch seed freshness regression, see  http://crbug.com/879252  and
 http://crbug.com/879115 

Original change's description:
> Reland "Reland "Migrate ResourceRequestAllowedNotifier to NetworkConnectionTracker""
>
> This is a reland of e00fddec92e49b1b57f5de2f1134697fd6b1c703
>
> Needed to initialize variables in ChromeBrowserMainBrowserTest for asan/msan.
> Verified this works with msan build.
>
> Original change's description:
> > Reland "Migrate ResourceRequestAllowedNotifier to NetworkConnectionTracker"
> >
> > This is a reland of a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c
> >
> > ChromeBrowserMainBrowserTest.VariationsServiceStartsRequestOnNetworkChange
> > was flaky in the original change. Added logic in the test to wait for the
> > connection type change.
> >
> > Only diffs from original are in chrome/browser/chrome_browser_main_browsertest.cc
> >
> > Original change's description:
> > > Migrate ResourceRequestAllowedNotifier to NetworkConnectionTracker
> > >
> > > A getter is used for NetworkConnectionTracker because some services that
> > > use ResourceRequestAllowedNotifier are initialized early in browser
> > > startup (e.g. VariationsService), and only perform the initialization
> > > of ResourceRequestAllowedNotifier later on the UI thread. The getter
> > > allows us to run get the connection tracker at that point so we don't get
> > > DCHECKs about being on the UI thread when running
> > > content::GetNetworkConnectionTracker().
> > >
> > > This also moves the NetworkConnectionTracker in ios/ from BrowserState to
> > > ApplicationContext, which is available everywhere. It also matches non-IOS
> > > usage more closely, since we have it as a global there.
> > >
> > > Bug:  868021 
> > > Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
> > > Change-Id: I130c6b47feb90f0f7f0776ccc65666414a1ae802
> > > Reviewed-on: https://chromium-review.googlesource.com/1180360
> > > Reviewed-by: Eugene But <eugenebut@chromium.org>
> > > Reviewed-by: Robert Sesek <rsesek@chromium.org>
> > > Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> > > Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
> > > Commit-Queue: Clark DuVall <cduvall@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#584849}
> >
> > TBR=eugenebut@chromium.org,rsesek@chromium.org,rmcelrath@chromium.org,jam@chromium.org
> >
> > Bug:  868021 
> > Change-Id: I5941b72474657159f0d4a1e6667fd77a3c475887
> > Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
> > Reviewed-on: https://chromium-review.googlesource.com/1185602
> > Reviewed-by: Clark DuVall <cduvall@chromium.org>
> > Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
> > Commit-Queue: Clark DuVall <cduvall@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#585387}
>
> TBR=eugenebut@chromium.org,rsesek@chromium.org,rmcelrath@chromium.org,jam@chromium.org
>
> Bug:  868021 ,  876861 
> Change-Id: I46fccf072d0b3080603e97c73ff055ac7c45e723
> Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
> Reviewed-on: https://chromium-review.googlesource.com/1187081
> Commit-Queue: Clark DuVall <cduvall@chromium.org>
> Reviewed-by: Clark DuVall <cduvall@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#585565}

TBR=jam@chromium.org,eugenebut@chromium.org,rsesek@chromium.org,rmcelrath@chromium.org,cduvall@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  868021 ,  876861 ,  879252 ,  879115 
Change-Id: If0417f53386a94de0ec3b64c283df123c673ec03
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs
Reviewed-on: https://chromium-review.googlesource.com/1197325
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587694}
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/chrome/browser/chrome_browser_main_browsertest.cc
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/chrome/browser/metrics/chrome_metrics_services_manager_client.cc
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/chrome/browser/plugins/plugins_resource_service.cc
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/chrome/browser/translate/translate_manager_render_view_host_unittest.cc
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/chrome/browser/translate/translate_service.cc
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/chrome/browser/translate/translate_service.h
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/chrome/browser/translate/translate_service_unittest.cc
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/components/variations/service/variations_service.cc
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/components/variations/service/variations_service.h
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/components/variations/service/variations_service_unittest.cc
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/components/web_resource/BUILD.gn
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/components/web_resource/resource_request_allowed_notifier.cc
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/components/web_resource/resource_request_allowed_notifier.h
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/components/web_resource/resource_request_allowed_notifier_test_util.cc
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/components/web_resource/resource_request_allowed_notifier_test_util.h
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/components/web_resource/resource_request_allowed_notifier_unittest.cc
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/components/web_resource/web_resource_service.cc
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/components/web_resource/web_resource_service.h
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/components/web_resource/web_resource_service_unittest.cc
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/ios/chrome/browser/BUILD.gn
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/ios/chrome/browser/DEPS
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/ios/chrome/browser/application_context.h
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/ios/chrome/browser/application_context_impl.cc
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/ios/chrome/browser/application_context_impl.h
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/ios/chrome/browser/google/BUILD.gn
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/ios/chrome/browser/google/google_url_tracker_factory.cc
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/ios/chrome/browser/metrics/ios_chrome_metrics_services_manager_client.mm
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/ios/chrome/browser/translate/translate_service_ios.cc
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/ios/chrome/test/testing_application_context.h
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/ios/chrome/test/testing_application_context.mm
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/ios/web/DEPS
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/ios/web/browser_state.mm
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/ios/web/public/browser_state.h
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/ios/web_view/BUILD.gn
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/ios/web_view/internal/DEPS
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/ios/web_view/internal/app/application_context.cc
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/ios/web_view/internal/app/application_context.h
[modify] https://crrev.com/22b92b162796a4d67497685889b836faa9fba994/ios/web_view/internal/translate/web_view_translate_service.cc

Status: Assigned (was: Fixed)
This was causing variations service to not fetch the variation seed, see  issue 879252  and  issue 879115 .
Project Member

Comment 18 by bugdroid1@chromium.org, Aug 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5bf72b12cff1b1a34d05e85dbacc71635550698b

commit 5bf72b12cff1b1a34d05e85dbacc71635550698b
Author: Clark DuVall <cduvall@chromium.org>
Date: Fri Aug 31 19:53:30 2018

Reland "Reland "Reland "Migrate ResourceRequestAllowedNotifier to NetworkConnectionTracker"""

This reverts commit 22b92b162796a4d67497685889b836faa9fba994.

Reason for revert: Fixed bug where listeners would not be notified when waiting for
initial connection status.

This change fixes the bug and adds a regression test. In addition, this puts the change
behind the ResourceRequestAllowedMigration feature, as suggested by asvitkine@. We can
test this on Canary to make sure the Variations.SeedFreshness metric is not affected.
Note that the Variations.ResourceRequestsAllowed will have different behavior with this
change, as now instead of just sending RESOURCE_REQUESTS_ALLOWED when the network is
connected, it will send RESOURCE_REQUESTS_NOT_ALLOWED_NETWORK_STATE_NOT_INITIALIZED and
then RESOURCE_REQUESTS_ALLOWED_NOTIFIED. This hopefully should not affect
Variations.SeedFreshness.

See diffs starting at Patchset 1 for changes from last reland. Some of the old code
needed to be added back to support both versions behind the feature flag.

Original change's description:
> Revert "Reland "Reland "Migrate ResourceRequestAllowedNotifier to NetworkConnectionTracker"""
>
> This reverts commit 34141782bc8b1e459167cbdafa9a18ef0c6294e2.
>
> Reason for revert: Caused Finch seed freshness regression, see  http://crbug.com/879252  and
>  http://crbug.com/879115 
>
> Original change's description:
> > Reland "Reland "Migrate ResourceRequestAllowedNotifier to NetworkConnectionTracker""
> >
> > This is a reland of e00fddec92e49b1b57f5de2f1134697fd6b1c703
> >
> > Needed to initialize variables in ChromeBrowserMainBrowserTest for asan/msan.
> > Verified this works with msan build.
> >
> > Original change's description:
> > > Reland "Migrate ResourceRequestAllowedNotifier to NetworkConnectionTracker"
> > >
> > > This is a reland of a9ed46b74b92dbd1e7ccf11deabfae7b0aed737c
> > >
> > > ChromeBrowserMainBrowserTest.VariationsServiceStartsRequestOnNetworkChange
> > > was flaky in the original change. Added logic in the test to wait for the
> > > connection type change.
> > >
> > > Only diffs from original are in chrome/browser/chrome_browser_main_browsertest.cc
> > >
> > > Original change's description:
> > > > Migrate ResourceRequestAllowedNotifier to NetworkConnectionTracker
> > > >
> > > > A getter is used for NetworkConnectionTracker because some services that
> > > > use ResourceRequestAllowedNotifier are initialized early in browser
> > > > startup (e.g. VariationsService), and only perform the initialization
> > > > of ResourceRequestAllowedNotifier later on the UI thread. The getter
> > > > allows us to run get the connection tracker at that point so we don't get
> > > > DCHECKs about being on the UI thread when running
> > > > content::GetNetworkConnectionTracker().
> > > >
> > > > This also moves the NetworkConnectionTracker in ios/ from BrowserState to
> > > > ApplicationContext, which is available everywhere. It also matches non-IOS
> > > > usage more closely, since we have it as a global there.
> > > >
> > > > Bug:  868021 
> > > > Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
> > > > Change-Id: I130c6b47feb90f0f7f0776ccc65666414a1ae802
> > > > Reviewed-on: https://chromium-review.googlesource.com/1180360
> > > > Reviewed-by: Eugene But <eugenebut@chromium.org>
> > > > Reviewed-by: Robert Sesek <rsesek@chromium.org>
> > > > Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> > > > Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
> > > > Commit-Queue: Clark DuVall <cduvall@chromium.org>
> > > > Cr-Commit-Position: refs/heads/master@{#584849}
> > >
> > > TBR=eugenebut@chromium.org,rsesek@chromium.org,rmcelrath@chromium.org,jam@chromium.org
> > >
> > > Bug:  868021 
> > > Change-Id: I5941b72474657159f0d4a1e6667fd77a3c475887
> > > Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
> > > Reviewed-on: https://chromium-review.googlesource.com/1185602
> > > Reviewed-by: Clark DuVall <cduvall@chromium.org>
> > > Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
> > > Commit-Queue: Clark DuVall <cduvall@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#585387}
> >
> > TBR=eugenebut@chromium.org,rsesek@chromium.org,rmcelrath@chromium.org,jam@chromium.org
> >
> > Bug:  868021 ,  876861 
> > Change-Id: I46fccf072d0b3080603e97c73ff055ac7c45e723
> > Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
> > Reviewed-on: https://chromium-review.googlesource.com/1187081
> > Commit-Queue: Clark DuVall <cduvall@chromium.org>
> > Reviewed-by: Clark DuVall <cduvall@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#585565}
>
> TBR=jam@chromium.org,eugenebut@chromium.org,rsesek@chromium.org,rmcelrath@chromium.org,cduvall@chromium.org
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug:  868021 ,  876861 ,  879252 ,  879115 
> Change-Id: If0417f53386a94de0ec3b64c283df123c673ec03
> Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs
> Reviewed-on: https://chromium-review.googlesource.com/1197325
> Commit-Queue: Clark DuVall <cduvall@chromium.org>
> Reviewed-by: Clark DuVall <cduvall@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#587694}

TBR=eugenebut@chromium.org

Bug:  868021 ,  876861 ,  879252 ,  879115 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I2190d82f8de1188763b6f1bb1de2b8762d23aa07
Reviewed-on: https://chromium-review.googlesource.com/1198102
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588133}
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/chrome/browser/chrome_browser_main_browsertest.cc
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/chrome/browser/metrics/chrome_metrics_services_manager_client.cc
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/chrome/browser/plugins/plugins_resource_service.cc
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/chrome/browser/translate/translate_manager_render_view_host_unittest.cc
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/chrome/browser/translate/translate_service.cc
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/chrome/browser/translate/translate_service.h
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/chrome/browser/translate/translate_service_unittest.cc
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/components/variations/service/variations_service.cc
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/components/variations/service/variations_service.h
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/components/variations/service/variations_service_unittest.cc
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/components/web_resource/BUILD.gn
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/components/web_resource/resource_request_allowed_notifier.cc
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/components/web_resource/resource_request_allowed_notifier.h
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/components/web_resource/resource_request_allowed_notifier_test_util.cc
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/components/web_resource/resource_request_allowed_notifier_test_util.h
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/components/web_resource/resource_request_allowed_notifier_unittest.cc
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/components/web_resource/web_resource_service.cc
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/components/web_resource/web_resource_service.h
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/components/web_resource/web_resource_service_unittest.cc
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/ios/chrome/browser/BUILD.gn
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/ios/chrome/browser/DEPS
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/ios/chrome/browser/application_context.h
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/ios/chrome/browser/application_context_impl.cc
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/ios/chrome/browser/application_context_impl.h
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/ios/chrome/browser/google/BUILD.gn
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/ios/chrome/browser/google/google_url_tracker_factory.cc
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/ios/chrome/browser/metrics/ios_chrome_metrics_services_manager_client.mm
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/ios/chrome/browser/translate/translate_service_ios.cc
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/ios/chrome/test/testing_application_context.h
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/ios/chrome/test/testing_application_context.mm
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/ios/web/DEPS
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/ios/web/browser_state.mm
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/ios/web/public/browser_state.h
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/ios/web_view/BUILD.gn
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/ios/web_view/internal/DEPS
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/ios/web_view/internal/app/application_context.cc
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/ios/web_view/internal/app/application_context.h
[modify] https://crrev.com/5bf72b12cff1b1a34d05e85dbacc71635550698b/ios/web_view/internal/translate/web_view_translate_service.cc

Labels: Hotlist-KnownIssue
Labels: -Hotlist-KnownIssue
Labels: ReleaseBlock-Stable
Project Member

Comment 22 by sheriffbot@chromium.org, Sep 7

Cc: dxie@chromium.org
This issue is marked as a release blocker with no milestone associated. Please add an appropriate milestone.

All release blocking issues should have milestones associated to it, so that the issue can tracked and the fixes can be pushed promptly.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -ReleaseBlock-Stable proj-
Labels: -proj- Proj-Servicification-Stable Hotlist-KnownIssue
Project Member

Comment 25 by bugdroid1@chromium.org, Sep 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b7d654bb9eacc70736ff401df7b908fa210192b6

commit b7d654bb9eacc70736ff401df7b908fa210192b6
Author: Clark DuVall <cduvall@chromium.org>
Date: Fri Sep 07 21:53:02 2018

Fix ResourceRequestsAllowedNotifier not being initialized on Android

VariationsService was not initializing ResourceRequestsAllowedNotifier
on the OS_ANDROID portion of the #ifdef, which was preventing it from
being notified of network changes.

This should fix the regressions in Variations.SeedFreshness on Android:
https://uma.googleplex.com/variations?sid=aeb0078452c432f2d6b3aa76b79354cd

Bug:  868021 
Change-Id: I40eaeea5dbd97e87c5ed16c7161781e3f7f67659
Reviewed-on: https://chromium-review.googlesource.com/1213316
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589679}
[modify] https://crrev.com/b7d654bb9eacc70736ff401df7b908fa210192b6/chrome/browser/chrome_browser_main.cc

Project Member

Comment 26 by bugdroid1@chromium.org, Sep 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/08ec83b03b7da77bff64fa5fc44f17c60ecb1fc9

commit 08ec83b03b7da77bff64fa5fc44f17c60ecb1fc9
Author: Clark DuVall <cduvall@chromium.org>
Date: Tue Sep 11 18:23:08 2018

Add enum description for RESOURCE_REQUESTS_NOT_ALLOWED_NETWORK_STATE_NOT_INITIALIZED

This enum was added in http://crrev.com/c/1198102

Bug:  868021 
Change-Id: I6f18a3b243a4c02385e160a31db767ae4bfe8f64
Reviewed-on: https://chromium-review.googlesource.com/1219897
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590410}
[modify] https://crrev.com/08ec83b03b7da77bff64fa5fc44f17c60ecb1fc9/components/variations/service/variations_service.cc
[modify] https://crrev.com/08ec83b03b7da77bff64fa5fc44f17c60ecb1fc9/tools/metrics/histograms/enums.xml

Labels: -Proj-Servicification-Stable
Project Member

Comment 28 by bugdroid1@chromium.org, Sep 20

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9a92e3962cca6363411dfcc8987dbed2872c4a5d

commit 9a92e3962cca6363411dfcc8987dbed2872c4a5d
Author: Clark DuVall <cduvall@chromium.org>
Date: Thu Sep 20 18:02:47 2018

Remove ResourceRequestAllowedMigration flag

This flag was used in a canary finch experiment to make sure there was
not a regression in Variations.SeedFreshness. The experiment shows no
change, so enabling the new behavior:
https://uma.googleplex.com/variations?sid=5356f4f970495339c96d6af50868accf

Bug:  868021 
Change-Id: I6f756be28a89226ef596c12cb09b032f2e672a75
Reviewed-on: https://chromium-review.googlesource.com/1222286
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592872}
[modify] https://crrev.com/9a92e3962cca6363411dfcc8987dbed2872c4a5d/chrome/browser/chrome_browser_main_browsertest.cc
[modify] https://crrev.com/9a92e3962cca6363411dfcc8987dbed2872c4a5d/components/web_resource/resource_request_allowed_notifier.cc
[modify] https://crrev.com/9a92e3962cca6363411dfcc8987dbed2872c4a5d/components/web_resource/resource_request_allowed_notifier.h
[modify] https://crrev.com/9a92e3962cca6363411dfcc8987dbed2872c4a5d/components/web_resource/resource_request_allowed_notifier_unittest.cc

Status: Fixed (was: Assigned)

Sign in to add a comment