New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 751735 link

Starred by 1 user
Status: Fixed
Owner:
xingliu@chromium.org
Closed: Aug 2017
Cc:
█ zpeng@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

C++ static initializer regression in resource_sizes (MonochromePublic.apk) at 490540:490540

Project Member Reported by zpeng@chromium.org, Aug 2 2017 
Caused by "Make downloads work when history database fails to initialize"

Commit: d1bc5d0b5f6336fe076466ee42644b2e8fef4bc6

Link to graph: https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgibL3pwkM

Unfortunately, policy for static initializer regression is to revert first and then reland with the fix.

Please run "tools/binary_size/diagnose_bloat.py HEAD -v" on the fix to verify that no static initializers are introduced.
Project Member

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

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

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


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

Android Builder

Comment 2 by zpeng@chromium.org, Aug 2 2017 

Created revert here:
https://chromium-review.googlesource.com/c/598347
Project Member

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

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

commit ceed071596100c2410dfc2584beb4b35b0e12e3f
Author: F . <zpeng@chromium.org>
Date: Wed Aug 02 17:08:13 2017

Revert "Make downloads work when history database fails to initialize."

This reverts commit d1bc5d0b5f6336fe076466ee42644b2e8fef4bc6.

Reason for revert: Introduces new C++ static initializers. See  crbug.com/751735 

Original change's description:
> Make downloads work when history database fails to initialize.
> 
> Previously all download calls will be stuck when history database fails
> to initialize.
> 
> This CL assigns 1 as the first id when history database failed to load,
> all downloads in this browser session will not be persisted to history
> database.
> 
> Bug:  736511 
> Change-Id: If4db04ca518633a7dd999f45629e60e97fc78b1a
> Reviewed-on: https://chromium-review.googlesource.com/587327
> Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
> Reviewed-by: David Trainor <dtrainor@chromium.org>
> Commit-Queue: Xing Liu <xingliu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#490540}

TBR=rkaplow@chromium.org,dtrainor@chromium.org,xingliu@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  736511 ,  751735 
Change-Id: Ifa73c694e919bda82eb37ea88020cf6b0ba89751
Reviewed-on: https://chromium-review.googlesource.com/598347
Reviewed-by: F . <zpeng@chromium.org>
Commit-Queue: F . <zpeng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491410}
[modify] https://crrev.com/ceed071596100c2410dfc2584beb4b35b0e12e3f/chrome/browser/download/chrome_download_manager_delegate.cc
[modify] https://crrev.com/ceed071596100c2410dfc2584beb4b35b0e12e3f/chrome/browser/download/chrome_download_manager_delegate.h
[modify] https://crrev.com/ceed071596100c2410dfc2584beb4b35b0e12e3f/chrome/browser/download/chrome_download_manager_delegate_unittest.cc
[modify] https://crrev.com/ceed071596100c2410dfc2584beb4b35b0e12e3f/chrome/browser/download/download_stats.cc
[modify] https://crrev.com/ceed071596100c2410dfc2584beb4b35b0e12e3f/chrome/browser/download/download_stats.h
[modify] https://crrev.com/ceed071596100c2410dfc2584beb4b35b0e12e3f/tools/metrics/histograms/histograms.xml

Comment 4 by zpeng@chromium.org, Aug 2 2017

Status: Fixed (was: Assigned)

Comment 5 by xingliu@chromium.org, Aug 7 2017 

Does CQ run the diagnosis?

Comment 6 by zpeng@chromium.org, Aug 8 2017 

Unfortunately, the current CQ does not run the diagnosis. However, it is WIP.

Comment 7 by xingliu@chromium.org, Aug 8 2017 

Do I need to revert it on M61 branch? The CL is cherry-picked to M61.

Comment 8 by agrieve@chromium.org, Aug 9 2017 

No need to revert on the branch. One initializer isn't going to hurt the release.
►

Sign in to add a comment