Disallow implicit re-bind of unreferenced WeakPtrs to new threads, and multi-threaded GetWeakPtr() calls |
||||||
Issue descriptionThis bug tracks the code changes needed to exorcise WeakPtr raciness from Chromium, bringing it into compliance with stricter WeakPtr semantics.
,
Jun 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f7882372fcf69764a301ee0b2cee078490024821 commit f7882372fcf69764a301ee0b2cee078490024821 Author: Kevin Marshall <kmarshall@chromium.org> Date: Tue Jun 06 22:07:46 2017 Fix deletion threading on SpeechRecognitionManager. Fix deletion threading on SpeechRecognitionManager. This CL modifies BrowserMainLoop to delete SpeechRecognitionManager on the IO thread. Prior to this CL, SpeechRecognitionManager was deleted on the UI thread, which illegally combined IO thread dereferences with UI thread invalidation. This racy behavior triggered DCHECK failures by the stricter WeakPtr checks in CL 2908073007. R=jochen@chromium.org,tommi@chromium.org CC=wez@chromium.org Bug: 729716 Change-Id: I1d57c9417693fb9233766842dee1e8f729519599 Reviewed-on: https://chromium-review.googlesource.com/525152 Reviewed-by: Jochen Eisinger <jochen@chromium.org> Commit-Queue: Kevin Marshall <kmarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#477429} [modify] https://crrev.com/f7882372fcf69764a301ee0b2cee078490024821/content/browser/browser_main_loop.cc [modify] https://crrev.com/f7882372fcf69764a301ee0b2cee078490024821/content/browser/browser_main_loop.h [modify] https://crrev.com/f7882372fcf69764a301ee0b2cee078490024821/content/browser/speech/speech_recognition_manager_impl.cc [modify] https://crrev.com/f7882372fcf69764a301ee0b2cee078490024821/content/browser/speech/speech_recognition_manager_impl.h
,
Jun 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/eadbb7ef68a5e62351c8db043dabf09401db9ef7 commit eadbb7ef68a5e62351c8db043dabf09401db9ef7 Author: Kevin Marshall <kmarshall@chromium.org> Date: Thu Jun 22 00:02:22 2017 Fix PepperOutputProtectionMessageFilter WeakPtr issues Fix PepperOutputProtectionMessageFilter WeakPtr issues PepperOutputProtectionMessageFilter is illegally combining UI-thread WeakPtr dereferences with IO thread invalidations. This CL adds static functions which transition the execution flow to the IO thread before dereferencing the WeakPtr. These issues were caught by stricter WeakPtr semantics in CL 2908073007, which enforces sequence affinity regardless of the existence of outstanding WeakPtrs. R=bbudge@chromium.org CC=wez@chromium.org Bug: 729716 Change-Id: Iac60bf85aa8d5aad4bcb391392aa043572c2a5c7 Reviewed-on: https://chromium-review.googlesource.com/527759 Reviewed-by: Wez <wez@chromium.org> Reviewed-by: Bill Budge <bbudge@chromium.org> Commit-Queue: Kevin Marshall <kmarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#481362} [modify] https://crrev.com/eadbb7ef68a5e62351c8db043dabf09401db9ef7/chrome/browser/renderer_host/pepper/pepper_output_protection_message_filter.cc [modify] https://crrev.com/eadbb7ef68a5e62351c8db043dabf09401db9ef7/chrome/browser/renderer_host/pepper/pepper_output_protection_message_filter.h
,
Jun 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0ac8d89ee822c431ea66e0a7da762bb8c28b1877 commit 0ac8d89ee822c431ea66e0a7da762bb8c28b1877 Author: Kevin Marshall <kmarshall@chromium.org> Date: Fri Jun 23 00:13:33 2017 Fix thread safety issues with safe browsing DB. Fix thread safety issues with safe browsing DB. The Safe Browsing DB has some sequence consistency issues, which is caught by the stricter WeakPtr semantics detailed on CL 2908073007. * Delete SafeBrowsingDatabaseManager on IO thread. * Remove illegal dereferencing of IO-thread WeakPtr from DB thread in V4Database::VerifyChecksumOnTaskRunner * Remove non-threadsafe accesses of |io_thread_| resident members from the DB thread in V4LocalDatabaseManager. * Add IO-thread runloops to unit tests to handle database teardown. R=nparker@chromium.org CC=wez@chromium.org Bug: 729716 Change-Id: I1bc620d42aca2f1cc99e482b7776a628d783d390 Reviewed-on: https://chromium-review.googlesource.com/523983 Commit-Queue: Kevin Marshall <kmarshall@chromium.org> Reviewed-by: Charlie Harrison <csharrison@chromium.org> Reviewed-by: Greg Thompson <grt@chromium.org> Reviewed-by: Varun Khaneja <vakh@chromium.org> Cr-Commit-Position: refs/heads/master@{#481735} [modify] https://crrev.com/0ac8d89ee822c431ea66e0a7da762bb8c28b1877/chrome/browser/safe_browsing/incident_reporting/resource_request_detector_unittest.cc [modify] https://crrev.com/0ac8d89ee822c431ea66e0a7da762bb8c28b1877/components/safe_browsing_db/BUILD.gn [modify] https://crrev.com/0ac8d89ee822c431ea66e0a7da762bb8c28b1877/components/safe_browsing_db/database_manager.cc [modify] https://crrev.com/0ac8d89ee822c431ea66e0a7da762bb8c28b1877/components/safe_browsing_db/database_manager.h [modify] https://crrev.com/0ac8d89ee822c431ea66e0a7da762bb8c28b1877/components/safe_browsing_db/database_manager_unittest.cc [modify] https://crrev.com/0ac8d89ee822c431ea66e0a7da762bb8c28b1877/components/safe_browsing_db/v4_database.cc [modify] https://crrev.com/0ac8d89ee822c431ea66e0a7da762bb8c28b1877/components/safe_browsing_db/v4_database.h [modify] https://crrev.com/0ac8d89ee822c431ea66e0a7da762bb8c28b1877/components/safe_browsing_db/v4_local_database_manager.h [modify] https://crrev.com/0ac8d89ee822c431ea66e0a7da762bb8c28b1877/components/safe_browsing_db/whitelist_checker_client_unittest.cc [modify] https://crrev.com/0ac8d89ee822c431ea66e0a7da762bb8c28b1877/components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc
,
Jun 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1f4730efcd8f4c7152b69665bc96ad7f7c464667 commit 1f4730efcd8f4c7152b69665bc96ad7f7c464667 Author: Derek Cheng <imcheng@chromium.org> Date: Tue Jun 27 00:33:06 2017 Revert "Fix thread safety issues with safe browsing DB." This reverts commit 0ac8d89ee822c431ea66e0a7da762bb8c28b1877. Reason for revert: Seems to have broken RemoteDatabaseManagerTest on android - crbug.com/736924 Original change's description: > Fix thread safety issues with safe browsing DB. > > Fix thread safety issues with safe browsing DB. > The Safe Browsing DB has some sequence consistency issues, which > is caught by the stricter WeakPtr semantics detailed on CL 2908073007. > > * Delete SafeBrowsingDatabaseManager on IO thread. > * Remove illegal dereferencing of IO-thread WeakPtr from DB thread in > V4Database::VerifyChecksumOnTaskRunner > * Remove non-threadsafe accesses of |io_thread_| resident members from > the DB thread in V4LocalDatabaseManager. > * Add IO-thread runloops to unit tests to handle database teardown. > > > R=nparker@chromium.org > CC=wez@chromium.org > > Bug: 729716 > Change-Id: I1bc620d42aca2f1cc99e482b7776a628d783d390 > Reviewed-on: https://chromium-review.googlesource.com/523983 > Commit-Queue: Kevin Marshall <kmarshall@chromium.org> > Reviewed-by: Charlie Harrison <csharrison@chromium.org> > Reviewed-by: Greg Thompson <grt@chromium.org> > Reviewed-by: Varun Khaneja <vakh@chromium.org> > Cr-Commit-Position: refs/heads/master@{#481735} TBR=kmarshall@chromium.org,nparker@chromium.org,csharrison@chromium.org,vakh@chromium.org,grt@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 729716 Change-Id: Ia10145f404619bdde8f0348d5ab764fd29a59fbc Reviewed-on: https://chromium-review.googlesource.com/549073 Reviewed-by: Derek Cheng <imcheng@chromium.org> Commit-Queue: Derek Cheng <imcheng@chromium.org> Cr-Commit-Position: refs/heads/master@{#482489} [modify] https://crrev.com/1f4730efcd8f4c7152b69665bc96ad7f7c464667/chrome/browser/safe_browsing/incident_reporting/resource_request_detector_unittest.cc [modify] https://crrev.com/1f4730efcd8f4c7152b69665bc96ad7f7c464667/components/safe_browsing_db/BUILD.gn [modify] https://crrev.com/1f4730efcd8f4c7152b69665bc96ad7f7c464667/components/safe_browsing_db/database_manager.cc [modify] https://crrev.com/1f4730efcd8f4c7152b69665bc96ad7f7c464667/components/safe_browsing_db/database_manager.h [modify] https://crrev.com/1f4730efcd8f4c7152b69665bc96ad7f7c464667/components/safe_browsing_db/database_manager_unittest.cc [modify] https://crrev.com/1f4730efcd8f4c7152b69665bc96ad7f7c464667/components/safe_browsing_db/v4_database.cc [modify] https://crrev.com/1f4730efcd8f4c7152b69665bc96ad7f7c464667/components/safe_browsing_db/v4_database.h [modify] https://crrev.com/1f4730efcd8f4c7152b69665bc96ad7f7c464667/components/safe_browsing_db/v4_local_database_manager.h [modify] https://crrev.com/1f4730efcd8f4c7152b69665bc96ad7f7c464667/components/safe_browsing_db/whitelist_checker_client_unittest.cc [modify] https://crrev.com/1f4730efcd8f4c7152b69665bc96ad7f7c464667/components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc
,
Jul 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/32f99d7e474d48ae4b80918e453e80991388a450 commit 32f99d7e474d48ae4b80918e453e80991388a450 Author: Kevin Marshall <kmarshall@chromium.org> Date: Tue Jul 11 23:17:46 2017 Fix ChromeContentBrowserClient::AllowWorkerFileSystem threading. Fix ChromeContentBrowserClient::AllowWorkerFileSystem threading. AllowWorkerFileSystem() responds incorrectly on the UI thread after executing a permission check. This CL fixes the behavior by executing the callback on the caller's thread task runner. (This bug was caught by the stricter WeakPtr, see CL 2908073007.) R=nhiroki@chromium.org R=wez@chromium.org Bug: 729716 Change-Id: I35ad289dc16736a6e7e8e248a78b216030009dc5 Reviewed-on: https://chromium-review.googlesource.com/525614 Commit-Queue: Kevin Marshall <kmarshall@chromium.org> Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Cr-Commit-Position: refs/heads/master@{#485725} [modify] https://crrev.com/32f99d7e474d48ae4b80918e453e80991388a450/chrome/browser/chrome_content_browser_client.cc [modify] https://crrev.com/32f99d7e474d48ae4b80918e453e80991388a450/content/browser/shared_worker/shared_worker_host.cc
,
Jul 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e6af5cab1a0d4fde0e721bcdb5de52254679b27f commit e6af5cab1a0d4fde0e721bcdb5de52254679b27f Author: Kevin Marshall <kmarshall@chromium.org> Date: Fri Jul 14 23:52:07 2017 Reland "Fix thread safety issues with safe browsing DB." This is a reland of 0ac8d89ee822c431ea66e0a7da762bb8c28b1877 Original change's description: > Fix thread safety issues with safe browsing DB. > > Fix thread safety issues with safe browsing DB. > The Safe Browsing DB has some sequence consistency issues, which > is caught by the stricter WeakPtr semantics detailed on CL 2908073007. > > * Delete SafeBrowsingDatabaseManager on IO thread. > * Remove illegal dereferencing of IO-thread WeakPtr from DB thread in > V4Database::VerifyChecksumOnTaskRunner > * Remove non-threadsafe accesses of |io_thread_| resident members from > the DB thread in V4LocalDatabaseManager. > * Add IO-thread runloops to unit tests to handle database teardown. > > > R=nparker@chromium.org > CC=wez@chromium.org > > Bug: 729716 > Change-Id: I1bc620d42aca2f1cc99e482b7776a628d783d390 > Reviewed-on: https://chromium-review.googlesource.com/523983 > Commit-Queue: Kevin Marshall <kmarshall@chromium.org> > Reviewed-by: Charlie Harrison <csharrison@chromium.org> > Reviewed-by: Greg Thompson <grt@chromium.org> > Reviewed-by: Varun Khaneja <vakh@chromium.org> > Cr-Commit-Position: refs/heads/master@{#481735} TBR=csharrison@chromium.org, grt@chromium.org, vakh@chromium.org Bug: 729716, 742596 Change-Id: I4d67aa201c54be7a1e75a7375ac9861a29f1d87f Reviewed-on: https://chromium-review.googlesource.com/571684 Reviewed-by: Kevin Marshall <kmarshall@chromium.org> Commit-Queue: Kevin Marshall <kmarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#486946} [modify] https://crrev.com/e6af5cab1a0d4fde0e721bcdb5de52254679b27f/chrome/browser/safe_browsing/incident_reporting/resource_request_detector_unittest.cc [modify] https://crrev.com/e6af5cab1a0d4fde0e721bcdb5de52254679b27f/components/safe_browsing_db/BUILD.gn [modify] https://crrev.com/e6af5cab1a0d4fde0e721bcdb5de52254679b27f/components/safe_browsing_db/database_manager.cc [modify] https://crrev.com/e6af5cab1a0d4fde0e721bcdb5de52254679b27f/components/safe_browsing_db/database_manager.h [modify] https://crrev.com/e6af5cab1a0d4fde0e721bcdb5de52254679b27f/components/safe_browsing_db/database_manager_unittest.cc [modify] https://crrev.com/e6af5cab1a0d4fde0e721bcdb5de52254679b27f/components/safe_browsing_db/remote_database_manager_unittest.cc [modify] https://crrev.com/e6af5cab1a0d4fde0e721bcdb5de52254679b27f/components/safe_browsing_db/v4_database.cc [modify] https://crrev.com/e6af5cab1a0d4fde0e721bcdb5de52254679b27f/components/safe_browsing_db/v4_database.h [modify] https://crrev.com/e6af5cab1a0d4fde0e721bcdb5de52254679b27f/components/safe_browsing_db/v4_local_database_manager.h [modify] https://crrev.com/e6af5cab1a0d4fde0e721bcdb5de52254679b27f/components/safe_browsing_db/whitelist_checker_client_unittest.cc [modify] https://crrev.com/e6af5cab1a0d4fde0e721bcdb5de52254679b27f/components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc
,
Jul 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/46eb9acce044810fc62cc61711b9c1149a796051 commit 46eb9acce044810fc62cc61711b9c1149a796051 Author: Kevin Marshall <kmarshall@chromium.org> Date: Tue Jul 18 22:42:06 2017 Fix invalid WeakPtr usage in cert reporting test utils. Fix invalid WeakPtr usage in cert reporting test utils. CertReportJobInterceptor::MaybeInterceptRequest was posting IO thread- affine WeakPtrs onto the UI thread, which is an invalid use of WeakPtr. This CL replaces the use of WeakPtr with Unretained(). WeakPtr seems overkill for test code, as the test cases may block & join to ensure proper destruction ordering. R=vakh@chromium.org CC=wez@chromium.org TBR=vakh@chromium.org Bug: 729716 Change-Id: I99a07e1098d3a8341dbc2043ca2b335b9d97ef60 Reviewed-on: https://chromium-review.googlesource.com/527732 Commit-Queue: Kevin Marshall <kmarshall@chromium.org> Reviewed-by: Mustafa Emre Acer <meacer@chromium.org> Cr-Commit-Position: refs/heads/master@{#487637} [modify] https://crrev.com/46eb9acce044810fc62cc61711b9c1149a796051/chrome/browser/safe_browsing/certificate_reporting_service_test_utils.cc [modify] https://crrev.com/46eb9acce044810fc62cc61711b9c1149a796051/chrome/browser/safe_browsing/certificate_reporting_service_test_utils.h
,
Jul 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4e5ecdce95c6989b948f2c0febc23400fe99aad8 commit 4e5ecdce95c6989b948f2c0febc23400fe99aad8 Author: Kevin Marshall <kmarshall@chromium.org> Date: Thu Jul 20 19:16:37 2017 Fix threading bug in RemovableStorageProvider test path Fix threading bug in RemovableStorageProvider test path RemovableStorageProvider replies to GetAllDevices() queries on the FILE thread instead of the caller's thread for test cases. This fixes the issue by posting the reply to the correct thread. (Caught by the stricter WeakPtr impl; see CL 2908073007) R=sky@chromium.org CC=wez@chromium.org Bug: 729716 Change-Id: Ie0983e03ba91c8c09ba339ea6c7061de28e69030 Reviewed-on: https://chromium-review.googlesource.com/529864 Reviewed-by: Ken Rockot <rockot@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Kevin Marshall <kmarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#488346} [modify] https://crrev.com/4e5ecdce95c6989b948f2c0febc23400fe99aad8/chrome/browser/extensions/api/image_writer_private/removable_storage_provider.cc
,
Nov 2 2017
,
Nov 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/006c4e7c6e2cf8f9359347fc7458a3a7ed2de145 commit 006c4e7c6e2cf8f9359347fc7458a3a7ed2de145 Author: Wez <wez@chromium.org> Date: Mon Nov 06 20:03:41 2017 Use WeakPtr member in ImageController, rather than calling GetWeakPtr. ImageController was previously calling GetWeakPtr() when Bind()ing, which is potentially racey with InvalidateWeakPtrs() on the thread on which the WeakPtrs are actually used. Bug: 729716 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel Change-Id: I38ac1d5621703665d6b1c9deed915598fcd48ea9 Reviewed-on: https://chromium-review.googlesource.com/751764 Reviewed-by: vmpstr <vmpstr@chromium.org> Commit-Queue: Wez <wez@chromium.org> Cr-Commit-Position: refs/heads/master@{#514217} [modify] https://crrev.com/006c4e7c6e2cf8f9359347fc7458a3a7ed2de145/cc/tiles/image_controller.cc [modify] https://crrev.com/006c4e7c6e2cf8f9359347fc7458a3a7ed2de145/cc/tiles/image_controller.h
,
Jan 9 2018
Another example where implicit re-bind has masked broken threading is issue 800352 .
,
Feb 2 2018
+danakj, FYI :)
,
Feb 2 2018
Can you let the authors/reviewers of the class know?
,
Feb 2 2018
Not sure what you mean - do you literally mean everyone listed in WeakPtr's git blame..?
,
Feb 2 2018
Oh. I thot ur FYI is about the last CL in cc/. Nvm then :)
,
May 14 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/896fd5599817ac4c292439a3667661bbf6f3ac8b commit 896fd5599817ac4c292439a3667661bbf6f3ac8b Author: Wez <wez@chromium.org> Date: Mon May 14 16:07:32 2018 Fix ResourceDispatcherHostImpl to InvalidateWeakPtrs() on IO thread. ResourceDispatcherHostImpl is a UI-thread-owned class which also does work on the IO thread. It is built to intrinsically rely on being destroyed only after all threads but the main thread have been torn- down - the BrowserMainLoop invokes a Shutdown() API which posts a task to perform teardown of state which is used on the IO thread, which is thereby guaranteed to have run before the object is deleted. Internally a WeakPtrFactory is also used, to ensure that replies posted to the object on the IO thread are dropped if the object has already performed its IO-thread teardown. However, although the WeakPtrs were bound to tasks run on the IO thread, the factory itself was being left to implicitly InvalidateWeakPtrs() on destruction (i.e. on the UI thread). This CL adds explicit InvalidateWeakPtrs() on the IO thread, during the IO-thread teardown task. Bug: 729716 Change-Id: Idce31d52476df529dd5a877dc06747426c4e19c6 Reviewed-on: https://chromium-review.googlesource.com/1056255 Commit-Queue: Wez <wez@chromium.org> Reviewed-by: Charlie Harrison <csharrison@chromium.org> Reviewed-by: Matt Menke <mmenke@chromium.org> Cr-Commit-Position: refs/heads/master@{#558332} [modify] https://crrev.com/896fd5599817ac4c292439a3667661bbf6f3ac8b/content/browser/loader/resource_dispatcher_host_impl.cc [modify] https://crrev.com/896fd5599817ac4c292439a3667661bbf6f3ac8b/content/browser/loader/resource_dispatcher_host_impl.h
,
May 14 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3299f75f290091024c313578ac36eae39685c014 commit 3299f75f290091024c313578ac36eae39685c014 Author: Wez <wez@chromium.org> Date: Mon May 14 16:58:01 2018 Fix URLFetcherTest.SequencedTaskTest to run things on the Sequence. This test previously used the WaitingURLFetcherDelegate helper's APIs to run a RunLoop on the main thread, and wait for it to be quit by a fetch completing on the Sequence. However, this violated the constraint that the RunLoop may only be created, run and destroyed from the same thread. Bug: 729716 Change-Id: I4b699d17da7888527ef8d22f8e73a7f91236ef23 Reviewed-on: https://chromium-review.googlesource.com/1056265 Commit-Queue: Wez <wez@chromium.org> Reviewed-by: Matt Menke <mmenke@chromium.org> Cr-Commit-Position: refs/heads/master@{#558344} [modify] https://crrev.com/3299f75f290091024c313578ac36eae39685c014/net/url_request/url_fetcher_impl_unittest.cc
,
May 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6ff12055ca9bd6a0e7ecc1031ece2f0b1ed8eed5 commit 6ff12055ca9bd6a0e7ecc1031ece2f0b1ed8eed5 Author: Wez <wez@chromium.org> Date: Wed May 30 00:40:02 2018 Fix IndexedDB tests to release test transactions on correct sequence. Bug: 729716 Change-Id: I34c8914e7f9a0298fe64734f9ec0bfbf0fe9e4c3 Reviewed-on: https://chromium-review.googlesource.com/1075857 Reviewed-by: Wez <wez@chromium.org> Reviewed-by: Daniel Murphy <dmurph@chromium.org> Commit-Queue: Wez <wez@chromium.org> Cr-Commit-Position: refs/heads/master@{#562675} [modify] https://crrev.com/6ff12055ca9bd6a0e7ecc1031ece2f0b1ed8eed5/content/browser/indexed_db/indexed_db_backing_store_unittest.cc
,
May 31 2018
,
Jun 2 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7a5f1db86fda6c73b7362eb9e990c080c9bb6d55 commit 7a5f1db86fda6c73b7362eb9e990c080c9bb6d55 Author: Wez <wez@chromium.org> Date: Sat Jun 02 02:41:26 2018 Break down the NativeMediaFileUtil into IO and MediaTaskRunner parts. The NativeMediaFileUtil runs tasks on a TaskScheduler sequence provided by the MediaFileSystemBackend, but was itself owned and deleted on the IO thread, where its AsyncFileUtil API was used. The NativeMediaFileUtil is split into an outer part, used and owned on the IO thread, and a "core" containing the MediaPathFilter for use on the MediaFileSystemBackend's media TaskRunner. The MediaFileValidatorTests are also fixed, to tear-down the storage FileSystemContext before the browser's threads are torn-down. Bug: 729716 Change-Id: I339fe68f28d1bf6330ec4d341d2730df26bc4b30 Reviewed-on: https://chromium-review.googlesource.com/1072965 Commit-Queue: Wez <wez@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Cr-Commit-Position: refs/heads/master@{#563918} [modify] https://crrev.com/7a5f1db86fda6c73b7362eb9e990c080c9bb6d55/chrome/browser/media_galleries/fileapi/media_file_system_backend.cc [modify] https://crrev.com/7a5f1db86fda6c73b7362eb9e990c080c9bb6d55/chrome/browser/media_galleries/fileapi/media_file_validator_browsertest.cc [modify] https://crrev.com/7a5f1db86fda6c73b7362eb9e990c080c9bb6d55/chrome/browser/media_galleries/fileapi/native_media_file_util.cc [modify] https://crrev.com/7a5f1db86fda6c73b7362eb9e990c080c9bb6d55/chrome/browser/media_galleries/fileapi/native_media_file_util.h
,
Jun 4 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4603361e99c204ae881ffd20f5c07b320c646249 commit 4603361e99c204ae881ffd20f5c07b320c646249 Author: Dominic Battré <battre@chromium.org> Date: Mon Jun 04 07:55:10 2018 Revert "Break down the NativeMediaFileUtil into IO and MediaTaskRunner parts." This reverts commit 7a5f1db86fda6c73b7362eb9e990c080c9bb6d55. Strongly expecting this to be the cause of crbug.com/849171 . First flakes happened when this CL landed. Original change's description: > Break down the NativeMediaFileUtil into IO and MediaTaskRunner parts. > > The NativeMediaFileUtil runs tasks on a TaskScheduler sequence provided > by the MediaFileSystemBackend, but was itself owned and deleted on the > IO thread, where its AsyncFileUtil API was used. > > The NativeMediaFileUtil is split into an outer part, used and owned on > the IO thread, and a "core" containing the MediaPathFilter for use on > the MediaFileSystemBackend's media TaskRunner. > > The MediaFileValidatorTests are also fixed, to tear-down the storage > FileSystemContext before the browser's threads are torn-down. > > Bug: 729716 > Change-Id: I339fe68f28d1bf6330ec4d341d2730df26bc4b30 > Reviewed-on: https://chromium-review.googlesource.com/1072965 > Commit-Queue: Wez <wez@chromium.org> > Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> > Cr-Commit-Position: refs/heads/master@{#563918} TBR=wez@chromium.org,kinuko@chromium.org,tzik@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 729716, 849171 Change-Id: I0034caf2868bbc94565e883a94e3d0bac2e6af33 Reviewed-on: https://chromium-review.googlesource.com/1084491 Reviewed-by: Dominic Battré <battre@chromium.org> Commit-Queue: Dominic Battré <battre@chromium.org> Cr-Commit-Position: refs/heads/master@{#564011} [modify] https://crrev.com/4603361e99c204ae881ffd20f5c07b320c646249/chrome/browser/media_galleries/fileapi/media_file_system_backend.cc [modify] https://crrev.com/4603361e99c204ae881ffd20f5c07b320c646249/chrome/browser/media_galleries/fileapi/media_file_validator_browsertest.cc [modify] https://crrev.com/4603361e99c204ae881ffd20f5c07b320c646249/chrome/browser/media_galleries/fileapi/native_media_file_util.cc [modify] https://crrev.com/4603361e99c204ae881ffd20f5c07b320c646249/chrome/browser/media_galleries/fileapi/native_media_file_util.h
,
Jun 4 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a08664ba834ee3eb9461e1db04c00741710831cf commit a08664ba834ee3eb9461e1db04c00741710831cf Author: Wez <wez@chromium.org> Date: Mon Jun 04 20:09:26 2018 Reland "Break down the NativeMediaFileUtil into IO and MediaTaskRunner parts." This is a reland of 7a5f1db86fda6c73b7362eb9e990c080c9bb6d55 Original change's description: > Break down the NativeMediaFileUtil into IO and MediaTaskRunner parts. > > The NativeMediaFileUtil runs tasks on a TaskScheduler sequence provided > by the MediaFileSystemBackend, but was itself owned and deleted on the > IO thread, where its AsyncFileUtil API was used. > > The NativeMediaFileUtil is split into an outer part, used and owned on > the IO thread, and a "core" containing the MediaPathFilter for use on > the MediaFileSystemBackend's media TaskRunner. > > The MediaFileValidatorTests are also fixed, to tear-down the storage > FileSystemContext before the browser's threads are torn-down. > > Bug: 729716 > Change-Id: I339fe68f28d1bf6330ec4d341d2730df26bc4b30 > Reviewed-on: https://chromium-review.googlesource.com/1072965 > Commit-Queue: Wez <wez@chromium.org> > Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> > Cr-Commit-Position: refs/heads/master@{#563918} TBR: kinuko Bug: 729716 Change-Id: I635ea5efbe4b513ceb10e7aea05a25282c48f6e1 Reviewed-on: https://chromium-review.googlesource.com/1085527 Commit-Queue: Wez <wez@chromium.org> Reviewed-by: Wez <wez@chromium.org> Cr-Commit-Position: refs/heads/master@{#564204} [modify] https://crrev.com/a08664ba834ee3eb9461e1db04c00741710831cf/chrome/browser/media_galleries/fileapi/media_file_system_backend.cc [modify] https://crrev.com/a08664ba834ee3eb9461e1db04c00741710831cf/chrome/browser/media_galleries/fileapi/media_file_validator_browsertest.cc [modify] https://crrev.com/a08664ba834ee3eb9461e1db04c00741710831cf/chrome/browser/media_galleries/fileapi/native_media_file_util.cc [modify] https://crrev.com/a08664ba834ee3eb9461e1db04c00741710831cf/chrome/browser/media_galleries/fileapi/native_media_file_util.h [modify] https://crrev.com/a08664ba834ee3eb9461e1db04c00741710831cf/storage/browser/fileapi/file_system_operation_runner.cc [modify] https://crrev.com/a08664ba834ee3eb9461e1db04c00741710831cf/storage/browser/fileapi/file_system_operation_runner.h
,
Aug 23
,
Aug 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1f1509c0282f5141c2bd5b4703b1f3ef1acd53c9 commit 1f1509c0282f5141c2bd5b4703b1f3ef1acd53c9 Author: Wez <wez@chromium.org> Date: Thu Aug 23 19:58:24 2018 Fix use of WeakPtr in AsyncLayerTreeFrameSink unit-tests. Delete the AsyncLayerTreeFrameSink from the |bg_thread|, on which the test actually uses it. Bug: 729716 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Id1fd667d44bb2a8e1b6cbd973f1ec0708f47ee99 Reviewed-on: https://chromium-review.googlesource.com/1186186 Reviewed-by: enne <enne@chromium.org> Commit-Queue: Wez <wez@chromium.org> Cr-Commit-Position: refs/heads/master@{#585589} [modify] https://crrev.com/1f1509c0282f5141c2bd5b4703b1f3ef1acd53c9/cc/mojo_embedder/async_layer_tree_frame_sink_unittest.cc |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by bugdroid1@chromium.org
, Jun 6 2017