Bug 28873

Summary: WebInspector: Migrate Databases tab to InjectedScript / serialized interaction.
Product: WebKit Reporter: Pavel Feldman <pfeldman>
Component: Web Inspector (Deprecated)Assignee: Yury Semikhatsky <yurys>
Status: RESOLVED FIXED    
Severity: Normal CC: timothy, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
patch
timothy: review-
patch
none
patch: serialize dom storage access
none
patch: serialize dom storage access none

Description Pavel Feldman 2009-09-01 07:08:04 PDT
This will consist of several steps.
Comment 1 Pavel Feldman 2009-09-01 10:32:49 PDT
Created attachment 38869 [details]
patch

This is a first step in serializing db access. It also contains couple of drive-by fixes.
Comment 2 Timothy Hatcher 2009-09-01 10:56:27 PDT
Comment on attachment 38869 [details]
patch

> +    isDB: function(db)

I assume this will do somthing more interesting later? If not, I prefer the callers just doing === or ==.

If this will be needed later, I think isDatabase is better.
> +    runWithTableNames: function(callback)

This name confused me when I saw it. I think getTableNames with a callback is better wording and more consistent with the other callback functions.

> +        function withTablesCallback(tableNames)

Starting a function with "with" is odd naming. Just tableNameCallback would work, or somthing more descriptive on what the function does like accumulateCompletions.

> -    _queryFinished: function(query, tx, result)
> +    _queryFinished: function(query, result)

Is this correct? I thought tx was required here…

> +        function withTableNames(tableNames)

Again about "with".

> +        function withTableNames(tableNames)

Ditto.
Comment 3 Pavel Feldman 2009-09-01 14:55:43 PDT
Created attachment 38887 [details]
patch

(In reply to comment #2)
> (From update of attachment 38869 [details])
> > +    isDB: function(db)
> 
> I assume this will do somthing more interesting later? If not, I prefer the
> callers just doing === or ==.
> 

The idea is to encapsulate Database::database. This quarantined object will be replaced with the proxy in the next step. I actually can already mark this member as private (updated the change).

> If this will be needed later, I think isDatabase is better.

Done. 

> > +    runWithTableNames: function(callback)
> 
> This name confused me when I saw it. I think getTableNames with a callback is
> better wording and more consistent with the other callback functions.
> 

Done.

> > +        function withTablesCallback(tableNames)
> 
> Starting a function with "with" is odd naming. Just tableNameCallback would
> work, or somthing more descriptive on what the function does like
> accumulateCompletions.
> 

Done.

> > -    _queryFinished: function(query, tx, result)
> > +    _queryFinished: function(query, result)
> 
> Is this correct? I thought tx was required here…
>

Yes, Database.prototype.executeSql encapsulates callback wrapping and drops this parameters since it is not used in the actual callbacks.
 
> > +        function withTableNames(tableNames)
> 
> Again about "with".
> 
> > +        function withTableNames(tableNames)
> 
> Ditto.

Done.
Comment 4 Pavel Feldman 2009-09-02 03:00:21 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/inspector/InspectorDOMAgent.cpp
	M	WebCore/inspector/front-end/Database.js
	M	WebCore/inspector/front-end/DatabaseQueryView.js
	M	WebCore/inspector/front-end/DatabaseTableView.js
	M	WebCore/inspector/front-end/StoragePanel.js
Committed r47960
Comment 5 Yury Semikhatsky 2009-09-17 06:41:07 PDT
DOM storage access is not serialized yet.
Comment 6 Yury Semikhatsky 2009-09-17 06:42:16 PDT
Created attachment 39692 [details]
patch: serialize dom storage access
Comment 7 Yury Semikhatsky 2009-09-18 02:41:07 PDT
Created attachment 39754 [details]
patch: serialize dom storage access

Fixed some issues in the previous patch:
* Implemented InspectorDOMStorageResource::operator== which is declared as pure virtual in EventListener. Also added new EventListener type.
* Removed trailing whitespaces in several places.
Comment 8 Eric Seidel (no email) 2009-09-18 12:15:59 PDT
Comment on attachment 38887 [details]
patch

Obsoleting this patch which has been landed.  It's better to use a new bug for a new change.
Comment 9 Yury Semikhatsky 2009-09-21 00:55:15 PDT
(In reply to comment #8)
> (From update of attachment 38887 [details])
> Obsoleting this patch which has been landed.  It's better to use a new bug for
> a new change.
Filed a new bug:
https://bugs.webkit.org/show_bug.cgi?id=29536
Comment 10 Yury Semikhatsky 2009-09-21 00:56:41 PDT
Comment on attachment 39754 [details]
patch: serialize dom storage access

Send this patch to its own bug: 29536