[Webkit-unassigned] [Bug 110779] WebKit API for enabling DOM logging for certain worlds

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=110779





--- Comment #23 from Ankur Taly <ataly at google.com>  2013-02-27 16:48:00 PST ---
(In reply to comment #22)
> (In reply to comment #21)
> > (In reply to comment #19)
> > > (From update of attachment 190556 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=190556&action=review
> > > 
> > > > Source/WebKit/chromium/public/WebCoverWrapping.h:54
> > > > +    // Sets the provided log object for the world identified 
> > > > +    // by worldID (worldID may be 0 identifying the main world).
> > > 
> > > I thought we weren't going to support installing a logger for the main world?
> > 
> > Logging can be enabled for any world. The guarantee is that the DOM bindings for all
> > those worlds for which logging is not enabled would not be affected. We include the main world
> > to log DOM activity inside extension background pages.
> 
> I see.  Won't you need to do the hash lookup to see if there is a logger configured for a world?  That seems like it will impact performance even if no logger is configured.

Yes for all worlds there will be a one-time HashMap lookup while initializing the context. (we consider the existence of a logger as an indication from the chromium side that logging should be enabled for that world).

> 
> > The logger will be invoked on all DOM API calls that are covered by the policy. 
> > To begin with we will have a small policy that includes critical calls like document.write, document.createElement, document.appendChild, window.location and so on.
> 
> Do you plan to teach WebKit about this policy, or does WebKit always just call the log function?  Maybe the answer to this question is related to my previous question and the issue is just that I don't understand this part of the design.

As mentioned earlier, think of the policy as a set of pairs of the form <cName,pName>. Each such pair means that all access to property pName of objects with className= cName must be logged.

This set will be hard-coded into the source code, in some struct that lives in WebCore.
If logging is enabled for the world then the methods batchConfigureCallbacks and configureAttribute in V8DOMConfiguration would consult this policy struct for every DOM attribute that they configure. If the attribute is found in  the policy struct then the methods will take a different branch and install wrappers for the attribute, otherwise they will behave the way they are right now. The actual patch makes a few other changes as well, but the gist is what is described above.

Note that in the current design the policy is the same for all worlds and it is hard-coded.

Later, if needed, we can have the policy struct also come from the Chromium side by proving a route 
through the WebKit API. Furthermore, the policy struct could also be different for each world. 

May be the word policy here is confusing?
> 
> > We do not have any performance measurements at the moment but we do plan to have them in the near future. 
> 
> Would you be willing to do some performance measurements before landing this patch?  The performance aspects are an important consideration here.  I would be interested in knowing the overhead for the following cases:
> 
> 1) No logging configured for world.
> 2) Logging configured for world, but the API is not logged according to the policy.
> 3) Logging is configured for world, and the API is actually logged.
> 
> In particular, I'm curious how this affects Dromeo's dom-traverse benchmark in these cases.  You can find dom-traverse in the PerformanceTests directory.  You can find more information about PerformaceTests on this wiki page:
> 
> http://trac.webkit.org/wiki/Performance%20Tests

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.