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

Issue 1423483004: Updated TODO(shreyasv) (Closed)

Created:
5 years, 2 months ago by shreyasv1
Modified:
5 years, 2 months ago
CC:
cbentzel+watch_chromium.org, chromium-reviews, sdefresne+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Updated TODO(shreyasv) Removed stale TODO in SnapshotManager. The sessionID is not the same for the lifetime of the SnapshotManager and can change e.g. during session restore. Changed TODOs to the format TODO(crbug.com/*) BUG=546145 Committed: https://crrev.com/f8d23b1d5938b2d3514802bde618194d2466437a Cr-Commit-Position: refs/heads/master@{#355564}

Patch Set 1 #

Total comments: 8

Patch Set 2 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -31 lines) Patch
M ios/chrome/browser/snapshots/snapshot_manager.h View 1 chunk +0 lines, -2 lines 0 comments Download
M ios/net/cookies/cookie_store_ios.mm View 1 chunk +0 lines, -3 lines 0 comments Download
M ios/web/browsing_data_managers/crw_cookie_browsing_data_manager.mm View 1 chunk +0 lines, -7 lines 0 comments Download
M ios/web/crw_browsing_data_store.mm View 4 chunks +0 lines, -10 lines 0 comments Download
M ios/web/public/test/crw_test_js_injection_receiver.mm View 1 2 chunks +4 lines, -3 lines 0 comments Download
M ios/web/public/test/response_providers/data_response_provider.mm View 1 chunk +2 lines, -2 lines 0 comments Download
M ios/web/public/web_state/ui/crw_native_content.h View 1 chunk +2 lines, -2 lines 0 comments Download
M ios/web/web_state/ui/wk_web_view_configuration_provider.mm View 1 chunk +1 line, -1 line 0 comments Download
M ios/web/web_state/web_state_impl.h View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 20 (7 generated)
shreyasv1
droger for ios/net eugenebut for ios/web
5 years, 2 months ago (2015-10-21 23:51:57 UTC) #2
shreyasv1
sdefresne for ios/
5 years, 2 months ago (2015-10-21 23:53:17 UTC) #4
droger
ios/net lgtm
5 years, 2 months ago (2015-10-21 23:54:24 UTC) #5
sdefresne
ios/chrome lgtm (but droger is also owners of that) https://codereview.chromium.org/1423483004/diff/1/ios/web/web_state/web_state_impl.h File ios/web/web_state/web_state_impl.h (right): https://codereview.chromium.org/1423483004/diff/1/ios/web/web_state/web_state_impl.h#newcode176 ios/web/web_state/web_state_impl.h:176: ...
5 years, 2 months ago (2015-10-21 23:56:21 UTC) #6
Eugene But (OOO till 7-30)
I have a question and a NIT. Otherwise lgtm https://codereview.chromium.org/1423483004/diff/1/ios/chrome/browser/snapshots/snapshot_manager.h File ios/chrome/browser/snapshots/snapshot_manager.h (left): https://codereview.chromium.org/1423483004/diff/1/ios/chrome/browser/snapshots/snapshot_manager.h#oldcode31 ios/chrome/browser/snapshots/snapshot_manager.h:31: ...
5 years, 2 months ago (2015-10-22 00:12:02 UTC) #7
shreyasv1
https://codereview.chromium.org/1423483004/diff/1/ios/chrome/browser/snapshots/snapshot_manager.h File ios/chrome/browser/snapshots/snapshot_manager.h (left): https://codereview.chromium.org/1423483004/diff/1/ios/chrome/browser/snapshots/snapshot_manager.h#oldcode31 ios/chrome/browser/snapshots/snapshot_manager.h:31: // TODO(shreyasv): Consider passing the sessionID into SnapshotManager from ...
5 years, 2 months ago (2015-10-22 00:33:59 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1423483004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1423483004/1
5 years, 2 months ago (2015-10-22 00:35:22 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator_ninja/builds/84548)
5 years, 2 months ago (2015-10-22 01:15:14 UTC) #13
sdefresne
https://codereview.chromium.org/1423483004/diff/1/ios/web/public/test/crw_test_js_injection_receiver.mm File ios/web/public/test/crw_test_js_injection_receiver.mm (right): https://codereview.chromium.org/1423483004/diff/1/ios/web/public/test/crw_test_js_injection_receiver.mm#newcode33 ios/web/public/test/crw_test_js_injection_receiver.mm:33: UIWebView* webView = weakEvaluator ? weakEvaluator->_webView : nil; Looks ...
5 years, 2 months ago (2015-10-22 03:17:34 UTC) #14
shreyasv1
https://codereview.chromium.org/1423483004/diff/1/ios/web/public/test/crw_test_js_injection_receiver.mm File ios/web/public/test/crw_test_js_injection_receiver.mm (right): https://codereview.chromium.org/1423483004/diff/1/ios/web/public/test/crw_test_js_injection_receiver.mm#newcode33 ios/web/public/test/crw_test_js_injection_receiver.mm:33: UIWebView* webView = weakEvaluator ? weakEvaluator->_webView : nil; On ...
5 years, 2 months ago (2015-10-22 17:08:26 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1423483004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1423483004/20001
5 years, 2 months ago (2015-10-22 17:09:21 UTC) #18
commit-bot: I haz the power
Committed patchset #2 (id:20001)
5 years, 2 months ago (2015-10-22 17:24:40 UTC) #19
commit-bot: I haz the power
5 years, 2 months ago (2015-10-22 17:25:24 UTC) #20
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/f8d23b1d5938b2d3514802bde618194d2466437a
Cr-Commit-Position: refs/heads/master@{#355564}

Powered by Google App Engine
This is Rietveld 408576698