New issue
Advanced search Search tips

Issue 857693 link

Starred by 1 user
Status: Fixed
Owner:
pauljensen@chromium.org
Closed: Aug 16
Cc:
rsleevi@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Remove setCertVerifierData() method in ICronetEngineBuilder

Project Member Reported by kapishnikov@chromium.org, Jun 28 2018 
After (and only after) the "Remove the Cronet CachingCertVerifier experiment"[1] CL lands, it should be safe to remove the setCertVerifierData() method in ICronetEngineBuilder. However, the presubmit checks assume that methods should never be removed from the API layer, which is 'true' in general. We should revisit the procedure of handling the exception cases. It is reasonable to expect that some other methods in the Experimental engine will be added and deprecated in the future.


[1] https://chromium-review.googlesource.com/c/chromium/src/+/1117036

Comment 1 by pauljensen@chromium.org, Jul 24

Cc: -pauljensen@chromium.org
Owner: pauljensen@chromium.org
Status: Started (was: Untriaged)
Started in:
https://chromium-review.googlesource.com/c/chromium/src/+/1148019

This kind of change has a high risk of breaking Cronet embedders so I don't want it to be easy to manipulate the checks into allowing.  If it absolutely must be done, update_api.py's check_api_update() can be simply made to return True.
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 16 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f2f8e8c2fe9c23edc59dd9441bf5b77fdba2ca56

commit f2f8e8c2fe9c23edc59dd9441bf5b77fdba2ca56
Author: Paul Jensen <pauljensen@chromium.org>
Date: Thu Aug 16 15:40:11 2018

[Cronet] Remove ICronetEngineBuilder.setCertVerifierData()

This API was entirely unused so it was allowed to be removed from the API
previously in crrev.com/571588. This CL cleans up the ICronetEngineBuilder
and api.txt references as they're no longer needed.  Removing an API
isn't normally allowed, so this required temporary modification of the
API update checks to rebuild api.txt.

Bug:  857693 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Change-Id: I1bffb016f781aeb5116d8507245630dedacbb14b
Reviewed-on: https://chromium-review.googlesource.com/1148019
Reviewed-by: Misha Efimov <mef@chromium.org>
Commit-Queue: Paul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583665}
[modify] https://crrev.com/f2f8e8c2fe9c23edc59dd9441bf5b77fdba2ca56/components/cronet/android/api.txt
[modify] https://crrev.com/f2f8e8c2fe9c23edc59dd9441bf5b77fdba2ca56/components/cronet/android/api/src/org/chromium/net/ICronetEngineBuilder.java
[modify] https://crrev.com/f2f8e8c2fe9c23edc59dd9441bf5b77fdba2ca56/components/cronet/android/api_version.txt

Comment 3 by pauljensen@chromium.org, Aug 16

Status: Fixed (was: Started)
►

Sign in to add a comment