Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(499)

Issue 196533013: Revert 256688 "Fix various issues in RedirectToFileResourceHandler." (Closed)

Created:
6 years, 9 months ago by cpu_(ooo_6.6-7.5)
Modified:
6 years, 9 months ago
Reviewers:
davidben
CC:
chromium-reviews
Visibility:
Public.

Description

Revert 256688 "Fix various issues in RedirectToFileResourceHandler." The linux asan bot went red ResourceDispatcherHostTest.DownloadToFile (run #1): [ RUN ] ResourceDispatcherHostTest.DownloadToFile [ OK ] ResourceDispatcherHostTest.DownloadToFile (212 ms) [----------] 1 test from ResourceDispatcherHostTest (212 ms total) [----------] Global test environment tear-down [==========] 1 test from 1 test case ran. (213 ms total) [ PASSED ] 1 test. YOU HAVE 5 DISABLED TESTS ================================================================= ==9125==ERROR: LeakSanitizer: detected memory leaks Direct leak of 16 byte(s) in 1 object(s) allocated from: #0 0x49be81 in operator new(unsigned long) /usr/local/google/work/chromium/src/third_party/llvm/projects/compiler-rt/lib/asan/asan_new_delete.cc:54 #1 0x23ce695 in content::ResourceDispatcherHostImpl::BeginRequest(int, ResourceHostMsg_Request const&, IPC::Message*, int) content/browser/loader/resource_dispatcher_host_impl.cc:1045 #2 0x23cba80 in OnRequestResource content/browser/loader/resource_dispatcher_host_impl.cc:879 #3 0x23cba80 in Dispatch\u003Ccontent::ResourceDispatcherHostImpl, content::ResourceDispatcherHostImpl, int, const ResourceHostMsg_Request &> content/common/resource_messages.h:305 #4 0x23cba80 in content::ResourceDispatcherHostImpl::OnMessageReceived(IPC::Message const&, content::ResourceMessageFilter*, bool*) content/browser/loader/resource_dispatcher_host_impl.cc:841 #5 0xe04954 in content::ResourceDispatcherHostTest_DownloadToFile_Test::TestBody() content/browser/loader/resource_dispatcher_host_unittest.cc:2840 #6 0x2c10c9a in HandleExceptionsInMethodIfSupported\u003Ctesting::Test, void> testing/gtest/src/gtest.cc:2045 #7 0x2c10c9a in testing::Test::Run() testing/gtest/src/gtest.cc:2061 #8 0x2c12dea in testing::TestInfo::Run() testing/gtest/src/gtest.cc:2237 #9 0x2c13bb3 in testing::TestCase::Run() testing/gtest/src/gtest.cc:2344 #10 0x2c24c6a in testing::internal::UnitTestImpl::RunAllTests() testing/gtest/src/gtest.cc:4065 #11 0x2c24250 in HandleExceptionsInMethodIfSupported\u003Ctesting::internal::UnitTestImpl, bool> testing/gtest/src/gtest.cc:2045 #12 0x2c24250 in testing::UnitTest::Run() testing/gtest/src/gtest.cc:3697 #13 0x2b9ed2c in RUN_ALL_TESTS testing/gtest/include/gtest/gtest.h:2231 #14 0x2b9ed2c in base::TestSuite::Run() base/test/test_suite.cc:213 #15 0x2b92bcb in Run base/callback.h:401 #16 0x2b92bcb in base::(anonymous namespace)::LaunchUnitTestsInternal(int, char**, base::Callback\u003Cint ()> const&, int) base/test/launcher/unit_test_launcher.cc:494 #17 0x199a83e in main content/test/run_all_unittests.cc:14 #18 0x7f6e898bf76c in __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226 SUMMARY: AddressSanitizer: 16 byte(s) leaked in 1 allocation(s). http://build.chromium.org/p/chromium.memory/builders/Linux%20ASan%2BLSan%20Tests%20%282%29/builds/409 > Fix various issues in RedirectToFileResourceHandler. > > - Handle errors in creating temporary files. > - Cancel on write failure instead of resuming. This used to work, but got > lost in some refactoring in r142108. > - Fix the OnResponseCompleted resume logic to account for partial writes. > - Don't lose write errors which occur after OnResponseCompleted is received. > - WeakPtrFactory goes after other members. > - OnWillStart was never forwarded to downstream handlers. > - Make sure the file is closed before deleting it. > > Also add a lot of machinery so we can better unit test this stack. > > BUG=316634, 347663 > TEST=ResourceDispatcherHostTest.DownloadToFile > ResourceDispatcherHostTest.RegisterDownloadedTempFile > ResourceLoaderTest.RedirectToFile* > TemporaryFileStreamTest.Basic > > Review URL: https://codereview.chromium.org/82273002 TBR=davidben@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=256704

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+128 lines, -1204 lines) Patch
M trunk/src/content/browser/loader/async_resource_handler.cc View 1 chunk +0 lines, -9 lines 0 comments Download
M trunk/src/content/browser/loader/redirect_to_file_resource_handler.h View 6 chunks +22 lines, -45 lines 0 comments Download
M trunk/src/content/browser/loader/redirect_to_file_resource_handler.cc View 8 chunks +52 lines, -157 lines 0 comments Download
M trunk/src/content/browser/loader/resource_dispatcher_host_impl.h View 1 chunk +1 line, -1 line 0 comments Download
M trunk/src/content/browser/loader/resource_dispatcher_host_impl.cc View 2 chunks +8 lines, -14 lines 0 comments Download
M trunk/src/content/browser/loader/resource_dispatcher_host_unittest.cc View 12 chunks +19 lines, -213 lines 0 comments Download
M trunk/src/content/browser/loader/resource_loader_unittest.cc View 9 chunks +9 lines, -346 lines 0 comments Download
M trunk/src/content/browser/loader/sync_resource_handler.cc View 1 chunk +0 lines, -12 lines 0 comments Download
D trunk/src/content/browser/loader/temporary_file_stream.h View 1 chunk +0 lines, -46 lines 0 comments Download
D trunk/src/content/browser/loader/temporary_file_stream.cc View 1 chunk +0 lines, -61 lines 0 comments Download
D trunk/src/content/browser/loader/temporary_file_stream_unittest.cc View 1 chunk +0 lines, -120 lines 0 comments Download
M trunk/src/content/content_browser.gypi View 1 chunk +0 lines, -2 lines 0 comments Download
M trunk/src/content/content_tests.gypi View 1 chunk +0 lines, -1 line 0 comments Download
M trunk/src/net/base/mock_file_stream.h View 4 chunks +7 lines, -40 lines 0 comments Download
M trunk/src/net/base/mock_file_stream.cc View 5 chunks +4 lines, -122 lines 0 comments Download
M trunk/src/net/url_request/url_request_test_job.h View 2 chunks +2 lines, -3 lines 0 comments Download
M trunk/src/net/url_request/url_request_test_job.cc View 2 chunks +4 lines, -12 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
cpu_(ooo_6.6-7.5)
6 years, 9 months ago (2014-03-12 23:59:22 UTC) #1
cpu_(ooo_6.6-7.5)
6 years, 9 months ago (2014-03-13 00:00:16 UTC) #2
Message was sent while issue was closed.
Committed patchset #1 manually as r256704 (tree was closed).

Powered by Google App Engine
This is Rietveld 408576698