New issue
Advanced search Search tips

Issue 876861 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 23
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: ----



Sign in to add a comment

browser_tests failing on chromium.win/Win 7 Tests x64 (1)

Project Member Reported by sheriff-...@appspot.gserviceaccount.com, Aug 22

Issue description

Filed by sheriff-o-matic@appspot.gserviceaccount.com on behalf of skym@chromium.org

browser_tests failing on chromium.win/Win 7 Tests x64 (1)

Builders failed on: 
- Win 7 Tests x64 (1): 
  https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win%207%20Tests%20x64%20%281%29


 
https://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=ChromeBrowserMainBrowserTest.VariationsServiceStartsRequestOnNetworkChange shows that things were green before ~1pm yesterday, at which point on windows things are ~half black/gray.
 Issue 876860  has been merged into this issue.
Project Member

Comment 5 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

 Issue 876929  has been merged into this issue.
Labels: -Sheriff-Chromium
Owner: cduvall@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 9 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 11 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

Project Member

Comment 12 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

Sign in to add a comment