Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1843383002:
Safe Browsing: CheckApiBlacklist request implementation (Closed)
|
Created:
4 years, 8 months ago by kcarattini Modified:
4 years, 8 months ago CC:
chromium-reviews, chrome-apps-syd-reviews_chromium.org, vakh (use Gerrit instead) Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionSafe Browsing: CheckApiBlacklist request implementation
BUG=561867
Committed: https://crrev.com/1343c9206db66bc5ec0a8aa67d745813b197cde4
Cr-Commit-Position: refs/heads/master@{#387869}
Committed: https://crrev.com/5fad5b496bbd1cacea9591f2f79ec7b524ecf397
Cr-Commit-Position: refs/heads/master@{#388132}
Patch Set 1 #Patch Set 2 : Test (Ignore) #Patch Set 3 : Test2 #Patch Set 4 : Test #
Total comments: 12
Patch Set 5 : Review Comments #
Total comments: 16
Patch Set 6 : Review comments #Patch Set 7 : Review comments and gyp fix #Patch Set 8 : Typo #Patch Set 9 : Fix Build Again #Patch Set 10 : Use shared_ptr #
Total comments: 7
Patch Set 11 : Review Comments #
Dependent Patchsets: Messages
Total messages: 51 (21 generated)
kcarattini@chromium.org changed reviewers: + nparker@chromium.org
The CQ bit was checked by kcarattini@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1843383002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1843383002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_compile_dbg on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
https://codereview.chromium.org/1843383002/diff/60001/components/safe_browsin... File components/safe_browsing_db/database_manager_unittest.cc (right): https://codereview.chromium.org/1843383002/diff/60001/components/safe_browsin... components/safe_browsing_db/database_manager_unittest.cc:1: // Copyright 2013 The Chromium Authors. All rights reserved. 2016 https://codereview.chromium.org/1843383002/diff/60001/components/safe_browsin... components/safe_browsing_db/database_manager_unittest.cc:42: callback.Run(full_hashes_, base::TimeDelta::FromMinutes(0)); You probably dont' need to call the callback, since you're not making use of that functionality in the tests. https://codereview.chromium.org/1843383002/diff/60001/components/safe_browsin... components/safe_browsing_db/database_manager_unittest.cc:72: TestV4GetHashProtocolManager* GetProtocolManager() { return pm_; } A thought: Could callers instead get it from the SBFBMgr? That way you would't need to stash it here, which ultimately might fail if the DBMgr had destructed/zero'd it. https://codereview.chromium.org/1843383002/diff/60001/components/safe_browsin... components/safe_browsing_db/database_manager_unittest.cc:99: TestV4GetHashProtocolManagerFactory get_hash_pm_factory; Could move the first few lines into the test class's SetUp() method. https://codereview.chromium.org/1843383002/diff/60001/components/safe_browsin... components/safe_browsing_db/database_manager_unittest.cc:123: std::vector<SBPrefix> expected_prefixes = Can you add a comment on how you generated these, in case we need to repeat it? https://codereview.chromium.org/1843383002/diff/60001/components/safe_browsin... File components/safe_browsing_db/test_database_manager.h (right): https://codereview.chromium.org/1843383002/diff/60001/components/safe_browsin... components/safe_browsing_db/test_database_manager.h:25: // non-abstract methods in the base class are not overridden. This changes the model a bit, since now tests may accidentally call real implementations of things w/o check-failing. I"m not convinced that's a problem, but something to be aware of. The trade off is that this class requires touching every time we add a new method, which is slightly cumbersome. What do you think?
https://codereview.chromium.org/1843383002/diff/60001/components/safe_browsin... File components/safe_browsing_db/database_manager_unittest.cc (right): https://codereview.chromium.org/1843383002/diff/60001/components/safe_browsin... components/safe_browsing_db/database_manager_unittest.cc:1: // Copyright 2013 The Chromium Authors. All rights reserved. On 2016/04/07 21:33:37, Nathan Parker wrote: > 2016 My how time flies! Done. https://codereview.chromium.org/1843383002/diff/60001/components/safe_browsin... components/safe_browsing_db/database_manager_unittest.cc:42: callback.Run(full_hashes_, base::TimeDelta::FromMinutes(0)); On 2016/04/07 21:33:38, Nathan Parker wrote: > You probably dont' need to call the callback, since you're not making use of > that functionality in the tests. I'd like to call it for cleanliness -- it deletes the allocated SafeBrowsingApiCheck. I also anticipate using it for the response tests when I implement the response. https://codereview.chromium.org/1843383002/diff/60001/components/safe_browsin... components/safe_browsing_db/database_manager_unittest.cc:72: TestV4GetHashProtocolManager* GetProtocolManager() { return pm_; } On 2016/04/07 21:33:37, Nathan Parker wrote: > A thought: Could callers instead get it from the SBFBMgr? That way you would't > need to stash it here, which ultimately might fail if the DBMgr had > destructed/zero'd it. Removed. Turns out I wasn't using it anyway. https://codereview.chromium.org/1843383002/diff/60001/components/safe_browsin... components/safe_browsing_db/database_manager_unittest.cc:99: TestV4GetHashProtocolManagerFactory get_hash_pm_factory; On 2016/04/07 21:33:37, Nathan Parker wrote: > Could move the first few lines into the test class's SetUp() method. Done. https://codereview.chromium.org/1843383002/diff/60001/components/safe_browsin... components/safe_browsing_db/database_manager_unittest.cc:123: std::vector<SBPrefix> expected_prefixes = On 2016/04/07 21:33:38, Nathan Parker wrote: > Can you add a comment on how you generated these, in case we need to repeat it? Done. https://codereview.chromium.org/1843383002/diff/60001/components/safe_browsin... File components/safe_browsing_db/test_database_manager.h (right): https://codereview.chromium.org/1843383002/diff/60001/components/safe_browsin... components/safe_browsing_db/test_database_manager.h:25: // non-abstract methods in the base class are not overridden. On 2016/04/07 21:33:38, Nathan Parker wrote: > This changes the model a bit, since now tests may accidentally call real > implementations of things w/o check-failing. I"m not convinced that's a > problem, but something to be aware of. The trade off is that this class > requires touching every time we add a new method, which is slightly cumbersome. > What do you think? What I've done seemed like a reasonable solution to me. I checked that none of the current subclasses are calling the non-abstract methods except for LocalDatabaseManager, and that class' test is not significantly impacted (it just creates and destroys the V4 PM on start/stop). I anticipate future subclasses to be similar. The class still requires touching every time we add a non-abstract method, though.
The CQ bit was checked by kcarattini@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1843383002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1843383002/80001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
kcarattini@chromium.org changed reviewers: + sky@chromium.org
sky@ please review addition of content/public/test to DEPS file.
LGTM
nparker@chromium.org changed reviewers: + vakh@chromium.org
https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... File components/safe_browsing_db/database_manager.cc (right): https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... components/safe_browsing_db/database_manager.cc:72: // TODO(kcarattini): Track checks in a map. Related to my CancelClient question... Should we keep these in the same structure that tracks regular checks? I guess that exists only in the Local* or Remote* implementation. Hmm. https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... components/safe_browsing_db/database_manager.cc:73: SafeBrowsingApiCheck* check = new Maybe a std::unique_ptr<>? https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... File components/safe_browsing_db/database_manager.h (right): https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... components/safe_browsing_db/database_manager.h:69: struct SafeBrowsingApiCheck { This could be a class, since you've got public and private sections. https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... components/safe_browsing_db/database_manager.h:79: SafeBrowsingDatabaseManager::Client* client; // Not owned https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... components/safe_browsing_db/database_manager.h:169: virtual void CancelCheck(Client* client) = 0; Does this apply to CheckAPIBlacklistUrl clients?
https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... File components/safe_browsing_db/database_manager.cc (right): https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... components/safe_browsing_db/database_manager.cc:72: // TODO(kcarattini): Track checks in a map. On 2016/04/11 16:28:50, Nathan Parker wrote: > Related to my CancelClient question... Should we keep these in the same > structure that tracks regular checks? I guess that exists only in the Local* or > Remote* implementation. Hmm. The SafeBorwsingCheck seems overly complicated for this API check. Also, since here we're using the V4 server, I think they should be tracked separately frpm the V3 serve checks. When the old checks use the V4 server, we can look into merging then. How does that sound? https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... components/safe_browsing_db/database_manager.cc:73: SafeBrowsingApiCheck* check = new On 2016/04/11 16:28:50, Nathan Parker wrote: > Maybe a std::unique_ptr<>? I need this to live past the end of this function so it is available in the callback, I think a unique_ptr would delete the memory at the end of this function. https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... File components/safe_browsing_db/database_manager.h (right): https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... components/safe_browsing_db/database_manager.h:69: struct SafeBrowsingApiCheck { On 2016/04/11 16:28:50, Nathan Parker wrote: > This could be a class, since you've got public and private sections. Done. https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... components/safe_browsing_db/database_manager.h:79: SafeBrowsingDatabaseManager::Client* client; On 2016/04/11 16:28:50, Nathan Parker wrote: > // Not owned Done. https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... components/safe_browsing_db/database_manager.h:169: virtual void CancelCheck(Client* client) = 0; On 2016/04/11 16:28:50, Nathan Parker wrote: > Does this apply to CheckAPIBlacklistUrl clients? There's no functionality for it at the moment but it could be added. I can't think of a time when it would be necessary. What do you think?
The CQ bit was checked by kcarattini@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1843383002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1843383002/100001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_dbg_recipe on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...) linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... File components/safe_browsing_db/database_manager.cc (right): https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... components/safe_browsing_db/database_manager.cc:72: // TODO(kcarattini): Track checks in a map. On 2016/04/12 01:27:02, kcarattini wrote: > On 2016/04/11 16:28:50, Nathan Parker wrote: > > Related to my CancelClient question... Should we keep these in the same > > structure that tracks regular checks? I guess that exists only in the Local* > or > > Remote* implementation. Hmm. > > The SafeBorwsingCheck seems overly complicated for this API check. Also, since > here we're using the V4 server, I think they should be tracked separately frpm > the V3 serve checks. When the old checks use the V4 server, we can look into > merging then. How does that sound? Sounds like a plan! https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... components/safe_browsing_db/database_manager.cc:73: SafeBrowsingApiCheck* check = new On 2016/04/12 01:27:02, kcarattini wrote: > On 2016/04/11 16:28:50, Nathan Parker wrote: > > Maybe a std::unique_ptr<>? > > I need this to live past the end of this function so it is available in the > callback, I think a unique_ptr would delete the memory at the end of this > function. Yes, but I think you can pass it into Bind() and have it pass it into another unique_ptr... maybe with std::move()? (I forgot the name of that and made the mistake of googling std::pass. That's not it.) https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... File components/safe_browsing_db/database_manager.h (right): https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... components/safe_browsing_db/database_manager.h:169: virtual void CancelCheck(Client* client) = 0; On 2016/04/12 01:27:02, kcarattini wrote: > On 2016/04/11 16:28:50, Nathan Parker wrote: > > Does this apply to CheckAPIBlacklistUrl clients? > > There's no functionality for it at the moment but it could be added. I can't > think of a time when it would be necessary. What do you think? What happens if the caller (who owns the Client) needs to get deleted before the callback is invoked? I think we need some way for it to say "nevermind, don't call me back." I could be a CancelApiClient(). In that case you could use something other than the Client signature. I think Client could be split into several different signatures, one for each set of parameters. That'd be cleaner in that you'd know that your Client doesn't need to support the other types.
https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... File components/safe_browsing_db/database_manager.cc (right): https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... components/safe_browsing_db/database_manager.cc:73: SafeBrowsingApiCheck* check = new On 2016/04/12 04:35:48, Nathan Parker wrote: > On 2016/04/12 01:27:02, kcarattini wrote: > > On 2016/04/11 16:28:50, Nathan Parker wrote: > > > Maybe a std::unique_ptr<>? > > > > I need this to live past the end of this function so it is available in the > > callback, I think a unique_ptr would delete the memory at the end of this > > function. > > Yes, but I think you can pass it into Bind() and have it pass it into another > unique_ptr... maybe with std::move()? (I forgot the name of that and made the > mistake of googling std::pass. That's not it.) Since I plan to track this also in a map, I don't think unique_ptr is the right thing to use (there will be multiple references to the same memory). I could make it a shared ptr perhaps? I'm not sure how that works as a function argument. https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... File components/safe_browsing_db/database_manager.h (right): https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... components/safe_browsing_db/database_manager.h:169: virtual void CancelCheck(Client* client) = 0; On 2016/04/12 04:35:49, Nathan Parker wrote: > On 2016/04/12 01:27:02, kcarattini wrote: > > On 2016/04/11 16:28:50, Nathan Parker wrote: > > > Does this apply to CheckAPIBlacklistUrl clients? > > > > There's no functionality for it at the moment but it could be added. I can't > > think of a time when it would be necessary. What do you think? > > What happens if the caller (who owns the Client) needs to get deleted before the > callback is invoked? I think we need some way for it to say "nevermind, don't > call me back." I could be a CancelApiClient(). In that case you could use > something other than the Client signature. I think Client could be split into > several different signatures, one for each set of parameters. That'd be cleaner > in that you'd know that your Client doesn't need to support the other types. I've added a TODO to add a CancelApiCheck method.
The CQ bit was checked by kcarattini@chromium.org to run a CQ dry run
The CQ bit was checked by kcarattini@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1843383002/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1843383002/140001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_compile_dbg on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
The CQ bit was checked by kcarattini@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1843383002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1843383002/160001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... File components/safe_browsing_db/database_manager.cc (right): https://codereview.chromium.org/1843383002/diff/80001/components/safe_browsin... components/safe_browsing_db/database_manager.cc:73: SafeBrowsingApiCheck* check = new On 2016/04/12 07:47:49, kcarattini wrote: > On 2016/04/12 04:35:48, Nathan Parker wrote: > > On 2016/04/12 01:27:02, kcarattini wrote: > > > On 2016/04/11 16:28:50, Nathan Parker wrote: > > > > Maybe a std::unique_ptr<>? > > > > > > I need this to live past the end of this function so it is available in the > > > callback, I think a unique_ptr would delete the memory at the end of this > > > function. > > > > Yes, but I think you can pass it into Bind() and have it pass it into another > > unique_ptr... maybe with std::move()? (I forgot the name of that and made the > > mistake of googling std::pass. That's not it.) > > Since I plan to track this also in a map, I don't think unique_ptr is the right > thing to use (there will be multiple references to the same memory). I could > make it a shared ptr perhaps? I'm not sure how that works as a function > argument. Updated to use shared_ptr. Let me know if you think that's not the right way to handle this.
lgtm with ideally a different soln for the shared_ptr... https://codereview.chromium.org/1843383002/diff/180001/components/safe_browsi... File components/safe_browsing_db/database_manager.cc (right): https://codereview.chromium.org/1843383002/diff/180001/components/safe_browsi... components/safe_browsing_db/database_manager.cc:79: base::Unretained(this), check)); The tracking of requests is required so that you can clean them up before *this is deleted. Otherwise, they could call back later and access *this in a bad state. or... does deleteing the v4_get_hash_protocol manager clean up pending requests sufficiently? https://codereview.chromium.org/1843383002/diff/180001/components/safe_browsi... File components/safe_browsing_db/database_manager.h (right): https://codereview.chromium.org/1843383002/diff/180001/components/safe_browsi... components/safe_browsing_db/database_manager.h:69: class SafeBrowsingApiCheck { Does this class need to be public in SBDbMgr? External clients shouldn't need it. https://codereview.chromium.org/1843383002/diff/180001/components/safe_browsi... components/safe_browsing_db/database_manager.h:187: virtual void HandleGetHashesWithApisResults( protected?
Thanks. I'll submit as-is and we can discuss the correct pointer storage in https://codereview.chromium.org/1890753002/ since that's where the check tracking happens. https://codereview.chromium.org/1843383002/diff/180001/components/safe_browsi... File components/safe_browsing_db/database_manager.cc (right): https://codereview.chromium.org/1843383002/diff/180001/components/safe_browsi... components/safe_browsing_db/database_manager.cc:79: base::Unretained(this), check)); On 2016/04/15 23:14:29, Nathan Parker wrote: > The tracking of requests is required so that you can clean them up before *this > is deleted. Otherwise, they could call back later and access *this in a bad > state. > > or... does deleteing the v4_get_hash_protocol manager clean up pending requests > sufficiently? Sorry if I'm missing something -- I don't see how the protocol manager could invoke a callback after it's been deleted? It clears its storage of pending requests on destruction without calling clients. What sort of cleanup would be necessary? https://codereview.chromium.org/1843383002/diff/180001/components/safe_browsi... File components/safe_browsing_db/database_manager.h (right): https://codereview.chromium.org/1843383002/diff/180001/components/safe_browsi... components/safe_browsing_db/database_manager.h:69: class SafeBrowsingApiCheck { On 2016/04/15 23:14:29, Nathan Parker wrote: > Does this class need to be public in SBDbMgr? External clients shouldn't need > it. Done. https://codereview.chromium.org/1843383002/diff/180001/components/safe_browsi... components/safe_browsing_db/database_manager.h:187: virtual void HandleGetHashesWithApisResults( On 2016/04/15 23:14:29, Nathan Parker wrote: > protected? Done.
The CQ bit was checked by kcarattini@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org, nparker@chromium.org Link to the patchset: https://codereview.chromium.org/1843383002/#ps200001 (title: "Review Comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1843383002/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1843383002/200001
Message was sent while issue was closed.
Committed patchset #11 (id:200001)
Message was sent while issue was closed.
Description was changed from ========== Safe Browsing: CheckApiBlacklist request implementation BUG=561867 ========== to ========== Safe Browsing: CheckApiBlacklist request implementation BUG=561867 Committed: https://crrev.com/1343c9206db66bc5ec0a8aa67d745813b197cde4 Cr-Commit-Position: refs/heads/master@{#387869} ==========
Message was sent while issue was closed.
Patchset 11 (id:??) landed as https://crrev.com/1343c9206db66bc5ec0a8aa67d745813b197cde4 Cr-Commit-Position: refs/heads/master@{#387869}
Message was sent while issue was closed.
A revert of this CL (patchset #11 id:200001) has been created in https://codereview.chromium.org/1895743002/ by kcarattini@chromium.org. The reason for reverting is: Causing memory failures: https://build.chromium.org/p/chromium.memory.fyi/builders/Chromium%20OS%20%28....
Message was sent while issue was closed.
https://codereview.chromium.org/1843383002/diff/180001/components/safe_browsi... File components/safe_browsing_db/database_manager.cc (right): https://codereview.chromium.org/1843383002/diff/180001/components/safe_browsi... components/safe_browsing_db/database_manager.cc:79: base::Unretained(this), check)); On 2016/04/18 06:56:56, kcarattini wrote: > On 2016/04/15 23:14:29, Nathan Parker wrote: > > The tracking of requests is required so that you can clean them up before > *this > > is deleted. Otherwise, they could call back later and access *this in a bad > > state. > > > > or... does deleteing the v4_get_hash_protocol manager clean up pending > requests > > sufficiently? > > Sorry if I'm missing something -- I don't see how the protocol manager could > invoke a callback after it's been deleted? It clears its storage of pending > requests on destruction without calling clients. What sort of cleanup would be > necessary? Yea so this class is then protected from those. SGTM.
Message was sent while issue was closed.
Description was changed from ========== Safe Browsing: CheckApiBlacklist request implementation BUG=561867 Committed: https://crrev.com/1343c9206db66bc5ec0a8aa67d745813b197cde4 Cr-Commit-Position: refs/heads/master@{#387869} ========== to ========== Safe Browsing: CheckApiBlacklist request implementation BUG=561867 Committed: https://crrev.com/1343c9206db66bc5ec0a8aa67d745813b197cde4 Cr-Commit-Position: refs/heads/master@{#387869} ==========
The CQ bit was checked by kcarattini@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1843383002/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1843383002/200001
Message was sent while issue was closed.
Description was changed from ========== Safe Browsing: CheckApiBlacklist request implementation BUG=561867 Committed: https://crrev.com/1343c9206db66bc5ec0a8aa67d745813b197cde4 Cr-Commit-Position: refs/heads/master@{#387869} ========== to ========== Safe Browsing: CheckApiBlacklist request implementation BUG=561867 Committed: https://crrev.com/1343c9206db66bc5ec0a8aa67d745813b197cde4 Cr-Commit-Position: refs/heads/master@{#387869} ==========
Message was sent while issue was closed.
Committed patchset #11 (id:200001)
Message was sent while issue was closed.
Description was changed from ========== Safe Browsing: CheckApiBlacklist request implementation BUG=561867 Committed: https://crrev.com/1343c9206db66bc5ec0a8aa67d745813b197cde4 Cr-Commit-Position: refs/heads/master@{#387869} ========== to ========== Safe Browsing: CheckApiBlacklist request implementation BUG=561867 Committed: https://crrev.com/1343c9206db66bc5ec0a8aa67d745813b197cde4 Cr-Commit-Position: refs/heads/master@{#387869} Committed: https://crrev.com/5fad5b496bbd1cacea9591f2f79ec7b524ecf397 Cr-Commit-Position: refs/heads/master@{#388132} ==========
Message was sent while issue was closed.
Patchset 11 (id:??) landed as https://crrev.com/5fad5b496bbd1cacea9591f2f79ec7b524ecf397 Cr-Commit-Position: refs/heads/master@{#388132} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
