New issue
Advanced search Search tips

Issue 756302 link

Starred by 1 user
Status: Fixed
Owner:
mthiesse@chromium.org
Closed: Aug 2017
Cc:
estevenson@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Static initializer regression in resource_sizes (MonochromePublic.apk) at 494950:494950

Project Member Reported by estevenson@chromium.org, Aug 17 2017 
Caused by "device/vr refactor: Remove GvrDelegate interface, clean up GvrDeviceProvider and VrShellDelegate."

Commit 5def8d45e821877c0bf84fd7dbef9a154f96f72c

Link to size graph: https://chromeperf.appspot.com/report?sid=2e1421013ce75306cdc0974cbd058b84083ef684dabed256879bacb31613b326&num_points=10&rev=494950

Policy is to revert the CL that added static initializers and reland it with the fix.

You can verify locally that no static initializers are added by running the following from within an Android checkout: tools/binary_size/diagnose_bloat.py HEAD -v

Common fixes include:
  * Add constexpr,
  * Use LazyInstance<>,
  * Use a getter to return a local static variable.

Thanks!
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, Aug 17 2017 

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

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


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

Android Builder

Comment 2 by estevenson@chromium.org, Aug 17 2017

Labels: -Pri-2 ReleaseBlock-Stable OS-Android Pri-1
Since the CL can't be reverted cleanly we can probably just submit a fix for this instead.
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 17 2017 

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

commit bc15910d7bca4939f15660af1e9384859728d83b
Author: Eric Stevenson <estevenson@chromium.org>
Date: Thu Aug 17 04:19:29 2017

Revert "device/vr refactor: Remove GvrDelegate interface, clean up GvrDeviceProvider and VrShellDelegate."

This reverts commit 5def8d45e821877c0bf84fd7dbef9a154f96f72c.

Reason for revert: Adds static initializers on Android.

Original change's description:
> device/vr refactor: Remove GvrDelegate interface, clean up GvrDeviceProvider and VrShellDelegate.
> 
> This CL removes the inheritance of GvrDelegate from vr_shell.cc, and
> refactors VrShellDelegate to get the Device directly from
> VrDeviceProvider and simplify lifecycles.
> 
> Bug:  752193 
> Change-Id: If662537bb5e5fc15236db772a94db41803e32255
> Reviewed-on: https://chromium-review.googlesource.com/617765
> Reviewed-by: Brandon Jones <bajones@chromium.org>
> Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#494950}

TBR=mthiesse@chromium.org,bajones@chromium.org

Change-Id: If94172a4673b668a20eaef398391fa60bd047d6d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  752193 ,  756302 
Reviewed-on: https://chromium-review.googlesource.com/618514
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Commit-Queue: Eric Stevenson <estevenson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495068}
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/chrome/browser/android/vr_shell/vr_shell.cc
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/chrome/browser/android/vr_shell/vr_shell.h
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/chrome/browser/android/vr_shell/vr_shell_delegate.cc
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/chrome/browser/android/vr_shell/vr_shell_delegate.h
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/device/vr/android/gvr/gvr_delegate.cc
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/device/vr/android/gvr/gvr_delegate.h
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/device/vr/android/gvr/gvr_delegate_provider.h
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/device/vr/android/gvr/gvr_device.cc
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/device/vr/android/gvr/gvr_device.h
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/device/vr/android/gvr/gvr_device_provider.cc
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/device/vr/android/gvr/gvr_device_provider.h
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/device/vr/vr_device_manager.cc
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/device/vr/vr_device_manager.h

Comment 4 by estevenson@chromium.org, Aug 17 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment