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

Issue 2480293004: Mandate unique_ptr for base::IDMap in IDMapOwnPointer mode. (Closed)

Created:
4 years, 1 month ago by rlanday
Modified:
4 years ago
CC:
ajwong+watch_chromium.org, asvitkine+watch_chromium.org, Avi (use Gerrit), awdf+watch_chromium.org, blink-worker-reviews_chromium.org, brettw, chromium-reviews, cmumford, creis+watch_chromium.org, darin-cc_chromium.org, feature-media-reviews_chromium.org, harkness+watch_chromium.org, horo+watch_chromium.org, jam, jkarlin+watch_chromium.org, johnme+watch_chromium.org, jsbell+idb_chromium.org, jsbell+serviceworker_chromium.org, kinuko+serviceworker, kinuko+fileapi, kinuko+watch, mcasas+watch+vc_chromium.org, michaeln, mlamouri+watch-permissions_chromium.org, mlamouri+watch-manifest_chromium.org, mlamouri+watch-content_chromium.org, mlamouri+watch-notifications_chromium.org, mlamouri+watch-screen-orientation_chromium.org, nhiroki, Peter Beverloo, serviceworker-reviews, shimazu+serviceworker_chromium.org, tzik, zea+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Mandate unique_ptr for base::IDMap in IDMapOwnPointer mode. This patch refactors all callsites of base::IDMap<T, IDMapOwnPointer> to pass an std::unique_ptr, eliminating implicit transfers of ownership when passing a raw pointer. This is step 1 of 2 of refactoring base::IDMap to use std::unique_ptr instead of custom memory management code. (Step 2 will be to eliminate the IDMapOwnershipSemantics enum and just make callers create IDMaps with unique_ptrs as the value type: pending at http://crrev.com/2496653002/) BUG=647091 TBR=boliu (for android_webview/) TBR=stevenjb (for networking_private_service_client.cc) TBR=jochen (other files) Committed: https://crrev.com/6eada0329b5e51ab20254aa20c37fbee6288dff1 Cr-Commit-Position: refs/heads/master@{#435362}

Patch Set 1 #

Total comments: 1

Patch Set 2 : Respond to aelias's comment, fix unit test #

Patch Set 3 : Attempt to fix all the failing builds #

Patch Set 4 : Fix bug causing crash during Android test cases, attempt to fix compilation for Mac/Windows #

Patch Set 5 : Clean up unnecessary release/cast #

Total comments: 8

Patch Set 6 : aelias's suggested changes #

Patch Set 7 : Fix typo breaking a bunch of trybot builds, oops #

Total comments: 40

Patch Set 8 : Make the changes requested by danakj (except for a few that turned out to be tricky) #

Patch Set 9 : Rebase #

Patch Set 10 : Attempt to fix some build errors #

Patch Set 11 : Fix typo (extra >)...oops #

Patch Set 12 : Fix some stuff in FileSystemOperationRunner, respond to danakj's comment about RendererBlinkPlatfor… #

Patch Set 13 : Convert PresentationDispatcher to unique_ptrs #

Patch Set 14 : Rebase on some ScreenOrientation changes, update that stuff to use unique_ptr (the change I was sca… #

Total comments: 1

Patch Set 15 : Rebase on some IndexedDB changes (some stuff was refactored to no longer use IDMap, reducing the am… #

Patch Set 16 : Rebase #

Total comments: 24

Patch Set 17 : Make changes requested by danakj, fix a few more headers #

Unified diffs Side-by-side diffs Delta from patch set Stats (+898 lines, -664 lines) Patch
M android_webview/browser/aw_permission_manager.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 6 chunks +26 lines, -20 lines 0 comments Download
M android_webview/native/aw_contents_client_bridge.cc View 1 2 3 4 5 6 7 4 chunks +6 lines, -3 lines 0 comments Download
M base/id_map.h View 1 2 3 4 5 6 7 2 chunks +9 lines, -21 lines 0 comments Download
M base/id_map_unittest.cc View 1 2 3 4 5 6 7 7 chunks +7 lines, -9 lines 0 comments Download
M chrome/browser/android/compositor/layer_title_cache.cc View 1 2 3 4 5 6 7 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/android/service_tab_launcher.cc View 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/media/android/router/media_router_android.cc View 1 2 3 4 5 6 7 8 9 4 chunks +8 lines, -13 lines 0 comments Download
M chrome/browser/media/cast_transport_host_filter.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 3 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/metrics/subprocess_metrics_provider.cc View 1 2 3 4 5 6 7 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/permissions/permission_manager.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 4 chunks +5 lines, -5 lines 0 comments Download
M chrome/browser/ui/blocked_content/popup_blocker_tab_helper.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/tab_contents/core_tab_helper.cc View 1 chunk +1 line, -1 line 0 comments Download
M components/drive/job_scheduler.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +5 lines, -3 lines 0 comments Download
M components/gcm_driver/instance_id/instance_id_android.cc View 5 chunks +10 lines, -7 lines 0 comments Download
M components/test_runner/mock_screen_orientation_client.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +6 lines, -3 lines 0 comments Download
M components/test_runner/mock_screen_orientation_client.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 3 chunks +7 lines, -5 lines 0 comments Download
M content/browser/cache_storage/cache_storage_cache.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +4 lines, -4 lines 0 comments Download
M content/browser/manifest/manifest_manager_host.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -1 line 0 comments Download
M content/browser/permissions/permission_service_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 4 chunks +13 lines, -7 lines 0 comments Download
M content/browser/quota_dispatcher_host.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 2 chunks +5 lines, -1 line 0 comments Download
M content/child/fileapi/file_system_dispatcher.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 2 chunks +29 lines, -20 lines 0 comments Download
M content/child/notifications/notification_manager.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +2 lines, -2 lines 0 comments Download
M content/child/notifications/notification_manager.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 6 chunks +6 lines, -8 lines 0 comments Download
M content/child/push_messaging/push_provider.h View 1 2 3 4 5 6 7 2 chunks +7 lines, -4 lines 0 comments Download
M content/child/push_messaging/push_provider.cc View 1 2 3 4 5 6 7 8 5 chunks +9 lines, -9 lines 0 comments Download
M content/child/quota_dispatcher.h View 1 2 3 4 5 6 7 2 chunks +4 lines, -3 lines 0 comments Download
M content/child/quota_dispatcher.cc View 1 2 3 4 5 6 7 4 chunks +10 lines, -6 lines 0 comments Download
M content/child/service_worker/service_worker_dispatcher.h View 1 2 3 4 5 6 7 1 chunk +13 lines, -10 lines 0 comments Download
M content/child/service_worker/service_worker_dispatcher.cc View 1 2 3 4 5 6 7 11 chunks +19 lines, -19 lines 0 comments Download
M content/child/service_worker/web_service_worker_provider_impl.h View 1 2 3 4 5 6 7 2 chunks +13 lines, -7 lines 0 comments Download
M content/child/service_worker/web_service_worker_provider_impl.cc View 1 2 3 4 5 6 7 2 chunks +16 lines, -11 lines 0 comments Download
M content/child/service_worker/web_service_worker_registration_impl.h View 1 2 3 4 5 6 7 8 1 chunk +5 lines, -3 lines 0 comments Download
M content/child/service_worker/web_service_worker_registration_impl.cc View 1 2 3 4 5 6 7 8 1 chunk +4 lines, -4 lines 0 comments Download
M content/renderer/cache_storage/cache_storage_dispatcher.h View 1 2 3 4 5 6 7 3 chunks +20 lines, -9 lines 0 comments Download
M content/renderer/cache_storage/cache_storage_dispatcher.cc View 1 2 3 4 5 6 7 7 chunks +38 lines, -27 lines 0 comments Download
M content/renderer/cache_storage/webserviceworkercachestorage_impl.h View 1 2 3 4 5 6 7 2 chunks +8 lines, -5 lines 0 comments Download
M content/renderer/cache_storage/webserviceworkercachestorage_impl.cc View 1 2 3 4 5 6 7 2 chunks +14 lines, -10 lines 0 comments Download
M content/renderer/presentation/presentation_dispatcher.h View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +7 lines, -4 lines 0 comments Download
M content/renderer/presentation/presentation_dispatcher.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 6 chunks +8 lines, -9 lines 0 comments Download
M content/renderer/push_messaging/push_messaging_dispatcher.h View 1 2 3 4 5 6 7 3 chunks +5 lines, -3 lines 0 comments Download
M content/renderer/push_messaging/push_messaging_dispatcher.cc View 1 2 3 4 5 6 7 8 4 chunks +13 lines, -8 lines 0 comments Download
M content/renderer/renderer_blink_platform_impl.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +2 lines, -2 lines 0 comments Download
M content/renderer/renderer_blink_platform_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 5 chunks +15 lines, -11 lines 0 comments Download
M content/renderer/screen_orientation/screen_orientation_dispatcher.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 2 chunks +6 lines, -2 lines 0 comments Download
M content/renderer/screen_orientation/screen_orientation_dispatcher.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +2 lines, -2 lines 0 comments Download
M content/renderer/screen_orientation/screen_orientation_dispatcher_browsertest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 7 chunks +25 lines, -19 lines 0 comments Download
M content/renderer/service_worker/embedded_worker_dispatcher.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +2 lines, -1 line 0 comments Download
M content/renderer/service_worker/service_worker_context_client.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +18 lines, -13 lines 0 comments Download
M content/renderer/service_worker/service_worker_context_client.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 6 chunks +18 lines, -18 lines 0 comments Download
M content/shell/browser/layout_test/layout_test_permission_manager.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 3 chunks +4 lines, -2 lines 0 comments Download
M extensions/browser/api/networking_private/networking_private_service_client.cc View 1 2 3 4 5 6 7 2 chunks +4 lines, -1 line 0 comments Download
M storage/browser/fileapi/file_system_operation_runner.h View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M storage/browser/fileapi/file_system_operation_runner.cc View 1 2 3 4 5 6 7 8 9 10 11 22 chunks +135 lines, -99 lines 0 comments Download
M third_party/WebKit/Source/modules/cachestorage/Cache.cpp View 1 2 3 4 5 6 7 7 chunks +19 lines, -14 lines 0 comments Download
M third_party/WebKit/Source/modules/cachestorage/CacheStorage.cpp View 1 2 3 4 5 6 7 6 chunks +18 lines, -11 lines 0 comments Download
M third_party/WebKit/Source/modules/cachestorage/CacheTest.cpp View 1 2 3 4 5 6 7 5 chunks +8 lines, -20 lines 0 comments Download
M third_party/WebKit/Source/modules/cachestorage/InspectorCacheStorageAgent.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 8 chunks +19 lines, -15 lines 0 comments Download
M third_party/WebKit/Source/modules/notifications/ServiceWorkerRegistrationNotifications.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 3 chunks +6 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/modules/presentation/PresentationReceiverTest.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 3 chunks +30 lines, -9 lines 0 comments Download
M third_party/WebKit/Source/modules/presentation/PresentationRequest.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +8 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/modules/push_messaging/PushManager.cpp View 1 2 3 4 5 6 7 3 chunks +5 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/modules/push_messaging/PushSubscription.cpp View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/modules/screen_orientation/ScreenOrientation.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/modules/screen_orientation/ScreenOrientationController.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +3 lines, -1 line 0 comments Download
M third_party/WebKit/Source/modules/screen_orientation/ScreenOrientationController.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 2 chunks +6 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerClients.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 5 chunks +10 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerContainer.cpp View 1 2 3 4 5 6 7 5 chunks +10 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerContainerTest.cpp View 1 2 3 4 5 6 7 3 chunks +9 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScope.cpp View 1 2 3 4 5 6 7 2 chunks +3 lines, -1 line 0 comments Download
M third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScopeClient.h View 1 2 3 4 5 6 7 2 chunks +12 lines, -8 lines 0 comments Download
M third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerRegistration.cpp View 1 2 3 4 5 6 7 3 chunks +5 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerWindowClient.cpp View 1 2 3 4 5 6 7 2 chunks +5 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/web/ServiceWorkerGlobalScopeClientImpl.h View 1 2 3 4 5 6 7 2 chunks +10 lines, -7 lines 0 comments Download
M third_party/WebKit/Source/web/ServiceWorkerGlobalScopeClientImpl.cpp View 1 2 3 4 5 6 7 3 chunks +15 lines, -14 lines 0 comments Download
M third_party/WebKit/Source/web/WebEmbeddedWorkerImplTest.cpp View 1 2 3 4 5 6 7 2 chunks +13 lines, -7 lines 0 comments Download
M third_party/WebKit/public/platform/modules/notifications/WebNotificationManager.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +10 lines, -8 lines 0 comments Download
M third_party/WebKit/public/platform/modules/presentation/WebPresentationClient.h View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +11 lines, -11 lines 0 comments Download
M third_party/WebKit/public/platform/modules/push_messaging/WebPushClient.h View 1 2 3 4 5 6 7 2 chunks +2 lines, -1 line 0 comments Download
M third_party/WebKit/public/platform/modules/push_messaging/WebPushProvider.h View 1 2 3 4 5 6 7 1 chunk +9 lines, -7 lines 0 comments Download
M third_party/WebKit/public/platform/modules/screen_orientation/WebScreenOrientationClient.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +2 lines, -1 line 0 comments Download
M third_party/WebKit/public/platform/modules/serviceworker/WebServiceWorkerCache.h View 1 2 3 4 5 6 7 2 chunks +6 lines, -4 lines 0 comments Download
M third_party/WebKit/public/platform/modules/serviceworker/WebServiceWorkerCacheStorage.h View 1 2 3 4 5 6 7 1 chunk +5 lines, -5 lines 0 comments Download
M third_party/WebKit/public/platform/modules/serviceworker/WebServiceWorkerProvider.h View 1 2 3 4 5 6 7 1 chunk +10 lines, -7 lines 0 comments Download
M third_party/WebKit/public/platform/modules/serviceworker/WebServiceWorkerRegistration.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 2 chunks +5 lines, -3 lines 0 comments Download
M third_party/WebKit/public/web/modules/serviceworker/WebServiceWorkerContextClient.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 4 chunks +13 lines, -8 lines 0 comments Download

Messages

Total messages: 120 (80 generated)
aelias_OOO_until_Jul13
https://codereview.chromium.org/2480293004/diff/1/base/id_map.h File base/id_map.h (right): https://codereview.chromium.org/2480293004/diff/1/base/id_map.h#newcode91 base/id_map.h:91: KeyType Add(T* data) { I don't think the explicit ...
4 years, 1 month ago (2016-11-08 01:55:09 UTC) #9
rlanday
4 years, 1 month ago (2016-11-09 18:06:56 UTC) #24
aelias_OOO_until_Jul13
Looks good overall. It doesn't seem like right now we can follow down all the ...
4 years, 1 month ago (2016-11-10 06:04:48 UTC) #25
rlanday
https://codereview.chromium.org/2480293004/diff/80001/components/drive/job_scheduler.cc File components/drive/job_scheduler.cc (right): https://codereview.chromium.org/2480293004/diff/80001/components/drive/job_scheduler.cc#newcode726 components/drive/job_scheduler.cc:726: std::unique_ptr<JobEntry>(job)); // Takes the ownership of |job|. On 2016/11/10 ...
4 years, 1 month ago (2016-11-10 18:20:01 UTC) #26
aelias_OOO_until_Jul13
https://codereview.chromium.org/2480293004/diff/80001/components/drive/job_scheduler.cc File components/drive/job_scheduler.cc (right): https://codereview.chromium.org/2480293004/diff/80001/components/drive/job_scheduler.cc#newcode726 components/drive/job_scheduler.cc:726: std::unique_ptr<JobEntry>(job)); // Takes the ownership of |job|. On 2016/11/10 ...
4 years, 1 month ago (2016-11-10 19:36:19 UTC) #27
aelias_OOO_until_Jul13
lgtm, adding brettw@ for root OWNERS since this is a large-ish mechanical transformation.
4 years, 1 month ago (2016-11-11 21:59:34 UTC) #39
boliu
""" For widespread or refactoring changes that are mechanical or global in nature, it is ...
4 years, 1 month ago (2016-11-11 22:06:13 UTC) #40
aelias_OOO_until_Jul13
OK, removing brettw@ since he doesn't have cycles. Adding danakj@ as base/ OWNER with plan ...
4 years, 1 month ago (2016-11-17 02:30:16 UTC) #43
danakj
So.. we should spread the unique_ptr love a bit further. https://codereview.chromium.org/2480293004/diff/120001/android_webview/browser/aw_permission_manager.cc File android_webview/browser/aw_permission_manager.cc (right): https://codereview.chromium.org/2480293004/diff/120001/android_webview/browser/aw_permission_manager.cc#newcode280 ...
4 years, 1 month ago (2016-11-18 00:15:34 UTC) #44
aelias_OOO_until_Jul13
Most of the left-alone callback objects are created in third_party/WebKit; I wasn't sure if there ...
4 years, 1 month ago (2016-11-18 00:24:06 UTC) #45
danakj
On Thu, Nov 17, 2016 at 4:24 PM, <aelias@chromium.org> wrote: > Most of the left-alone ...
4 years, 1 month ago (2016-11-18 00:29:44 UTC) #46
rlanday
https://codereview.chromium.org/2480293004/diff/120001/content/renderer/renderer_blink_platform_impl.cc File content/renderer/renderer_blink_platform_impl.cc (right): https://codereview.chromium.org/2480293004/diff/120001/content/renderer/renderer_blink_platform_impl.cc#newcode1206 content/renderer/renderer_blink_platform_impl.cc:1206: PlatformEventObserverBase* observer = platform_event_observers_.Lookup(type); On 2016/11/18 at 00:15:33, danakj ...
4 years, 1 month ago (2016-11-18 18:57:13 UTC) #47
danakj
https://codereview.chromium.org/2480293004/diff/120001/content/renderer/renderer_blink_platform_impl.cc File content/renderer/renderer_blink_platform_impl.cc (right): https://codereview.chromium.org/2480293004/diff/120001/content/renderer/renderer_blink_platform_impl.cc#newcode1206 content/renderer/renderer_blink_platform_impl.cc:1206: PlatformEventObserverBase* observer = platform_event_observers_.Lookup(type); On 2016/11/18 18:57:13, rlanday wrote: ...
4 years, 1 month ago (2016-11-18 19:13:29 UTC) #48
rlanday
https://codereview.chromium.org/2480293004/diff/120001/content/child/fileapi/file_system_dispatcher.cc File content/child/fileapi/file_system_dispatcher.cc (right): https://codereview.chromium.org/2480293004/diff/120001/content/child/fileapi/file_system_dispatcher.cc#newcode175 content/child/fileapi/file_system_dispatcher.cc:175: std::unique_ptr<CallbackDispatcher>( On 2016/11/18 at 00:15:33, danakj wrote: > can ...
4 years, 1 month ago (2016-11-18 19:32:10 UTC) #49
rlanday
https://codereview.chromium.org/2480293004/diff/120001/content/child/indexed_db/indexed_db_dispatcher.h File content/child/indexed_db/indexed_db_dispatcher.h (right): https://codereview.chromium.org/2480293004/diff/120001/content/child/indexed_db/indexed_db_dispatcher.h#newcode133 content/child/indexed_db/indexed_db_dispatcher.h:133: void init_params(T* params, blink::WebIDBCallbacks* callbacks_ptr) { On 2016/11/18 at ...
4 years, 1 month ago (2016-11-18 20:40:41 UTC) #50
danakj
https://codereview.chromium.org/2480293004/diff/120001/content/child/fileapi/file_system_dispatcher.cc File content/child/fileapi/file_system_dispatcher.cc (right): https://codereview.chromium.org/2480293004/diff/120001/content/child/fileapi/file_system_dispatcher.cc#newcode175 content/child/fileapi/file_system_dispatcher.cc:175: std::unique_ptr<CallbackDispatcher>( On 2016/11/18 19:32:10, rlanday wrote: > On 2016/11/18 ...
4 years, 1 month ago (2016-11-18 21:21:14 UTC) #51
rlanday
On 2016/11/18 at 20:40:41, rlanday wrote: > https://codereview.chromium.org/2480293004/diff/120001/content/child/indexed_db/indexed_db_dispatcher.h > File content/child/indexed_db/indexed_db_dispatcher.h (right): > > https://codereview.chromium.org/2480293004/diff/120001/content/child/indexed_db/indexed_db_dispatcher.h#newcode133 ...
4 years, 1 month ago (2016-11-18 22:24:51 UTC) #52
rlanday
4 years, 1 month ago (2016-11-18 22:25:01 UTC) #53
rlanday
https://codereview.chromium.org/2480293004/diff/120001/content/renderer/presentation/presentation_dispatcher.cc File content/renderer/presentation/presentation_dispatcher.cc (right): https://codereview.chromium.org/2480293004/diff/120001/content/renderer/presentation/presentation_dispatcher.cc#newcode247 content/renderer/presentation/presentation_dispatcher.cc:247: blink::WebPresentationAvailabilityCallbacks* callbacks) { On 2016/11/18 at 00:15:33, danakj wrote: ...
4 years, 1 month ago (2016-11-18 23:46:56 UTC) #54
danakj
https://codereview.chromium.org/2480293004/diff/120001/content/renderer/presentation/presentation_dispatcher.cc File content/renderer/presentation/presentation_dispatcher.cc (right): https://codereview.chromium.org/2480293004/diff/120001/content/renderer/presentation/presentation_dispatcher.cc#newcode247 content/renderer/presentation/presentation_dispatcher.cc:247: blink::WebPresentationAvailabilityCallbacks* callbacks) { On 2016/11/18 23:46:56, rlanday wrote: > ...
4 years, 1 month ago (2016-11-21 21:34:26 UTC) #72
rlanday
@danakj, I think I've responded to all your feedback and this is ready for review ...
4 years, 1 month ago (2016-11-22 18:22:33 UTC) #83
rlanday
https://codereview.chromium.org/2480293004/diff/120001/content/renderer/screen_orientation/screen_orientation_dispatcher.cc File content/renderer/screen_orientation/screen_orientation_dispatcher.cc (right): https://codereview.chromium.org/2480293004/diff/120001/content/renderer/screen_orientation/screen_orientation_dispatcher.cc#newcode69 content/renderer/screen_orientation/screen_orientation_dispatcher.cc:69: blink::WebLockOrientationCallback* callback) { On 2016/11/22 at 18:22:33, rlanday wrote: ...
4 years, 1 month ago (2016-11-22 19:08:04 UTC) #84
rlanday
Rebasing again. Should I split this into multiple changesets?
4 years ago (2016-11-30 00:02:31 UTC) #90
danakj
Next time your CL is getting big like this you should split it, it would ...
4 years ago (2016-11-30 00:34:01 UTC) #92
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2480293004/320001
4 years ago (2016-11-30 01:59:57 UTC) #95
commit-bot: I haz the power
Try jobs failed on following builders: blimp_linux_dbg on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) cast_shell_linux on ...
4 years ago (2016-11-30 04:03:02 UTC) #97
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2480293004/320001
4 years ago (2016-11-30 04:46:28 UTC) #99
commit-bot: I haz the power
Exceeded global retry quota
4 years ago (2016-11-30 06:48:11 UTC) #101
rlanday
https://codereview.chromium.org/2480293004/diff/300001/content/browser/permissions/permission_service_impl.cc File content/browser/permissions/permission_service_impl.cc (right): https://codereview.chromium.org/2480293004/diff/300001/content/browser/permissions/permission_service_impl.cc#newcode9 content/browser/permissions/permission_service_impl.cc:9: #include <memory> On 2016/11/30 at 00:34:00, danakj wrote: > ...
4 years ago (2016-11-30 15:52:01 UTC) #102
rlanday
4 years ago (2016-11-30 15:52:04 UTC) #103
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2480293004/320001
4 years ago (2016-11-30 16:08:35 UTC) #105
rlanday
On 2016/11/30 at 16:08:35, commit-bot wrote: > CQ is trying da patch. Follow status at ...
4 years ago (2016-11-30 16:16:47 UTC) #107
rlanday
4 years ago (2016-11-30 16:16:56 UTC) #108
Avi (use Gerrit)
lgtm stamp to the extent it's useful.
4 years ago (2016-11-30 17:02:01 UTC) #110
rlanday
On 2016/11/30 at 17:02:01, avi wrote: > lgtm > > stamp to the extent it's ...
4 years ago (2016-11-30 18:31:44 UTC) #111
Avi (use Gerrit)
On 2016/11/30 18:31:44, rlanday wrote: > On 2016/11/30 at 17:02:01, avi wrote: > > lgtm ...
4 years ago (2016-11-30 18:32:53 UTC) #112
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2480293004/320001
4 years ago (2016-11-30 18:52:57 UTC) #115
commit-bot: I haz the power
Committed patchset #17 (id:320001)
4 years ago (2016-11-30 19:00:07 UTC) #117
commit-bot: I haz the power
Patchset 17 (id:??) landed as https://crrev.com/6eada0329b5e51ab20254aa20c37fbee6288dff1 Cr-Commit-Position: refs/heads/master@{#435362}
4 years ago (2016-11-30 19:05:34 UTC) #119
danakj
4 years ago (2016-11-30 19:25:06 UTC) #120
Message was sent while issue was closed.
https://codereview.chromium.org/2480293004/diff/300001/content/renderer/scree...
File content/renderer/screen_orientation/screen_orientation_dispatcher.cc
(right):

https://codereview.chromium.org/2480293004/diff/300001/content/renderer/scree...
content/renderer/screen_orientation/screen_orientation_dispatcher.cc:7: #include
<memory>
On 2016/11/30 15:52:01, rlanday wrote:
> On 2016/11/30 at 00:34:01, danakj wrote:
> > this file wants utility for move() rather than memory. the unique_ptr is
> coming from the header so it should already be there.
> 
> Oops...looks like git cl lint's flagging a couple other files for this as well
> (e.g. the .h file for this .cc file), I'll add the header on those too

Ya we don't reeally have include-what-you-use tho some sorta warnings I think
exist for a few STL types.

> I assume I don't need this #include both here and in the .h?

Right, if it's in the .h u can skip it in the .cc

Powered by Google App Engine
This is Rietveld 408576698