Beef up ResourceLoader unittests |
||||
Issue descriptionResourceLoader has a grand total of 3 general unittests. As I'm significantly refactoring the code, we should have more tests against regressions. In particular, we should test every operation in the sync/async succes case, and sync/cancellation case. I'll also switch the tests over to using the TestResourceHandler used by the MIME sniffing/intercepting tests.
,
Nov 30 2016
[+rdsmith]: Confirmed: ResourceLoader is broken.
,
Nov 30 2016
And to make sure we're correctly fixing the brokenness, we should test all URLRequest failure cases - sync and async failures during all the major phases (Actually, only Read has sync failures, I believe, but those are the ones that are broken).
,
Nov 30 2016
,
Dec 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1d5e2e389193fc814291e64469a9b28d3412a9e9 commit 1d5e2e389193fc814291e64469a9b28d3412a9e9 Author: mmenke <mmenke@chromium.org> Date: Tue Dec 06 21:24:42 2016 Fix a pair of ResourceLoader cancellation/error bugs. Both of these would result in two completion messages being passed down the ResourceHandler chain. Also add a bunch of tests of various flows through ResourceLoader, one of which is disabled due to yet another bug (Which I'll fix in another CL), and make the ResourceLoader tests use the same TestResourceHandler that the MIME sniffing / intercepting ResourceHandler tests use. BUG= 669709 Review-Url: https://codereview.chromium.org/2543633004 Cr-Commit-Position: refs/heads/master@{#436732} [modify] https://crrev.com/1d5e2e389193fc814291e64469a9b28d3412a9e9/content/browser/loader/intercepting_resource_handler_unittest.cc [modify] https://crrev.com/1d5e2e389193fc814291e64469a9b28d3412a9e9/content/browser/loader/resource_loader.cc [modify] https://crrev.com/1d5e2e389193fc814291e64469a9b28d3412a9e9/content/browser/loader/resource_loader_unittest.cc [modify] https://crrev.com/1d5e2e389193fc814291e64469a9b28d3412a9e9/content/browser/loader/test_resource_handler.cc [modify] https://crrev.com/1d5e2e389193fc814291e64469a9b28d3412a9e9/content/browser/loader/test_resource_handler.h [modify] https://crrev.com/1d5e2e389193fc814291e64469a9b28d3412a9e9/net/url_request/url_request_test_job.cc [modify] https://crrev.com/1d5e2e389193fc814291e64469a9b28d3412a9e9/net/url_request/url_request_test_job.h
,
Dec 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ab650bf37d14c76a794fe42f7cd0739dc01b752a commit ab650bf37d14c76a794fe42f7cd0739dc01b752a Author: carlosk <carlosk@chromium.org> Date: Tue Dec 06 23:17:13 2016 Revert of Fix a pair of ResourceLoader cancellation/error bugs. (patchset #7 id:120001 of https://codereview.chromium.org/2543633004/ ) Reason for revert: Preemptively reverting because this seems to be the most likely cause of these webkit_tests crashing: http/tests/serviceworker/registration.html virtual/disable-mojo-service-worker/http/tests/serviceworker/register-link-element.html http/tests/serviceworker/chromium/register-error-messages.html virtual/mojo-loading/http/tests/serviceworker/register-link-element.html virtual/disable-mojo-service-worker/http/tests/serviceworker/registration.html virtual/service-worker-navigation-preload/http/tests/serviceworker/register-link-element.html virtual/service-worker-navigation-preload/http/tests/serviceworker/registration.html http/tests/serviceworker/register-link-element.html virtual/disable-mojo-service-worker/http/tests/serviceworker/chromium/register-error-messages.html virtual/service-worker-navigation-preload/http/tests/serviceworker/chromium/register-error-messages.html virtual/mojo-loading/http/tests/serviceworker/chromium/register-error-messages.html virtual/mojo-loading/http/tests/serviceworker/registration.html If this is not the actual root I'll re-revert it back. Original issue's description: > Fix a pair of ResourceLoader cancellation/error bugs. > > Both of these would result in two completion messages being passed > down the ResourceHandler chain. > > Also add a bunch of tests of various flows through ResourceLoader, one > of which is disabled due to yet another bug (Which I'll fix in another > CL), and make the ResourceLoader tests use the same TestResourceHandler > that the MIME sniffing / intercepting ResourceHandler tests use. > > BUG= 669709 > > Committed: https://crrev.com/1d5e2e389193fc814291e64469a9b28d3412a9e9 > Cr-Commit-Position: refs/heads/master@{#436732} TBR=rdsmith@chromium.org,mmenke@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 669709 Review-Url: https://codereview.chromium.org/2557433005 Cr-Commit-Position: refs/heads/master@{#436769} [modify] https://crrev.com/ab650bf37d14c76a794fe42f7cd0739dc01b752a/content/browser/loader/intercepting_resource_handler_unittest.cc [modify] https://crrev.com/ab650bf37d14c76a794fe42f7cd0739dc01b752a/content/browser/loader/resource_loader.cc [modify] https://crrev.com/ab650bf37d14c76a794fe42f7cd0739dc01b752a/content/browser/loader/resource_loader_unittest.cc [modify] https://crrev.com/ab650bf37d14c76a794fe42f7cd0739dc01b752a/content/browser/loader/test_resource_handler.cc [modify] https://crrev.com/ab650bf37d14c76a794fe42f7cd0739dc01b752a/content/browser/loader/test_resource_handler.h [modify] https://crrev.com/ab650bf37d14c76a794fe42f7cd0739dc01b752a/net/url_request/url_request_test_job.cc [modify] https://crrev.com/ab650bf37d14c76a794fe42f7cd0739dc01b752a/net/url_request/url_request_test_job.h
,
Dec 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ab650bf37d14c76a794fe42f7cd0739dc01b752a commit ab650bf37d14c76a794fe42f7cd0739dc01b752a Author: carlosk <carlosk@chromium.org> Date: Tue Dec 06 23:17:13 2016 Revert of Fix a pair of ResourceLoader cancellation/error bugs. (patchset #7 id:120001 of https://codereview.chromium.org/2543633004/ ) Reason for revert: Preemptively reverting because this seems to be the most likely cause of these webkit_tests crashing: http/tests/serviceworker/registration.html virtual/disable-mojo-service-worker/http/tests/serviceworker/register-link-element.html http/tests/serviceworker/chromium/register-error-messages.html virtual/mojo-loading/http/tests/serviceworker/register-link-element.html virtual/disable-mojo-service-worker/http/tests/serviceworker/registration.html virtual/service-worker-navigation-preload/http/tests/serviceworker/register-link-element.html virtual/service-worker-navigation-preload/http/tests/serviceworker/registration.html http/tests/serviceworker/register-link-element.html virtual/disable-mojo-service-worker/http/tests/serviceworker/chromium/register-error-messages.html virtual/service-worker-navigation-preload/http/tests/serviceworker/chromium/register-error-messages.html virtual/mojo-loading/http/tests/serviceworker/chromium/register-error-messages.html virtual/mojo-loading/http/tests/serviceworker/registration.html If this is not the actual root I'll re-revert it back. Original issue's description: > Fix a pair of ResourceLoader cancellation/error bugs. > > Both of these would result in two completion messages being passed > down the ResourceHandler chain. > > Also add a bunch of tests of various flows through ResourceLoader, one > of which is disabled due to yet another bug (Which I'll fix in another > CL), and make the ResourceLoader tests use the same TestResourceHandler > that the MIME sniffing / intercepting ResourceHandler tests use. > > BUG= 669709 > > Committed: https://crrev.com/1d5e2e389193fc814291e64469a9b28d3412a9e9 > Cr-Commit-Position: refs/heads/master@{#436732} TBR=rdsmith@chromium.org,mmenke@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 669709 Review-Url: https://codereview.chromium.org/2557433005 Cr-Commit-Position: refs/heads/master@{#436769} [modify] https://crrev.com/ab650bf37d14c76a794fe42f7cd0739dc01b752a/content/browser/loader/intercepting_resource_handler_unittest.cc [modify] https://crrev.com/ab650bf37d14c76a794fe42f7cd0739dc01b752a/content/browser/loader/resource_loader.cc [modify] https://crrev.com/ab650bf37d14c76a794fe42f7cd0739dc01b752a/content/browser/loader/resource_loader_unittest.cc [modify] https://crrev.com/ab650bf37d14c76a794fe42f7cd0739dc01b752a/content/browser/loader/test_resource_handler.cc [modify] https://crrev.com/ab650bf37d14c76a794fe42f7cd0739dc01b752a/content/browser/loader/test_resource_handler.h [modify] https://crrev.com/ab650bf37d14c76a794fe42f7cd0739dc01b752a/net/url_request/url_request_test_job.cc [modify] https://crrev.com/ab650bf37d14c76a794fe42f7cd0739dc01b752a/net/url_request/url_request_test_job.h
,
Dec 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/94f1bd9459e17972e8e3f6f51a3d10fbf4a94df7 commit 94f1bd9459e17972e8e3f6f51a3d10fbf4a94df7 Author: mmenke <mmenke@chromium.org> Date: Wed Dec 07 21:13:05 2016 ResourceLoader: Fix a bunch of double-cancellation/double-error notification cases. There are a number of cases where ResourceLoader ends up notifying ResourceHandlers twice of the same failure. This CL fixes them and adds some tests. It also makes URLRequest no longer post notification on sync read errors, which was causing double notifications in one of those cases. Also fixes two other URLRequest consumers that depended on that behavior. BUG= 669709 , 670400 Review-Url: https://codereview.chromium.org/2542843006 Cr-Commit-Position: refs/heads/master@{#437057} [modify] https://crrev.com/94f1bd9459e17972e8e3f6f51a3d10fbf4a94df7/content/browser/cache_storage/cache_storage_blob_to_disk_cache.cc [modify] https://crrev.com/94f1bd9459e17972e8e3f6f51a3d10fbf4a94df7/content/browser/loader/intercepting_resource_handler_unittest.cc [modify] https://crrev.com/94f1bd9459e17972e8e3f6f51a3d10fbf4a94df7/content/browser/loader/resource_loader.cc [modify] https://crrev.com/94f1bd9459e17972e8e3f6f51a3d10fbf4a94df7/content/browser/loader/resource_loader.h [modify] https://crrev.com/94f1bd9459e17972e8e3f6f51a3d10fbf4a94df7/content/browser/loader/resource_loader_unittest.cc [modify] https://crrev.com/94f1bd9459e17972e8e3f6f51a3d10fbf4a94df7/content/browser/loader/test_resource_handler.cc [modify] https://crrev.com/94f1bd9459e17972e8e3f6f51a3d10fbf4a94df7/content/browser/loader/test_resource_handler.h [modify] https://crrev.com/94f1bd9459e17972e8e3f6f51a3d10fbf4a94df7/net/url_request/url_request.cc [modify] https://crrev.com/94f1bd9459e17972e8e3f6f51a3d10fbf4a94df7/net/url_request/url_request_job.cc [modify] https://crrev.com/94f1bd9459e17972e8e3f6f51a3d10fbf4a94df7/net/url_request/url_request_job.h [modify] https://crrev.com/94f1bd9459e17972e8e3f6f51a3d10fbf4a94df7/net/url_request/url_request_test_job.cc [modify] https://crrev.com/94f1bd9459e17972e8e3f6f51a3d10fbf4a94df7/net/url_request/url_request_test_job.h [modify] https://crrev.com/94f1bd9459e17972e8e3f6f51a3d10fbf4a94df7/net/url_request/url_request_test_util.cc [modify] https://crrev.com/94f1bd9459e17972e8e3f6f51a3d10fbf4a94df7/net/url_request/url_request_test_util.h [modify] https://crrev.com/94f1bd9459e17972e8e3f6f51a3d10fbf4a94df7/net/url_request/url_request_unittest.cc [modify] https://crrev.com/94f1bd9459e17972e8e3f6f51a3d10fbf4a94df7/remoting/host/token_validator_base.cc [modify] https://crrev.com/94f1bd9459e17972e8e3f6f51a3d10fbf4a94df7/remoting/host/token_validator_factory_impl_unittest.cc
,
Dec 9 2016
Oops, forgot to link https://codereview.chromium.org/2552463002/ to this issue. |
||||
►
Sign in to add a comment |
||||
Comment 1 by mmenke@chromium.org
, Nov 30 2016