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

Issue 734194 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Last visit 26 days ago
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 0
Type: Bug



Sign in to add a comment

[Play Review Stable] Can not download images

Project Member Reported by hongchic...@chromium.org, Jun 16 2017

Issue description

Chrome Version: 59.0.3071.92
OS: Android

What steps will reproduce the problem?
Users can not download images by long pressing on the images

What is the expected result?

What happens instead?

Please use labels and text to provide additional information.


For graphics-related bugs, please copy/paste the contents of the about:gpu
page at the end of this report.

 
Components: UI>Browser>Downloads
Labels: Merge-Approved-59
Merge of diagnostic patch to M59 branch 3071 approved.
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 21 2017

Labels: -merge-approved-59 merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/51999b65b6986f271bb75dee64d8e1f2602b4308

commit 51999b65b6986f271bb75dee64d8e1f2602b4308
Author: Min Qin <qinmin@chromium.org>
Date: Wed Jun 21 01:35:10 2017

CherryPick:Add new UMA to record image download issues

User are reporting that they cannot download images.
This CL adds some UMA to record some metrics about the failure
The UMA includes duplicate infobar, storage permission and context menu option.

(cherry picked from commit 18cd74a816078edc85a9bffce90831f3c133a114)

Bug= 734194 
TBR=dtrainor@chromium.org,isherman@chromium.org

Change-Id: I4c202a1db8011dc5a7b2493b70e804d1d424522b
Reviewed-on: https://chromium-review.googlesource.com/542040
Reviewed-by: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/branch-heads/3071@{#809}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}
[modify] https://crrev.com/51999b65b6986f271bb75dee64d8e1f2602b4308/chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java
[modify] https://crrev.com/51999b65b6986f271bb75dee64d8e1f2602b4308/chrome/browser/android/download/chrome_duplicate_download_infobar_delegate.cc
[modify] https://crrev.com/51999b65b6986f271bb75dee64d8e1f2602b4308/chrome/browser/android/download/chrome_duplicate_download_infobar_delegate.h
[modify] https://crrev.com/51999b65b6986f271bb75dee64d8e1f2602b4308/chrome/browser/android/download/download_controller.cc
[modify] https://crrev.com/51999b65b6986f271bb75dee64d8e1f2602b4308/chrome/browser/android/download/download_controller.h
[modify] https://crrev.com/51999b65b6986f271bb75dee64d8e1f2602b4308/tools/metrics/histograms/histograms.xml

Project Member

Comment 4 by bugdroid1@chromium.org, Jun 21 2017

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

commit 2cc12536a8b000ea3e2e4e289b2a9b1989757336
Author: Min Qin <qinmin@chromium.org>
Date: Wed Jun 21 05:00:36 2017

Add new UMA to record image download issues

User are reporting that they cannot download images.
This CL adds some UMA to record some metrics about the failure
The UMA includes duplicate infobar, storage permission and context menu option.

Bug= 734194 

Change-Id: Idb43dc03cd14c99488d253a8a76bd7685ce53cb6
Reviewed-on: https://chromium-review.googlesource.com/541684
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481113}
[modify] https://crrev.com/2cc12536a8b000ea3e2e4e289b2a9b1989757336/chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java
[modify] https://crrev.com/2cc12536a8b000ea3e2e4e289b2a9b1989757336/chrome/browser/android/download/chrome_duplicate_download_infobar_delegate.cc
[modify] https://crrev.com/2cc12536a8b000ea3e2e4e289b2a9b1989757336/chrome/browser/android/download/chrome_duplicate_download_infobar_delegate.h
[modify] https://crrev.com/2cc12536a8b000ea3e2e4e289b2a9b1989757336/chrome/browser/android/download/download_controller.cc
[modify] https://crrev.com/2cc12536a8b000ea3e2e4e289b2a9b1989757336/chrome/browser/android/download/download_controller.h
[modify] https://crrev.com/2cc12536a8b000ea3e2e4e289b2a9b1989757336/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/2cc12536a8b000ea3e2e4e289b2a9b1989757336/tools/metrics/histograms/histograms.xml

Project Member

Comment 5 by bugdroid1@chromium.org, Jun 21 2017

Labels: merge-merged-3137
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/18d20f2fcb2aef8483a05ab887effaa8002240a6

commit 18d20f2fcb2aef8483a05ab887effaa8002240a6
Author: Alex Mineer <amineer@chromium.org>
Date: Wed Jun 21 15:14:03 2017

Add new UMA to record image download issues

User are reporting that they cannot download images.
This CL adds some UMA to record some metrics about the failure
The UMA includes duplicate infobar, storage permission and context menu option.

Bug= 734194 

(cherry picked from commit 2cc12536a8b000ea3e2e4e289b2a9b1989757336)

Change-Id: Idb43dc03cd14c99488d253a8a76bd7685ce53cb6
Reviewed-on: https://chromium-review.googlesource.com/541684
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#481113}
Cr-Commit-Position: refs/branch-heads/3137@{#3}
Cr-Branched-From: 59326d74274f028a233a2561f940a84675e0c967-refs/heads/master@{#481056}

[modify] https://crrev.com/18d20f2fcb2aef8483a05ab887effaa8002240a6/chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java
[modify] https://crrev.com/18d20f2fcb2aef8483a05ab887effaa8002240a6/chrome/browser/android/download/chrome_duplicate_download_infobar_delegate.cc
[modify] https://crrev.com/18d20f2fcb2aef8483a05ab887effaa8002240a6/chrome/browser/android/download/chrome_duplicate_download_infobar_delegate.h
[modify] https://crrev.com/18d20f2fcb2aef8483a05ab887effaa8002240a6/chrome/browser/android/download/download_controller.cc
[modify] https://crrev.com/18d20f2fcb2aef8483a05ab887effaa8002240a6/chrome/browser/android/download/download_controller.h
[modify] https://crrev.com/18d20f2fcb2aef8483a05ab887effaa8002240a6/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/18d20f2fcb2aef8483a05ab887effaa8002240a6/tools/metrics/histograms/histograms.xml

I tried below steps and found "download image" issue in 59.0.3071.112  on Lenovo K3 Note (k50a40) / MRA58K 

Steps to reproduce: 

1) Install and launch chrome
2) Open new tab and search for any thing  (eg., trump)
3) In search results page long press on any image ( in "more about Donald Trump" section)
4) Select "Download image" from context menu

Observed behavior: Image is not getting downloaded.

Expected behavior: Image should get downloaded.

Frequency: 5/5

Note: 
1) Please find logs and Video taken on Lenovo k50a40/MRA58K @ http://go/chrome-androidlogs1/6/downloadimage.
2) Please find device info screenshots(Chrome://version) @ http://go/chrome-androidlogs1/6/chromeversion.
3) Issue is also observed on Moto G4 Plus(athene_f)/7.0.0, Blu Life play / KOT49H

Note: Please find updated steps below

Steps to reproduce:
1) Have older version of chrome and update to 59.0.3071.112. ( in this case 58.0.3029.83)
2) Now "clear data" from device settings
3) Launch chrome (Not signed in)
4) Open new tab and search for any thing  (eg., trump)
5) In search results page long press on any image ( in "more about Donald Trump" section)
6) Select "Download image" from context menu.
7) Notice Image is not getting downloaded.




Cc: qin...@chromium.org brettw@chromium.org
Labels: -Pri-3 Pri-1
qinmin@ will post an update shortly re: a potential link to history corruption. 
 I had been chatting with skym@ about this, here's what they had to say:

""you folks probably alraedy got here, but if you didn't
i installed 59.0.3071.92
appended some text to the History database (with same filename)
it seems to recover on restart
but if i completely remove it and put in junk text
downloads stop working
i click 'Download Image', then it popup goes away
and nothing
who knows if this is what users are running into
though, i restarted my app again, and it's fixed now
so it recovers, i'd only get stuck broken if my history db kept getting corrupted""
For c#6 / c#7 above - we determined that the cause of this reproducible case was due to google.com appending a location infobar to the screen that is pinned to the top of the page.  When trying to download an item, a "duplicate image" infobar is stuck behind the location infobar, and also hidden towards the top of the page.  This behavior, while bad and should be corrected, is present in M58 as well as M59, and we don't think it is the cause of this general issue that users are reporting.
Cc: dtrainor@chromium.org shaktisahu@chromium.org xingliu@chromium.org
We have also noticed that a number of the reports talking about busted images also talk about history not working:

"Since I was updated google chrome I can not download images of google and in the history does not save anything and stays blank"

"Unfortunately update, now can not download images directly to the computer or memory, nor does it allow access to history, increasingly worse with its programmers, for a leading global company are leaving to duty"

"It does not let me download more images, the history does not leave anything therefore I can not clean it"
We found that the issue is caused to history database corruption, and History.AttemptedToFixProfileError is almost 10X higher on M59 (5% rollout), compared to M58.

When a download starts, download core queries history backend for the next downloadId: https://cs.chromium.org/chromium/src/components/history/core/browser/history_backend.cc?type=cs&q=attemptedtofixprofileerror&l=1148
However, if the database is nuked, then it will return an invalid Id. This causes download system to think that history DB is not initialized and start to wait indefinitely. As a result, no download will be possible unless user restarts Chrome with a new empty profile.

Here is a list of the M59 feedback reports that have image download issues between 6/13-6/19:

https://pulse.corp.google.com/#/feedbackneutron?filterList=%7B%22product_name%22:%5B%22Chrome%20-%20Mobile%22%5D%7D,%7B%22product_version%22:%5B%2259.0.3071.92%22%5D%7D,%7B%22FLATTENED_ISSUEDEPTH1%22:%5B%22%23issue%3EDownloads%22%5D%7D,%7B%22FLATTENED_LABELTAG%22:%5B%22%23label%3EClankM59%22,%22%23label%3EClank60%22%5D%7D,%7B%22FLATTENED_UNIGRAM%22:%5B%22image%22,%22pic%22,%22picture%22%5D%7D&dimensions=product_name,category_name,FLATTENED_ISSUEDEPTH1,FLATTENED_ISSUEDEPTH2,FLATTENED_ISSUEDEPTH3&rtm=true&selected=All,,&level=0&unique=0&printView=false&days=CUSTOM&weeklyAvg=false&start=%22Jun%2013,%202017%22&end=%22Jun%2019,%202017%22&fbtab=reports

For these reports, 4 of them contains metric data. And we digged into all those metrics, and found that all of the 4 metrics contains information about profile error (contains "History.AttemptedToFixProfileError").

https://feedback.corp.google.com/product/282/neutron?lView=rd&lReport=65989871294
https://feedback.corp.google.com/product/282/neutron?lView=rd&lReport=65975512219
https://feedback.corp.google.com/product/282/neutron?lView=rd&lReport=65894993587
https://feedback.corp.google.com/product/282/neutron?lView=rd&lReport=65750203418

Thanks

Cc: dah...@chromium.org
Cc: dim...@chromium.org
+dimich@ could profile corruption and restart also be causing offline page vanishes?
Cc: costan@google.com
+costan from a history db perspective.
Here's a graph of History.AttemptedToFixProfileError splitting by milestone: https://uma.googleplex.com/p/chrome/timeline_v2/?sid=03d3ebc355ed37a46e4fd718ced3614a

Note we're only shipping at 5%, so if the trend continues that's where we get the 10x compared to M58 (and below) from.
If you're in a state that causes this problem, does killing and restarting Chrome fix the problem? Theoretically Chrome should recover from a corrupt DB by deleting it.
If you can repro, the most interesting thing is what in HistoryDatabase::Init failed first?
Cc: amineer@chromium.org
Cc: -brettw@chromium.org
Labels: -Pri-1 Pri-0
Owner: brettw@chromium.org
Status: Assigned (was: Untriaged)
Setting as Pri-0 as this has caused a ~1.5 week delay and is still actively gating Android shipping M59 to 100%.

Assigning to brettw@ as corrupted DB is top candidate based on all analysis performed to date - perhaps not perfect but we need to start somewhere.

brettw@, I know this issue sucks, but where do you think we can go from here?  We need to find a way to move forward.  Unfortunately we haven't been able to reproduce the issue, so we can't answer your questions above as best I can tell (though dtrainor@ correct me if I'm wrong).

Do we need to review CLs to see what might be going wrong here?  Do we need to add debugging and push a new version to stable users?  What else can we do?
Cc: -costan@google.com pwnall@chromium.org
brettw: I can look into the history part starting tomorrow.
Further evidence something has shifted with M59 - on prior releases, KitKat absolutely dominated counts of this histogram, e.g. ~20k events / day on that platform, with only ~500-600 events / day on other platforms.  For M59, the distribution has shifted, and we're seeing more errors on Lollipop than on any other platform, with ~5-6k events / day - this is a 10x increase over M58 and we're only shipped at 5%...

FWIW I don't believe this means the problem is only due to a change in Lollipop - M59 Android Marshmallow is already ~1k events / day at 5%, which would also be a massive spike relative to prior levels.

Graphs per milestone below.

M56: https://uma.googleplex.com/p/chrome/timeline_v2/?sid=c3161fee0e2fdd9d764afb1f97cbaa9f
M57: https://uma.googleplex.com/p/chrome/timeline_v2/?sid=0894b3d2b2c3c1ee1c52d0e2d8645822
M58: https://uma.googleplex.com/p/chrome/timeline_v2/?sid=33d531e026a6a5d1646d7d4a4ca6837f
M59: https://uma.googleplex.com/p/chrome/timeline_v2/?sid=5a450a1991b563fe47ffb64ab8a93784
I'm interested in answers to the questions in comments 16 and 17.
I think there is no reproducible cases yet, so #16 and #17 can not be clearly answered at the moment.

With Offline Pages, we have issue that can be similar, when sometimes (very few cases) we are unable to open SQLite DB. We did kill it before and it restores functionality but also looses the user-downloaded content which is undesirable. We temporarily stopped killing the DB hoping it's transient issue but are collecting more info. See bug 725122.
Since we had seen a number of Japanese and Spanish reviews related to this, I broke things down by locale; we see 2x the amount of usage in the US relative to Japan [1], but 2x the amount of profile corruptions in Japan relative to the US [2].  Spanish was inconclusive, so not sure if this is relevant or just coincidental.

[1] https://uma.googleplex.com/p/chrome/timeline_v2/?sid=afb51764785591d3ea62fa41fbacaac7
[2] https://uma.googleplex.com/p/chrome/timeline_v2/?sid=3ef9a5bfb0d62cf29e4e4528e93e3102
In 59 stable, nearly 100% of all reports of History.AttemptedToFixProfileError is "not attempted" which means the error was not fatal.

This means that history is not getting corrupted, but something else is going on. This is actually much more plausible since many common errors can cause failures but not corruption.

Possibilities include:
 - DB is locked or can't be opened (some other Chrome instance has it open).
 - Certain system I/O errors (probably unlikely)
 - Disk full (maybe migration makes the size explode?)
 - SQL parsing errors.
 - Schema problems.
 - General API misuse.
Can migration passes also cause those errors without triggering a fatal error?
Yes, any error thrown by sqlite during migration will end up in this code path.
If the error isn't fatal, do we still think these errors could be the root cause for our inability to download images or inability to view history?  Or would that only occur if the DB was entirely corrupted?

Just trying to ID if we need to continue evaluating this as the true root cause or if we need to start looking for potential culprits elsewhere.
We would still fail the load.  We wouldn't attempt to nuke the database though.  Looking at History.AttemptedToFixProfileError shows about 8000 people hitting the error (64000 instances).  So I'm guessing 8000 people are repeatedly hitting it?

Comment 30 by s...@chromium.org, Jun 22 2017

Cc: s...@chromium.org
Reading #25, and in particular the line

>  - Disk full (maybe migration makes the size explode?)

Reminded me of one of the History migrations present in M59 creates a temporary copy of the URLs table to add AUTOINCREMENT, see https://codereview.chromium.org/2721713002 and https://chromiumdash.appspot.com/commit/e8cbd06453b8b4dfc01fa61d004fa0469f42bd98

I'm not sure what percentage of the History database the URLs table makes up, total DB size seems to be a few MB https://uma.googleplex.com/p/chrome/histograms?endDate=20170621&dayCount=1&histograms=Sqlite.SizeKB.History&fixupData=true&showMax=true&filters=platform%2Ceq%2CA%2Cchannel%2Ceq%2C4%2Cisofficial%2Ceq%2CTrue&implicitFilters=isofficial

You could check the metrics data referenced in comment #11 and see what their Sqlite.SizeKB.History is.
My best theory is that these people's phones are out of storage.

This explains why it's Android-only, as desktops don't run out of disk space (if they do not being able to download images will be the least of your problems). Many people are likely at the storage limit on their mobile devices.

This change: https://codereview.chromium.org/2721713002 duplicates the URL table as part of migration. It is new in 59. It will vary depending on the size of the URL table, but could cause an increase of a couple of MB in disk usage during migration. This is substantially more space than normal history operations. If there isn't enough space, migration will fail and we will see the currently manifested problems (nonfatal init error, things not working, database not nuked).

About 70% of all sqlite errors are SQLITE_FULL. This doesn't change from 58 to 59. But the migration code will tend to trigger this more than previous initialization code (probably without this step the disk requirements for initialization are minimal and often 0).

Any error, including FULL, during migration will result in a broken database. During normal operation, we just continue when we hit errors and whatever was supposed to happen won't happen. We see a decrease in logs of SQLITE_FULL in history errors (from 65% of errors in M58 to 50% in M59) which is consistent with this analysis. If the user is almost full and passes initialization, errors can trigger repeatedly (for every failing operation), but if initialization doesn't pass, we'll only see the one error per browser execution.
Ignore the paragraph of "About 70% of all sqlite errors are SQLITE_FULL and doesn't change" in previous comment. That was looking at the wrong stat. The last paragraph is the correct one.
I checked with Gang Wu and the code that requires the AUTOINCREMENT update is not enabled yet. Therefore, we can ignore errors during this migration and continue.

I will write a patch to do this, which should be very low-risk to merge. I just need to verify first that subsequent operations succeed once we get into a FULL state.

Assuming we do this change, we will have to think about how to fix this state for when we actually need AUTOINCREMENT.
Project Member

Comment 34 by bugdroid1@chromium.org, Jun 23 2017

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

commit ac7b23b2ea06f2826e7b2bca77475b2c11ce0973
Author: Brett Wilson <brettw@chromium.org>
Date: Fri Jun 23 18:13:21 2017

History init: add metrics and skip migration.

Removes the migration code to history DB version 34. This version did a
large migration of the URL table which can use several MB of disk storage. The
suspicion is that this is causing more initialization failures for users with
full devices.

The change in version 34 is to add an AUTOINCREMENT tag on the primary key.
This is required for some sync work that is in progress but is not complete,
so skipping this migration step won't break any users. We will need to fix
this before rolling out the new sync features.

The 34 change also removed the favicon_id column. Because some databases
may have the column and some won't, the code to copy to the in-memory
database (which previously was implicit) now lists the columns to copy
explicitly.

Adds UMA logging for history database initialization failures, as well as
versions for migration failures. Logging for the Sqlite error code for the
various failure states may have been nice but would have required more
plumbing and seemed impractical for now.

There is one behavior change the error logging resulted in: errors migrating
to version 22 were previously ignored but are now fatal. I believe continuing
in this case was a mistake. This will only affect users migrating from an
Android Chrome version prior to 2012, and failures here will likely be fatal
anyway.

BUG= 734194 ,  736136 
R=dtrainor@chromium.org, gangwu@chromium.org, mpearson@chromium.org, yzshen@chromium.org

Review-Url: https://codereview.chromium.org/2954593002 .
Cr-Commit-Position: refs/heads/master@{#481958}

[modify] https://crrev.com/ac7b23b2ea06f2826e7b2bca77475b2c11ce0973/components/history/core/browser/history_backend.cc
[modify] https://crrev.com/ac7b23b2ea06f2826e7b2bca77475b2c11ce0973/components/history/core/browser/history_database.cc
[modify] https://crrev.com/ac7b23b2ea06f2826e7b2bca77475b2c11ce0973/components/history/core/browser/in_memory_database.cc
[modify] https://crrev.com/ac7b23b2ea06f2826e7b2bca77475b2c11ce0973/components/history/core/browser/url_database.cc
[modify] https://crrev.com/ac7b23b2ea06f2826e7b2bca77475b2c11ce0973/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/ac7b23b2ea06f2826e7b2bca77475b2c11ce0973/tools/metrics/histograms/histograms.xml

Project Member

Comment 35 by bugdroid1@chromium.org, Jun 23 2017

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

commit 56680659ec10ca2d931cf1787b836ce481e46ded
Author: Brett Wilson <brettw@chromium.org>
Date: Fri Jun 23 18:30:58 2017

History init: add metrics and skip migration.

Removes the migration code to history DB version 34. This version did a
large migration of the URL table which can use several MB of disk storage. The
suspicion is that this is causing more initialization failures for users with
full devices.

The change in version 34 is to add an AUTOINCREMENT tag on the primary key.
This is required for some sync work that is in progress but is not complete,
so skipping this migration step won't break any users. We will need to fix
this before rolling out the new sync features.

The 34 change also removed the favicon_id column. Because some databases
may have the column and some won't, the code to copy to the in-memory
database (which previously was implicit) now lists the columns to copy
explicitly.

Adds UMA logging for history database initialization failures, as well as
versions for migration failures. Logging for the Sqlite error code for the
various failure states may have been nice but would have required more
plumbing and seemed impractical for now.

There is one behavior change the error logging resulted in: errors migrating
to version 22 were previously ignored but are now fatal. I believe continuing
in this case was a mistake. This will only affect users migrating from an
Android Chrome version prior to 2012, and failures here will likely be fatal
anyway.

BUG= 734194 ,  736136 
R=dtrainor@chromium.org, gangwu@chromium.org, mpearson@chromium.org, yzshen@chromium.org

Review-Url: https://codereview.chromium.org/2954593002 .
Cr-Original-Commit-Position: refs/heads/master@{#481958}
Review-Url: https://codereview.chromium.org/2952333003 .
Cr-Commit-Position: refs/branch-heads/3071@{#822}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/56680659ec10ca2d931cf1787b836ce481e46ded/components/history/core/browser/history_backend.cc
[modify] https://crrev.com/56680659ec10ca2d931cf1787b836ce481e46ded/components/history/core/browser/history_database.cc
[modify] https://crrev.com/56680659ec10ca2d931cf1787b836ce481e46ded/components/history/core/browser/in_memory_database.cc
[modify] https://crrev.com/56680659ec10ca2d931cf1787b836ce481e46ded/components/history/core/browser/url_database.cc
[modify] https://crrev.com/56680659ec10ca2d931cf1787b836ce481e46ded/tools/metrics/histograms/histograms.xml

brettw: Should I still look into this issue, or has the investigation completed Thanks!
Since you're the new owner here for sqllite, it'd be good if you can review the thread, confirm findings, review patch, think of other test cases, etc.  If this doesn't work we're in bad shape, and understanding this issue will probably be good - just my opinion, though.
#37: This seems reasonable to me! Will do.
Project Member

Comment 39 by bugdroid1@chromium.org, Jun 23 2017

Labels: merge-merged-3139
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/567c0200d67292ebdc86d894ca5348acc3e2dea2

commit 567c0200d67292ebdc86d894ca5348acc3e2dea2
Author: Alex Mineer <amineer@chromium.org>
Date: Fri Jun 23 20:44:08 2017

History init: add metrics and skip migration.

Removes the migration code to history DB version 34. This version did a
large migration of the URL table which can use several MB of disk storage. The
suspicion is that this is causing more initialization failures for users with
full devices.

The change in version 34 is to add an AUTOINCREMENT tag on the primary key.
This is required for some sync work that is in progress but is not complete,
so skipping this migration step won't break any users. We will need to fix
this before rolling out the new sync features.

The 34 change also removed the favicon_id column. Because some databases
may have the column and some won't, the code to copy to the in-memory
database (which previously was implicit) now lists the columns to copy
explicitly.

Adds UMA logging for history database initialization failures, as well as
versions for migration failures. Logging for the Sqlite error code for the
various failure states may have been nice but would have required more
plumbing and seemed impractical for now.

There is one behavior change the error logging resulted in: errors migrating
to version 22 were previously ignored but are now fatal. I believe continuing
in this case was a mistake. This will only affect users migrating from an
Android Chrome version prior to 2012, and failures here will likely be fatal
anyway.

BUG= 734194 ,  736136 
R=dtrainor@chromium.org, gangwu@chromium.org, mpearson@chromium.org, yzshen@chromium.org

(cherry picked from commit ac7b23b2ea06f2826e7b2bca77475b2c11ce0973)

Review-Url: https://codereview.chromium.org/2954593002 .
Cr-Original-Commit-Position: refs/heads/master@{#481958}
Cr-Commit-Position: refs/branch-heads/3139@{#7}
Cr-Branched-From: 2dd0ea84298d45e0962c595c591529bc84dd0ebd-refs/heads/master@{#481757}

[modify] https://crrev.com/567c0200d67292ebdc86d894ca5348acc3e2dea2/components/history/core/browser/history_backend.cc
[modify] https://crrev.com/567c0200d67292ebdc86d894ca5348acc3e2dea2/components/history/core/browser/history_database.cc
[modify] https://crrev.com/567c0200d67292ebdc86d894ca5348acc3e2dea2/components/history/core/browser/in_memory_database.cc
[modify] https://crrev.com/567c0200d67292ebdc86d894ca5348acc3e2dea2/components/history/core/browser/url_database.cc
[modify] https://crrev.com/567c0200d67292ebdc86d894ca5348acc3e2dea2/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/567c0200d67292ebdc86d894ca5348acc3e2dea2/tools/metrics/histograms/histograms.xml

Labels: Merge-Approved-60
Whoops, we'll need this for M60 as well - approved for branch 3112 merge.
Project Member

Comment 41 by bugdroid1@chromium.org, Jun 26 2017

Labels: -merge-approved-60 merge-merged-3112
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/81fc74ee0a399613c2f4d054c363744ccfe2e291

commit 81fc74ee0a399613c2f4d054c363744ccfe2e291
Author: Brett Wilson <brettw@chromium.org>
Date: Mon Jun 26 17:26:45 2017

History init: add metrics and skip migration.

Removes the migration code to history DB version 34. This version did a
large migration of the URL table which can use several MB of disk storage. The
suspicion is that this is causing more initialization failures for users with
full devices.

The change in version 34 is to add an AUTOINCREMENT tag on the primary key.
This is required for some sync work that is in progress but is not complete,
so skipping this migration step won't break any users. We will need to fix
this before rolling out the new sync features.

The 34 change also removed the favicon_id column. Because some databases
may have the column and some won't, the code to copy to the in-memory
database (which previously was implicit) now lists the columns to copy
explicitly.

Adds UMA logging for history database initialization failures, as well as
versions for migration failures. Logging for the Sqlite error code for the
various failure states may have been nice but would have required more
plumbing and seemed impractical for now.

There is one behavior change the error logging resulted in: errors migrating
to version 22 were previously ignored but are now fatal. I believe continuing
in this case was a mistake. This will only affect users migrating from an
Android Chrome version prior to 2012, and failures here will likely be fatal
anyway.

BUG= 734194 ,  736136 
R=dtrainor@chromium.org, gangwu@chromium.org, mpearson@chromium.org, yzshen@chromium.org

Review-Url: https://codereview.chromium.org/2954593002 .
Cr-Original-Commit-Position: refs/heads/master@{#481958}
Review-Url: https://codereview.chromium.org/2956913002 .
Cr-Commit-Position: refs/branch-heads/3112@{#465}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}

[modify] https://crrev.com/81fc74ee0a399613c2f4d054c363744ccfe2e291/components/history/core/browser/history_backend.cc
[modify] https://crrev.com/81fc74ee0a399613c2f4d054c363744ccfe2e291/components/history/core/browser/history_database.cc
[modify] https://crrev.com/81fc74ee0a399613c2f4d054c363744ccfe2e291/components/history/core/browser/in_memory_database.cc
[modify] https://crrev.com/81fc74ee0a399613c2f4d054c363744ccfe2e291/components/history/core/browser/url_database.cc
[modify] https://crrev.com/81fc74ee0a399613c2f4d054c363744ccfe2e291/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/81fc74ee0a399613c2f4d054c363744ccfe2e291/tools/metrics/histograms/histograms.xml

Status: Fixed (was: Assigned)
/me crosses fingers and marks bug as fixed.

Sign in to add a comment