Bug 28873 – WebInspector: Migrate Databases tab to InjectedScript / serialized interaction.

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Pavel Feldman]

This will consist of several steps.

[Pavel Feldman]

Created attachment 38869 [details]
patch

This is a first step in serializing db access. It also contains couple of drive-by fixes.

[Timothy Hatcher]

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.

[Pavel Feldman]

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.

[Pavel Feldman]

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

[Yury Semikhatsky]

DOM storage access is not serialized yet.

[Yury Semikhatsky]

Created attachment 39692 [details]
patch: serialize dom storage access

[Yury Semikhatsky]

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.

[Eric Seidel (no email)]

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.

[Yury Semikhatsky]

(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

[Yury Semikhatsky]

Comment on attachment 39754 [details]
patch: serialize dom storage access

Send this patch to its own bug: 29536