Bug 29924

Summary: Database Inspector crashes Safari when table has more than 21 columns
Product: WebKit Reporter: Tim Lind <timlind>
Component: Web Inspector (Deprecated)Assignee: Joseph Pecoraro <joepeck>
Severity: Normal CC: joepeck, pmuellr, timothy
Priority: P2 Keywords: NeedsReduction
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Description Flags
test case
[PATCH] Fix Hang
timothy: review-
[PATCH] Fix Hang timothy: review+

Description Tim Lind 2009-09-30 07:29:16 PDT
Create a table with 21 or more columns. Select it under the database in the Web Inspector, and Safari hangs (infinitely).
Comment 1 Patrick Mueller 2009-09-30 11:32:29 PDT
Here's an example that creates exactly 21 columns which does not appear to hang Safari when viewing the db in Web Inspector in nightly r48908.  Reloading the page over and over will result in a new row being added each time.

I wonder if it's some data that's causing a problem?  Why do you think it's the 21st column that causes a problem?

---------- clip here ----------
var db = openDatabase("bug-29924", "1.0", "sample for webkit bug 29924", 200000);

var cols21def   = "";
var cols21spec  = "";
var vals21spec  = "";
var vals21      = []
for (var i = 1; i <= 21; ++i) {
    cols21def  = cols21def  + "col" + i + " TEXT";
    cols21spec = cols21spec + "col" + i;
    vals21spec = vals21spec + "?";
    vals21.push(new Date().toString());
    if (i != 21) {
        cols21def  += ", ";
        cols21spec += ", ";
        vals21spec += ", ";

db.transaction(function(tx) {
    var statement = "CREATE TABLE IF NOT EXISTS TwentyOneColumns (" + cols21def + ")";
        statement, [],  
        function(tx, result) {
            var statement = "INSERT INTO TwentyOneColumns (" + cols21spec + ") VALUES (" + vals21spec + ")";
                statement, vals21,  
                function(tx, result) {},
                function(tx, error) {
                    alert("Error during INSERT: " + JSON.stringify(error));
        function(tx, error) {
            alert("Error during CREATE TABLE: " + JSON.stringify(error));

---------- clip here ----------
Comment 2 Tim Lind 2009-09-30 12:35:21 PDT
Created attachment 40391 [details]
test case

21 columns, with one unique constraint on two of the columns.
Comment 3 Tim Lind 2009-09-30 12:37:37 PDT
Your code didn't cause any problems for me either. I tried adding a one key unique constraint to yours and it was still fine.

The file I attached now shows the problem. It has one unique constraint on two columns. Inserts undefined values for some columns. The problem only occurs once data is inserted that can be viewed.

Taking one column away should fix the problem.

Comment 4 Patrick Mueller 2009-09-30 14:00:11 PDT
Great.  That test does result in nightly hanging, sitting at nearly 100% cpu.  Should be fun to debug :-(
Comment 5 Joseph Pecoraro 2009-09-30 20:57:16 PDT
(In reply to comment #4)
> Great.  That test does result in nightly hanging, sitting at nearly 100% cpu. 
> Should be fun to debug :-(

I've had some experience working with the DataGrids so the number 21 jumped out at me immediately. The "minimum" size of each column was set at 5%, and in Storage.js dataGridForResult.  There is a section where it will continue to "adjust" the columns not willing to go less then their minimum in order to fit into 100%.  Great spot for an infinite loop.

Patch to come soon.
Comment 6 Joseph Pecoraro 2009-09-30 21:20:21 PDT
Created attachment 40420 [details]
[PATCH] Fix Hang

Since I moved the DataGrid creation code into its view (like DOM Storage and Cookies did and are unaffected due to fixed/known number of columns) its hard to see the explicit changes, so I'll mention them here.

- numColumns variable to count the number of columns
- const minimumPrecent = Math.min(5, Math.floor(100/numColumns));

The Math.floor is not really necessary but working with integer values seemed better then having horrible percentages like "4.761904761904762".  I don't think we should worry about the unrealistic case of >100 columns, causing the minimumPercent to floor to 0.  The grid could not be useful, but at least it wouldn't crash.
Comment 7 Timothy Hatcher 2009-10-02 12:43:10 PDT
Comment on attachment 40420 [details]
[PATCH] Fix Hang

DatabaseQueryView.js uses dataGridForResult. So we shouldn't move it yet.
Comment 8 Joseph Pecoraro 2009-10-02 12:52:40 PDT
Created attachment 40542 [details]
[PATCH] Fix Hang

Good Catch. Fixed.
Comment 9 Joseph Pecoraro 2009-10-02 13:11:08 PDT
Committed in http://trac.webkit.org/changeset/49039
	M	WebCore/ChangeLog
	M	WebCore/inspector/front-end/StoragePanel.js
r49039 = 9b3be88c052ba7e1b9c6a78f125d1a6222530cf7 (trunk)