Commit Graph

640 Commits

Author SHA1 Message Date
John W. Bruce
ca00dc7ae4 Simplify How Request ID Indices are Generated
(This is a merge of http://go/wvgerrit/70667)

Request ID Index generation has historically worked by incrementing a
shared variable in one place and reading it in another place and
trusting the fact that CdmLicense calls these operations in a certain
order and only once per session to give each session a unique value.
This patch cleans this up a bit, having each session store the current
Request ID Index at the same time as it stores its Request ID Base. This
guarantees that each CryptoSession will receive a unique but stable
combination of Base and ID rather than relying on the calling pattern.

Since all this generation happens during the same function, the full
Request ID can be generated up-front and stored, making
GenerateRequestId() no longer necessary.

This patch also simplifies the threading story around this shared state
by using a std::atomic<uint64_t>. Bringing the code that interacts with
the shared state together into one place and replacing it with atomic
operations will simplify locking around this code when CryptoSession
locking is revamped in a future patch.

Bug: 70889998
Bug: 118584039
Test: CE CDM Unit Tests
Test: Android Unit Tests
Change-Id: I12d2f6501f872f1973e5a9af5125ca03f23e5a56
2019-01-23 09:52:46 -08:00
Adam Stone
700ee5160a Add session metric pruning.
[ Merge of http://go/wvgerrit/65983 ]

Add support to drop closed metrics in order to save space for
long-running applications (and CdmEngine instances). The code now keeps
only a limited number of metrics collections after the session is closed.
As a session (and its metrics session) is closed, the oldest, closed metrics
session is dropped. This means those metrics will not be reported nor
accessible in the client.

Bug: http://b/118664842

Test: CDM Unit tests. Android Unit Test. Ran GPLay Manually.
Change-Id: I27d6e61a8fe4148ad1ef2a433c8e5f4cdd84cc72
2019-01-17 13:30:11 -08:00
Srujan Gaddam
92e123d8ea Merge "Fix handling of INSUFFICIENT_RESOURCES in LoadEntry" 2019-01-17 03:33:49 +00:00
John Bruce
0c194ff115 Merge "Re-enable Binary Provisioning Test Again" 2019-01-17 00:30:42 +00:00
John Bruce
3b328bb896 Merge "Make Properties::Init() Take Action Only Once" 2019-01-17 00:30:07 +00:00
John W. Bruce
777abaef01 Add some platform abstractions.
(This is a merge of http://go/wvgerrit/70303)

This adds a platform.h file to abstract some of the differences
between Windows and POSIX platforms.  This includes ntohl, setenv,
and ssize_t.

Bug: 122953649
Test: Android Unit Tests
Change-Id: I3235f3f284b53d24d7365ff3f4a06dcd9b403697
2019-01-16 11:06:55 -08:00
John W. Bruce
e12a837e14 Re-enable Binary Provisioning Test Again
(This is a merge of http://go/wvgerrit/70384)

It turns out the reason the binary provisioning unit test was failing is
because the test base class was setting the property to turn on binary
provisioning before calling Init(), however all current Init()
implementations overwrite the value of that field. As such, the tests
weren't actually using binary provisioning. With that fixed, everything
passes; the binary provisioning flow doesn't appear to actually be
broken.

This is the exact same commit as was previously reverted,
cf5464d7a2fbecd1938ae0700199145b7b61c3c3. However, a pending patch fixes
the multiple runs of Properties::Init() that lead to the inconsistent
failures of the previous patch. (See http://go/wvgerrit/70383)

Bug: 112046733
Test: CE CDM Unit Tests
Test: Android Unit Tests
Change-Id: I79fc2c8d4d24505b46cab0e21eef85a37d66748d
2019-01-15 16:28:01 -08:00
John W. Bruce
86efc7534a Make Properties::Init() Take Action Only Once
(This is a merge of http://go/wvgerrit/70383)

Up until now, implementations of Properties::Init() have had to handle
potentially being called multiple times, at any point during runtime. In
practice, this has meant little for the actual implementations, and all
of them have committed the error of blowing away mutated property state
if the method is re-run at the wrong time.

This patch makes the platform implementations a private function,
Properties::InitOnce(), which Properties::Init() ensures will never be
called more than once per run.

Bug: 112046733
Test: CE CDM Unit Tests
Test: Android Unit Tests
Change-Id: If33f5e37abfad5d26da8380b4bc25fc018450970
2019-01-15 16:27:57 -08:00
Rahul Frias
5d6c12ff5d Add additional logging to Cdm Engine
[ Merge of http://go/wvgerrit/69965 ]

Bug: 120786527
Test: WV unit/integration tests
Change-Id: I8fea89b25435c3a3b632155fe6a2b07349b63a19
2019-01-11 14:01:49 -08:00
Srujan Gaddam
13b5c48512 Fix handling of INSUFFICIENT_RESOURCES in LoadEntry
Bug: b/121214641
Merge of http://go/wvgerrit/69703
Test: Android + Linux unit tests

LoadEntry attempts to handle INSUFFICIENT_RESOURCES by deleting an entry
and retrying, but it's possible that the randomly-generated number of
the entry to be deleted might match the entry we want to load. In this
case, we have wasted a retry, since the code just continues on to the
next iteration. This is changed to generate a number different from the
entry to load. Furthermore, if the number of usage entries is 1, we
break since there are no more entries to delete besides the one we want
to load. The code is also changed to call srand in the creation of the
usage_table_header, since without it, rand() would produce the same
values, and similarly, our random generation is changed to use a simple
mod. Tests are modified to reflect these changes.

Change-Id: I95e125b8adbd85d0189f9d40ca15f3fe69e6d6b9
2019-01-11 11:39:54 -08:00
Rahul Frias
22e991343b Allow queries without opening a session
[ Merge of http://go/wvgerrit/69724 ]

Some queries no longer require a session to be opened before they
can be answered - security level, current HDCP level, max HDCP level,
usage support, number of open sessions, max sessions,
OEMCrypto API version, current SRM version, SRM update support,
resource rating tier and OEMCrypto build information.

b/117104043
Test: WV unit/integration tests

Change-Id: I92f8249e5599860da8cbf42d3b16f25515a46c55
2019-01-10 09:01:46 -08:00
John Bruce
38c45a66a3 Revert "Re-enable Binary Provisioning Test"
(The matching reversion in the Widevine Repository is
http://go/wvgerrit/69923)

This reverts commit c207ef8ea6.

Reason for revert: Widevine Buildbot failures

Change-Id: Ied9e36f9eaa94bb7e9cd13b8dff7a2cbdca07a07
2019-01-09 18:55:06 +00:00
John W. Bruce
c207ef8ea6 Re-enable Binary Provisioning Test
(This is a merge of http://go/wvgerrit/69843)

It turns out the reason the binary provisioning unit test was failing is
because the test base class was setting the property to turn on binary
provisioning before calling Init(), however all current Init()
implementations overwrite the value of that field. As such, the tests
weren't actually using binary provisioning. With that fixed, everything
passes; the binary provisioning flow doesn't appear to actually be
broken.

Bug: 112046733
Test: CE CDM Unit Tests
Test: Android Unit Tests
Change-Id: I7413f7fb2227e596fb610d6ddc5b95cda2f406b8
2019-01-08 18:08:26 -08:00
Srujan Gaddam
2870210598 Merge changes I09cec674,Iee28fa90,I1ef35e15
* changes:
  Modify Level 3 full decrypt path testing for 15.1
  Remove OEMCrypto_InitializeDecryptHash
  Update OEMCrypto documentation to 15.1
2019-01-08 17:32:14 +00:00
Rahul Frias
589a3cf27e Add decrypt hash support
[ Merge of http://go/wvgerrit/68083 ]

Add ability to query decrypt hash support, set a hash computed over a frame
and retrieve the last error at a later point.

Bug: 34080802
Test: WV unit/integration tests. New tests added to cdm_engine_test,
      libwvdrmdrmplugin_hidl_test and request_license_test.

Change-Id: I7548c8798c873a6af3e1cfc0df57c117e1e474a6
2019-01-07 10:14:25 -08:00
Srujan Gaddam
ff307a7c0e Modify Level 3 full decrypt path testing for 15.1
Merge of http://go/wvgerrit/68986
Bug: b/120797208
Test: Android + Linux unit tests

OEMCrypto v15.1 introduced changes to full decrypt path testing.
This CL reflects those changes for the Level 3 code, including
removing InitializeDecryptHash and changes to error reporting.

Change-Id: I09cec6743524d326cb1a6c3ba4dd1764dbefff5f
2019-01-07 09:53:08 -08:00
Fred Gylys-Colwell
9c95e4caae Remove OEMCrypto_InitializeDecryptHash
Merge from Widevine repo of http://go/wvgerrit/68464

The Full Decrypt Path Testing design has changed to remove
OEMCrypto_InitializeDecryptHash.  This CL updates the unit tests and
reference code.

Bug: 120795057
Test: unit tests

Change-Id: Iee28fa9034dc21cee81c5b894c192e260375eeee
2019-01-07 09:53:07 -08:00
Edwin Wong
47c2b65cba Cleanup unaligned code.
Merge from: http://go/wvgerrit/69211

Cleanup unaligned code while working on b/117570686 and remove
a duplicate break statement.

Test: unit tests
  ./run_all_unit_tests.sh

Bug: 121971420
Change-Id: I0ca1dc0fe54e7fe3a87aa7c8dac7ef9da04e90b2
2018-12-24 18:31:12 -08:00
Adam Stone
27e26110b4 Add Oemcrypto build information to metrics.
Import from http://go/wvgerrit/68385

Adds the build information returned from OEMCrypto_BuildInformation()
to the CDM session metrics.

Bug: 117117555
Test: Unit tests. GPlay manual. GTS Tests.
Change-Id: I505c46fec61a7c62538f843185ec0358f860da79
2018-12-21 13:09:59 -08:00
Adam Stone
0a64d25067 Add a field indicating online vs offline licenses.
Import of http://go/wvgerrit/68188

This adds an attribute to metrics indicating if the license was online
or offline.

Also, added a unit test for CdmEngineMetricsImpl.

Test: Unit tests. GPlay manual. GTS tests.
Bug: 115523917

Change-Id: Id315c643048914a2c51904451f9665987bc87eb7
2018-12-21 13:09:29 -08:00
Srujan Gaddam
4c5c4caf66 Add detail about L3 initialization
Bug: b/70299597
Merge of http://go/wvgerrit/67304
Test: Android, CE CDM, and Linux tests

There's a few different things that can go wrong in the L3
initialization, with seeding and device key failures among others. They
should be recorded in metrics to track. Along the same lines, since
multiple errors can happen in conjunction, metrics needs to change to
add more fields for errors. This CL also adds the
hidl_metrics_adapter_unittest to the Android test scripts.

Change-Id: Ie5bcf81bbe294a1136c58410f90087a13b3d911d
2018-12-14 11:32:19 -08:00
Rahul Frias
0e28104cff Change from custom Lock to std::mutex.
[ Merge of http://go/wvgerrit/67884 ]

Now that we can use C++11, we should use the cross-platform std::mutex
type, not the custom pthread version.

Bug: 111850982
Test: WV unit/integration tests
Change-Id: If2fde2836826c5184609e6b1f3a6511206bd4594
2018-12-13 11:55:54 -08:00
Rahul Frias
65c64292b7 Merge "Enforce OEMCrypto insufficient resources error reporting" 2018-12-12 16:47:11 +00:00
Srujan Gaddam
b10d5c2a8f Merge changes from topic "level_3_oecv15"
* changes:
  Changes to adapter + gyp to support v15
  Add full decrypt path testing functionality to Level 3
  Update pointers to substrings in Level 3
  Update level 3 reference API for v15
2018-12-11 19:54:39 +00:00
Rahul Frias
afd11c0da5 Add support for Resource Rating Tiers
[ Merge of http://go/wvgerrit/67324 ]

This introduces the ability to query resource rating tier information
through the plugin and CDM. Resource rating tiers are also
sent in the client identification portion of the license request.

Bug: 117112392
Test: WV unit/integration tests
Change-Id: I68ac6dfc4362f61150af822bd526e346b5cc4bf7
2018-12-11 01:43:58 -08:00
Rahul Frias
e01b559bb7 Update ClientIdentification in proto
[ Merge of http://go/wvgerrit/67323 ]

This allows the client to report resource rating tier in
ClientIdentification.

Bug: 117112392
Test: WV unit/integration test
Change-Id: I5bcbd5217c376cb52be2548ee63e0b0232b411d3
2018-12-11 01:40:11 -08:00
Rahul Frias
5f96a20d99 Allow querying of OEMCrypto build information
[ Merge of http://go/wvgerrit/67503 ]

Bug: 117118184
Test: WV unit/integration tests
Change-Id: I8db90c1617e068a4711eb41a5852d15b5e228f2f
2018-12-10 21:48:13 -08:00
Rahul Frias
f98fbbb187 Enforce OEMCrypto insufficient resources error reporting
[ Merge of http://go/wvgerrit/63682 and http://go/wvgerrit/4977904 ]

Enforce OEMCrypto insufficient resources error reporting in CDM tests.
In addition, when CreateNew/LoadUsageEntry fails with OEMCrypto
insufficient resources error, delete a random usage entry
and retry. The current behavior evicts low index entries
from the usage table, which was a crude attempt to emulate a LRU.
This was deficient as, on occasion, it will result in the deletion
of a recently added usage entry.

Bug: 111260263
Bug: 113828866
Bug: 120433165

Test: Widevine OEMCrypto tests, integration tests. GtsMediaTestCases.
      Play movies and Netflix playback tests.
      GtsMediaTestCases MediaDrmTest#testUsageTableCapacity

Change-Id: I63340f76d1e2af3c6834b98ad816e11eea18fc7f
2018-12-10 00:09:56 -08:00
John Bruce
dbef7f34ac Merge changes Ic23e2e48,I5d25844b
* changes:
  Google-style override & virtual
  Remove Sub-Licenses and Sub-Sessions
2018-12-07 00:35:51 +00:00
Srujan Gaddam
732c077f79 Changes to adapter + gyp to support v15
Bug: b/117558570
Test: Android, CE CDM, and Linux tests
Merge of http://go/wvgerrit/67566

This CL adds changes to support the Level 3 upgrade to v15 with both the
dynamic and static adapters. It also rearranges the wvcrc.cpp files so
that the tests have their own copy and Level 3 can just use the one in
ref/src.

Change-Id: Ieee2859601881aa1800622454e6ce0345eb94aa7
2018-12-06 23:04:12 +00:00
John W. Bruce
92cf993cf5 Add Missing Includes Netflix Pointed Out
(This is a merge of http://go/wvgerrit/66810)

Netflix discovered that several files were only compiling because of
transitive includes via the Metrics code. This patch adds the missing
headers they noted.

Bug: 118676365
Test: CE CDM Build
Change-Id: Ifbc4e5d4276d1c3fb9bbd677230cd431e34e5c76
2018-12-06 09:31:42 -08:00
John W. Bruce
b771d93514 Google-style override & virtual
(This is a merge of http://go/wvgerrit/66625)

Google C++ Style dictates that methods which override base class or
interface methods should be declared "override" but not "virtual". Since
our codebase has not had access to "override" until now, many of our
classes do not follow this rule. I've updated as many places as I could
find to follow Google C++ Style, which should hopefully help us catch
errors better in the future.

Bug: 111851141
Test: CE CDM Unit Tests
Test: Android Unit Tests
Change-Id: Ic23e2e482e967256da306791532b5fec7b81b2f2
2018-12-06 09:01:18 -08:00
John W. Bruce
5629a646d8 Remove Sub-Licenses and Sub-Sessions
(This is a merge of http://go/wvgerrit/66643)

The sub-license feature has been removed from the server and packager.
So that we do not have to continue maintaining the code that supports
this feature that never shipped, I am removing it from the CDM as well.

Bug: 113165466
Test: CE CDM Unit Tests
Test: Android Unit Tests
Change-Id: I5d25844b161e74aa19adf19a29c56e4881aa7304
2018-12-06 09:00:51 -08:00
Srujan Gaddam
896ce2b5aa Refactor file_store to use smart pointers
Bug: b/119276649
Merge from: http://go/wvgerrit/66367
Test: Android, CE CDM, Linux unit tests

The FileSystem interface as it exists expects an Open for a file and
then a Close when finished. However, the Close doesn't delete the file
itself and depending on the platform, the underlying impl_ as well,
leading to a memory leak. To fix this leak as well as harden against
future memory issues, this change refactors the interface to shift away
from raw pointers and towards smart pointers.

Change-Id: I7a7132ea95cd3775796a540f510b698f4f27dd24
2018-11-30 16:25:58 -08:00
Fred Gylys-Colwell
e59b976625 Update comments and unit tests for InstallKeyboxOrCert
Merge from Widevine repo of http://go/wvgerrit/66865

This CL updates the comments for the code that uses
InstallKeyboxOrCert to install a keybox or oem cert at runtime.  There
is also a new unit test for the device id.  There are not many unit
tests because we don't want to accidentally overwrite the production
keybox or cert on a device when running tests.

bug: 111725154
test: unit tests
Change-Id: Ide9b3d4732f3f96a723a128907babe7e59c02ffc
2018-11-26 14:08:17 -08:00
Rahul Frias
7beac0352e Merge "Address NTS TCs AUDIO-026-* failures" 2018-11-26 18:28:43 +00:00
Rahul Frias
14978a0832 Address NTS TCs AUDIO-026-* failures
[ Merge of http://go/wvgerrit/66083 and http://go/ag/5445191 ]

Fix proposed by amlogic. Release usage session early in the CdmEngine
destructor to avoid SIGBUS.

Bug: 118646062
Test: WV unit/integration tests.

Change-Id: I8b419c57814dc32e9873173ab6c24d209bdcc3a0
2018-11-21 13:43:15 -08:00
Fred Gylys-Colwell
3a3307242d Fix for off by one error and correct comments
Merge from Widevine repo of http://go/wvgerrit/66711

This addresses review comments for the OEMCrypto v15 CLs, as documented in
b/119685652.

Test: unit tests
Bug: 119685652
Change-Id: I0e7ce13dd80335eb1a12240df5c1df2c34ba4298
2018-11-18 14:00:28 -08:00
Rahul Frias
dd55fefe87 Add support for HDCP 2.3
[ Merge of http://go/wvgerrit/66283 ]

Bug: 78773763
Test: WV unit/integration tests

Change-Id: I5050295a7aa6d829a66adce5d5de86b725a82738
2018-11-16 23:46:27 -08:00
Rahul Frias
d8cc5481e3 Update license_protocol and client_identification protos
[ Merge of http://go/wvgerrit/65442 ]

This also requires the removal of sub session related code as references
were removed from the proto.

Bug: 119077124
Test: WV unit/integration tests
Change-Id: Ida1a591afc267ec97344e5bba00bbf401887a202
2018-11-16 23:44:43 -08:00
Fred Gylys-Colwell
246621c5ce Full decrypt path testing
Merge from master branch of Widevine repo of http://go/wvgerrit/66080
Merge from oemcrypto-v15 branch of Widevine repo of http://go/wvgerrit/64002

This CL updates OEMCrypto reference code and unit tests to support full decrypt
path testing.

Test: unit tests
Test: tested as part of http://go/ag/5501993
Bug: 34078913
Change-Id: Ia67374599d6619698a336f41513068ad04294e7f
2018-11-16 17:14:00 -08:00
Fred Gylys-Colwell
4fa255ea51 Add Sandbox ID support
Merge from master branch of Widevine repo of http://go/wvgerrit/66078
Merge from oemcrypto-v15 branch of Widevine repo of http://go/wvgerrit/64022

This CL updates OEMCrypto ref code, unit tests, and core code for
setting the sandbox id before initializing OEMCrypto.

Test: unit tests only
Test: tested as part of http://go/ag/5501993
Bug: 115834255
Change-Id: Id9831680fe4db1c69413815931cae4bc80df0c01
2018-11-16 17:14:00 -08:00
Srujan Gaddam
926a780397 Resize mac_key string to 2 * MAC_KEY_SIZE
Merge from master branch of Widevine repo of http://go/wvgerrit/66074
Merge from oemcrypto-v15 branch of Widevine repo of http://go/wvgerrit/65102

Currently, the string only contains the mac_key_server, which isn't an
issue currently because we just get the offset into the message for the
enc_mac_keys pointer when calling LoadKeys, and rely on OEMCrypto to
read the 64 bytes. However, v15 will need the length of the enc_mac_keys
to reflect the true size.

Test: tested as part of http://go/ag/5501993
Bug: 115874964

Change-Id: Id76e91feb176755467c0433e6b0e87d2bb221033
2018-11-16 17:14:00 -08:00
Srujan Gaddam
e6439255ba Update OEMCrypto calls to use substrings
Merge from master branch of Widevine repo of http://go/wvgerrit/66073
Merge from oemcrypto-v15 branch of Widevine repo of http://go/wvgerrit/64083

As part of the update to v15, LoadKeys, RefreshKeys, and
LoadEntitledContentKeys should all use offsets and lengths into the
message rather than a pointer for its parameters. The CDM, tests,
adapters, and OEMCrypto implementations are changed to reflect this.

Test: tested as part of http://go/ag/5501993
Bug: 115874964

Change-Id: I981fa322dec7c565066fd163ca5775dbff71fccf
2018-11-16 17:14:00 -08:00
Fred Gylys-Colwell
4550979f22 OEMCrypto Resource Rating
Merge from master branch of Widevine repo of http://go/wvgerrit/66072
Merge from oemcrypto-v15 branch of Widevine repo of http://go/wvgerrit/63764

This adds the function OEMCrypto_ResourceRatingTier to the oemcrypto referenece
code, dynamic adapter, and unit tests.

Bug: 117110800
Test: tested as part of http://go/ag/5501993

Change-Id: Idf47af405f0c69601108b75c788a97b30abdb39d
2018-11-16 17:14:00 -08:00
Fred Gylys-Colwell
43a47a60c0 Add session parameter to OEMCrypto_CopyBuffer
Merge from master branch of Widevine repo of http://go/wvgerrit/66070
Merge from oemcrypto-v15 branch of Widevine repo of http://go/wvgerrit/63662

To make the threading model more clear, CopyBuffer is now a session function.
This means we need to pass in which session the current thread locks.

Test: unit tests.
Test: tested as part of http://go/ag/5501993
Bug: 113680369
Change-Id: I2fdd2cfcaab99f3793950b3845941463675f5e4c
2018-11-16 17:14:00 -08:00
Fred Gylys-Colwell
ef067572bc Remove error OEMCrypto_KEY_NOT_LOADED
Merge from master branch of Widevine repo of http://go/wvgerrit/66066
Merge from oemcrypto-v15 branch of Widevine repo of http://go/wvgerrit/63628

The error code OEMCrypto_KEY_NOT_LOADED is redundant with
OEMCrypto_ERROR_NO_CONTENT_KEY and OEMCrypto_KEY_NOT_ENTITLED.  The
function LoadEntitledContentKey should return KEY_NOT_ENTITLED if it
does not find the corresponding entitlement key in its key table.  All
other functions that do not find a key id in the key table should
return OEMCrypto_ERROR_NO_CONTENT_KEY.  This includes QueryKeyControl,
SelectKey, and RefreshKeys.

Test: unit tests
Test: tested as part of http://go/ag/5501993
Bug: 115574797
Change-Id: Ida2111f32e331b99f3f0c77fa404a42654d0870c
2018-11-16 17:09:32 -08:00
Fred Gylys-Colwell
f3e9d84484 Add build info API to OEMCrypto
Merge from master branch of Widevine repo of http://go/wvgerrit/66065
Merge from oemcrypto-v15 branch of Widevine repo of http://go/wvgerrit/63080

This is for the reference code, the unit tests, and the oemcrypto adapter.

Bug: 116414218
Test: unit tests
Test: tested as part of http://go/ag/5501993

Change-Id: I05a631f6cfcf1584a748b3a0c9ae48633893589f
2018-11-16 15:38:45 -08:00
Fred Gylys-Colwell
f2edb8f2dd Update API version to 15
Merge from master branch of Widevine repo of http://go/wvgerrit/66064
Merge from oemcrypto-v15 branch of Widevine repo of http://go/wvgerrit/63063

This is in the reference code for OEMCrypto, and in the unit tetss.

Bug: 111939411
Test: unit tests
Test: tested as part of http://go/ag/5501993

Change-Id: I2cc2e7028f62d1c375eb632452eef94566fa9ae3
2018-11-16 15:38:45 -08:00
Fred Gylys-Colwell
05fc8b5e97 Temporarily use old test keybox
Merge from Widevine repo of http://go/wvgerrit/65922

This CL puts the old 4121 test keybox back into the unit and android
integration tests so that we can run tests on a device with OEMCrypto v13.

This change should not be released, and should not be included in CE CDM.  It
should be removed once we have more test devices with v14 or v15.

Bug: 119313532
Bug: 119316243
Test: unit tests
Test: tested as part of http://go/ag/5501993

Change-Id: If6a459e11176e07c66fbe6fc45c63d87595a20dc
2018-11-16 15:38:45 -08:00