All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=765671

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=f0a461e403ccb036dde29a7e04e0eadc826502c1d2b2d67c6c66832ad96d6ae6


Bot(s) for this bug's original alert(s):

Android Builder

493 symbols added (+), 5 changed (~), 0 removed (-), 523621 unchanged (not shown)
Number of unique symbols 405033 -> 405304 (+271)
8 paths added, 0 removed, 2 changed

Showing 498 symbols (aliases not grouped for diffs) with total pss: 20088 bytes
.text=17.4kb     .rodata=2.00kb     .data.rel.ro=256 bytes  .data=0 bytes    .bss=0 bytes    total=19.6kb    
Number of unique paths: 10

Section Legend: t=.text, r=.rodata, R=.data.rel.ro, d=.data, b=.bss
Index | Running Total | Section@Address | Δ PSS (Δ size_without_padding) | Path
------------------------------------------------------------
~ 0)       1890 (9.3%)  r@Group      1890 (2892803->2894693) {{no path}}
               ** merge strings (count=7)
+ 1)       2476 (12.2%) t@0xeb6144   586 (0->584)       device/hid/public/interfaces/hid.mojom.cc
               device::mojom::HidConnectionStubDispatch::AcceptWithResponder
+ 2)       3050 (15.0%) t@0xeb6ae6   574 (0->574)       device/hid/public/interfaces/hid.mojom.cc
               mojo::internal::Serializer::Serialize
+ 3)       3578 (17.6%) t@0xeb5a20   528 (0->528)       device/hid/public/interfaces/hid.mojom.cc
               device::mojom::HidManagerStubDispatch::AcceptWithResponder
~ 4)       3996 (19.7%) t@Group      418 (0->0)         {{no path}}
               ** symbol gaps (count=9)
+ 5)       4334 (21.3%) t@0xeb31a4   338 (0->336)       device/hid/hid_connection.cc
               device::HidConnection::Write
+ 6)       4602 (22.6%) t@0xeb3368   268 (0->268)       device/hid/hid_connection.cc
               device::HidConnection::GetFeatureReport
+ 7)       4842 (23.8%) t@0xeb3474   240 (0->240)       device/hid/hid_connection.cc
               device::HidConnection::SendFeatureReport
+ 8)       5082 (25.0%) t@0xeb5e5c   240 (0->240)       device/hid/public/interfaces/hid.mojom.cc
               device::mojom::HidConnection_Read_ProxyToResponder::Run
+ 9)       5312 (26.1%) t@0xeb5fd4   230 (0->230)       device/hid/public/interfaces/hid.mojom.cc
               device::mojom::HidConnection_GetFeatureReport_ProxyToResponder::Run
+ 10)      5528 (27.2%) t@0xeb651c   216 (0->216)       device/hid/public/interfaces/hid.mojom.cc
               device::mojom::HidConnectionRequestValidator::Accept
+ 11)      5744 (28.2%) t@0xeb5008   216 (0->216)       device/hid/{{shared}}/2
               device::mojom::HidDeviceInfo::Clone const
+ 12)      5944 (29.2%) t@0xeb5808   200 (0->200)       device/hid/public/interfaces/hid.mojom.cc
               device::mojom::HidManager_GetDevicesAndSetClient_ProxyToResponder::Run
+ 13)      6144 (30.2%) t@0xeb58d0   200 (0->200)       device/hid/public/interfaces/hid.mojom.cc
               device::mojom::HidManager_GetDevices_ProxyToResponder::Run
+ 14)      6336 (31.2%) t@0xeb5d78   192 (0->192)       device/hid/public/interfaces/hid.mojom.cc
               device::mojom::HidManagerRequestValidator::Accept
+ 15)      6520 (32.1%) t@0xeb5344   184 (0->184)       device/hid/hid_service.cc
               device::HidService::RunPendingEnumerations
+ 16)      6696 (32.9%) t@0xeb5294   176 (0->176)       device/hid/hid_service.cc
               device::HidService::GetDevices
+ 17)      6860 (33.7%) t@0xeb44c0   164 (0->164)       device/hid/hid_manager_impl.cc
               mojo::BindingSetBase::Entry::Entry
+ 18)      7024 (34.5%) t@0xeb4dc4   164 (0->164)       device/hid/hid_manager_impl.cc
               mojo::StrongBinding::StrongBinding
+ 19)      7188 (35.3%) t@0xeb6de8   164 (0->164)       device/hid/public/interfaces/hid.mojom.cc
               mojo::internal::Serializer::Serialize
+ 20)      7350 (36.1%) t@0xeb3714   162 (0->160)       device/hid/hid_connection_impl.cc
               device::HidConnectionImpl::Write
+ 21)      7510 (36.9%) t@0xeb38e8   160 (0->160)       device/hid/hid_connection_impl.cc
               device::HidConnectionImpl::SendFeatureReport
+ 22)      7668 (37.7%) t@0xeb41a4   158 (0->158)       device/hid/hid_manager_impl.cc
               device::HidManagerImpl::CreateConnection
+ 23)      7818 (38.4%) t@0xeb56dc   150 (0->150)       device/hid/public/interfaces/hid.mojom.cc
               device::mojom::HidManagerClientProxy::DeviceAdded
+ 24)      7968 (39.2%) t@0xeb5772   150 (0->150)       device/hid/public/interfaces/hid.mojom.cc
               device::mojom::HidManagerClientProxy::DeviceRemoved
+ 25)      8104 (39.9%) t@0xeb3ecc   136 (0->136)       device/hid/hid_manager_impl.cc
               device::HidManagerImpl::CreateDeviceList
+ 26)      8240 (40.5%) t@0xeb60ba   136 (0->136)       device/hid/public/interfaces/hid.mojom.cc
               device::mojom::HidConnection_SendFeatureReport_ProxyToResponder::Run
+ 27)      8376 (41.2%) t@0xeb5f4c   136 (0->136)       device/hid/public/interfaces/hid.mojom.cc
               device::mojom::HidConnection_Write_ProxyToResponder::Run
+ 28)      8512 (41.9%) t@0xeb5998   136 (0->136)       device/hid/public/interfaces/hid.mojom.cc
               device::mojom::HidManager_Connect_ProxyToResponder::Run
+ 29)      8642 (42.5%) t@0xeb1a48   130 (0->128)       services/device/device_service.cc
               service_manager::CallbackBinder::BindInterface
+ 30)      8764 (43.1%) t@0xeb30fc   122 (0->120)       device/hid/hid_connection.cc
               device::HidConnection::Read
+ 31)      8884 (43.7%) t@0xeb712c   120 (0->120)       device/hid/public/interfaces/hid.mojom-shared.cc
               device::mojom::internal::HidConnection_SendFeatureReport_Params_Data::Validate
+ 32)      9004 (44.3%) t@0xeb70b4   120 (0->120)       device/hid/public/interfaces/hid.mojom-shared.cc


It's not clear to me whether or not this increase was expected. Please have a look and either:

  1. Close as “Won't Fix” with a short justification, or
  2. Land a revert / fix-up.

Deleted: diff_results.txt 81.5 KB

diff_results.txt 81.5 KB View Download

That CL added some new files and the mojom file, so it is in expect the binary size increases.

But, the Hid is not enabled for Android, I think the apk size shouldn't increase.

I have some following CLs that are still ongoing in which we will move the Hid into device service. I'll check this after all those CLs gets landed.

Just to be clear - //device/hid isn't used on Android? Is there any way we can exclude this from Android builds then? Or is the plan to enable this for Android soon?

Yes after it is moved into device service, I'll check and exclude it from Android builds.

No plan to enable it for Android.

ke.he, since there is still work to do on your next step patches can you prepare an isolated patch that removes //device/hid from the Android build? Sorry I didn't notice this change when reviewing your earlier patches.

Reilly, the //device/hid is already included in the Android build before our mojofication CL. I think the binary-size increasing is because of the new added mojom file and Hid{Connection|Manager}Impl files.

If we remove the //device/hid from Android build now, all the clients that depend on //device/hid (like u2f, extension) need to be changed. That's why I feel it might be better to do it after the //device/hid be moved into //services/device/hid. Then we only needs to change the //services/device/BUILD.gn. 

Finally the clients like u2f and extension only depend on the //service/public/{cpp|interfaces}, they don't need to take care on Android build flag at all.

Do you think it's ok? Thanks!

So strictly speaking, it is not a regression. It is a new feature but definitely we will resolve it:)

I have a change out that should fix this issue,

https://chromium-review.googlesource.com/c/chromium/src/+/674035

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

commit 170f5d42dab16c6733cadad4a8124068e5341c76
Author: Reilly Grant <reillyg@chromium.org>
Date: Wed Sep 20 01:26:54 2017

Remove Chrome dependency on //device/hid

Chrome only depends on HID support when built with extensions enabled.
This is reflected in the dependency in //extensions/browser and does not
need to be duplicated in //chrome/browser. This prevents HID support
from being compiled on Android where it is not used.

Bug:  765671 
Change-Id: Icefc679b216f2ea25bc89ed5b9d44d90e5ed6696
Reviewed-on: https://chromium-review.googlesource.com/674035
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503020}
[modify] https://crrev.com/170f5d42dab16c6733cadad4a8124068e5341c76/chrome/browser/BUILD.gn

This turns out to have regressed in an older change: https://chromium-review.googlesource.com/627738

I was checking dependencies wrong. There's another path through //services/device that I am cleaning up.

Hmm, looks like the bot we used was failing when https://chromium-review.googlesource.com/627738 landed and that CL got batched in with a bunch of others so we missed it. Thanks for following up on this!

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

commit 7211fa23c09493145ad8e231e4e166561e2a4c0c
Author: Reilly Grant <reillyg@chromium.org>
Date: Thu Sep 21 00:56:50 2017

Remove HID from the device service on Android

HID is not supported on Android so there's no reason to include all this
code. To prevent this from regressing an assert() is added to
device/hid/BUILD.gn which requires some updates to MockDeviceClient to
completely remove the dependency when building tests.

Bug:  765671 
Change-Id: Ieaab1f28594a7e0cdd177389d1bdb91159e868c9
Reviewed-on: https://chromium-review.googlesource.com/674050
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503303}
[modify] https://crrev.com/7211fa23c09493145ad8e231e4e166561e2a4c0c/device/base/BUILD.gn
[modify] https://crrev.com/7211fa23c09493145ad8e231e4e166561e2a4c0c/device/base/mock_device_client.cc
[modify] https://crrev.com/7211fa23c09493145ad8e231e4e166561e2a4c0c/device/base/mock_device_client.h
[modify] https://crrev.com/7211fa23c09493145ad8e231e4e166561e2a4c0c/device/hid/BUILD.gn
[modify] https://crrev.com/7211fa23c09493145ad8e231e4e166561e2a4c0c/services/device/BUILD.gn
[modify] https://crrev.com/7211fa23c09493145ad8e231e4e166561e2a4c0c/services/device/device_service.cc
[modify] https://crrev.com/7211fa23c09493145ad8e231e4e166561e2a4c0c/services/device/device_service.h

Confirmed drop in apk size on the performance dashboard.

Awesome!

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

commit c3855d4a217284cc180a0382c7a19cbe403983fe
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Thu Sep 21 02:03:02 2017

Revert "Remove HID from the device service on Android"

This reverts commit 7211fa23c09493145ad8e231e4e166561e2a4c0c.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 503303 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzcyMTFmYTIzYzA5NDkzMTQ1YWQ4ZTIzMWU0ZTE2NjU2MWUyYTRjMGMM

Sample Failed Build: https://luci-milo.appspot.com/buildbot/chromium/Android/76380

Original change's description:
> Remove HID from the device service on Android
> 
> HID is not supported on Android so there's no reason to include all this
> code. To prevent this from regressing an assert() is added to
> device/hid/BUILD.gn which requires some updates to MockDeviceClient to
> completely remove the dependency when building tests.
> 
> Bug:  765671 
> Change-Id: Ieaab1f28594a7e0cdd177389d1bdb91159e868c9
> Reviewed-on: https://chromium-review.googlesource.com/674050
> Reviewed-by: Ken Rockot <rockot@chromium.org>
> Commit-Queue: Reilly Grant <reillyg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#503303}

Change-Id: If55149984005054e69ab7a8a704c35fd85d11514
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  765671 
Reviewed-on: https://chromium-review.googlesource.com/675757
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503309}
[modify] https://crrev.com/c3855d4a217284cc180a0382c7a19cbe403983fe/device/base/BUILD.gn
[modify] https://crrev.com/c3855d4a217284cc180a0382c7a19cbe403983fe/device/base/mock_device_client.cc
[modify] https://crrev.com/c3855d4a217284cc180a0382c7a19cbe403983fe/device/base/mock_device_client.h
[modify] https://crrev.com/c3855d4a217284cc180a0382c7a19cbe403983fe/device/hid/BUILD.gn
[modify] https://crrev.com/c3855d4a217284cc180a0382c7a19cbe403983fe/services/device/BUILD.gn
[modify] https://crrev.com/c3855d4a217284cc180a0382c7a19cbe403983fe/services/device/device_service.cc
[modify] https://crrev.com/c3855d4a217284cc180a0382c7a19cbe403983fe/services/device/device_service.h

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

commit c3f8f215a9d7973ffb8dbb051acaba6e60d1ac3e
Author: Reilly Grant <reillyg@chromium.org>
Date: Thu Sep 21 05:22:28 2017

Reland "Remove Chrome dependency on //device/hid"

Chrome only depends on HID support when built with extensions enabled.
This is reflected in the dependency in //extensions/browser and does not
need to be duplicated in //chrome/browser. This prevents HID support
from being compiled on Android where it is not used.

TBR=rockot@chromium.org

Bug:  765671 
Change-Id: I127d71265283d836803e7e1acbadc5d93db75f81
Reviewed-on: https://chromium-review.googlesource.com/675847
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503360}
[modify] https://crrev.com/c3f8f215a9d7973ffb8dbb051acaba6e60d1ac3e/chrome/browser/chrome_device_client.cc
[modify] https://crrev.com/c3f8f215a9d7973ffb8dbb051acaba6e60d1ac3e/chrome/browser/chrome_device_client.h
[modify] https://crrev.com/c3f8f215a9d7973ffb8dbb051acaba6e60d1ac3e/device/base/BUILD.gn
[modify] https://crrev.com/c3f8f215a9d7973ffb8dbb051acaba6e60d1ac3e/device/base/mock_device_client.cc
[modify] https://crrev.com/c3f8f215a9d7973ffb8dbb051acaba6e60d1ac3e/device/base/mock_device_client.h
[modify] https://crrev.com/c3f8f215a9d7973ffb8dbb051acaba6e60d1ac3e/device/hid/BUILD.gn
[modify] https://crrev.com/c3f8f215a9d7973ffb8dbb051acaba6e60d1ac3e/services/device/BUILD.gn
[modify] https://crrev.com/c3f8f215a9d7973ffb8dbb051acaba6e60d1ac3e/services/device/device_service.cc
[modify] https://crrev.com/c3f8f215a9d7973ffb8dbb051acaba6e60d1ac3e/services/device/device_service.h

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

commit 12795036c8e14045aaeef8b18161c8a4d63b0301
Author: Reilly Grant <reillyg@chromium.org>
Date: Thu Sep 21 06:26:22 2017

Revert "Reland "Remove Chrome dependency on //device/hid""

This reverts commit c3f8f215a9d7973ffb8dbb051acaba6e60d1ac3e.

Reason for revert: Broke Android compile again.

Original change's description:
> Reland "Remove Chrome dependency on //device/hid"
> 
> Chrome only depends on HID support when built with extensions enabled.
> This is reflected in the dependency in //extensions/browser and does not
> need to be duplicated in //chrome/browser. This prevents HID support
> from being compiled on Android where it is not used.
> 
> TBR=rockot@chromium.org
> 
> Bug:  765671 
> Change-Id: I127d71265283d836803e7e1acbadc5d93db75f81
> Reviewed-on: https://chromium-review.googlesource.com/675847
> Commit-Queue: Reilly Grant <reillyg@chromium.org>
> Reviewed-by: Reilly Grant <reillyg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#503360}

TBR=rockot@chromium.org,reillyg@chromium.org

Change-Id: I27f63a497c20363ef427279871aee34d49723c00
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  765671 
Reviewed-on: https://chromium-review.googlesource.com/676544
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503366}
[modify] https://crrev.com/12795036c8e14045aaeef8b18161c8a4d63b0301/chrome/browser/chrome_device_client.cc
[modify] https://crrev.com/12795036c8e14045aaeef8b18161c8a4d63b0301/chrome/browser/chrome_device_client.h
[modify] https://crrev.com/12795036c8e14045aaeef8b18161c8a4d63b0301/device/base/BUILD.gn
[modify] https://crrev.com/12795036c8e14045aaeef8b18161c8a4d63b0301/device/base/mock_device_client.cc
[modify] https://crrev.com/12795036c8e14045aaeef8b18161c8a4d63b0301/device/base/mock_device_client.h
[modify] https://crrev.com/12795036c8e14045aaeef8b18161c8a4d63b0301/device/hid/BUILD.gn
[modify] https://crrev.com/12795036c8e14045aaeef8b18161c8a4d63b0301/services/device/BUILD.gn
[modify] https://crrev.com/12795036c8e14045aaeef8b18161c8a4d63b0301/services/device/device_service.cc
[modify] https://crrev.com/12795036c8e14045aaeef8b18161c8a4d63b0301/services/device/device_service.h

I'm trying again to land this fix.

https://chromium-review.googlesource.com/c/chromium/src/+/677043

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

commit 5ac479c1b4168339df12d8fc49ddd761977a91bf
Author: Reilly Grant <reillyg@chromium.org>
Date: Tue Oct 03 16:57:37 2017

Reland "Remove HID from the device service on Android"

HID is not supported on Android so there's no reason to include all this
code.

This change has been hard to land because removing a generated file
from the build can cause false negatives on the trybots because the
file still exists in their build directory.

Bug:  765671 
Change-Id: I28bdaa06d246a188a0486502d29a55718545a94b
Reviewed-on: https://chromium-review.googlesource.com/677043
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506075}
[modify] https://crrev.com/5ac479c1b4168339df12d8fc49ddd761977a91bf/chrome/browser/chrome_device_client.cc
[modify] https://crrev.com/5ac479c1b4168339df12d8fc49ddd761977a91bf/chrome/browser/chrome_device_client.h
[modify] https://crrev.com/5ac479c1b4168339df12d8fc49ddd761977a91bf/device/BUILD.gn
[modify] https://crrev.com/5ac479c1b4168339df12d8fc49ddd761977a91bf/device/base/BUILD.gn
[modify] https://crrev.com/5ac479c1b4168339df12d8fc49ddd761977a91bf/device/base/mock_device_client.cc
[modify] https://crrev.com/5ac479c1b4168339df12d8fc49ddd761977a91bf/device/base/mock_device_client.h
[modify] https://crrev.com/5ac479c1b4168339df12d8fc49ddd761977a91bf/device/hid/BUILD.gn
[modify] https://crrev.com/5ac479c1b4168339df12d8fc49ddd761977a91bf/device/test/test_device_client.cc
[modify] https://crrev.com/5ac479c1b4168339df12d8fc49ddd761977a91bf/device/test/test_device_client.h
[modify] https://crrev.com/5ac479c1b4168339df12d8fc49ddd761977a91bf/device/u2f/BUILD.gn
[modify] https://crrev.com/5ac479c1b4168339df12d8fc49ddd761977a91bf/services/device/BUILD.gn
[modify] https://crrev.com/5ac479c1b4168339df12d8fc49ddd761977a91bf/services/device/device_service.cc
[modify] https://crrev.com/5ac479c1b4168339df12d8fc49ddd761977a91bf/services/device/device_service.h

Verified again that this drop is reflected in the perf graphs.

Thanks for your persistence on this reillyg@!