Bug 31830 - Chromium: Expose method for reporting user stats to the host.
Summary: Chromium: Expose method for reporting user stats to the host.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Feldman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-24 06:43 PST by Pavel Feldman
Modified: 2009-11-24 11:39 PST (History)
2 users (show)

See Also:


Attachments
[PATCH] (1.07 KB, patch)
2009-11-24 06:44 PST, Pavel Feldman
dglazkov: review+
abarth: commit-queue-
Details | Formatted Diff | Diff
[PATCH] Darin suggests that it is exposed to whole WebKit. (1.40 KB, patch)
2009-11-24 10:38 PST, Pavel Feldman
fishd: review+
pfeldman: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2009-11-24 06:43:02 PST
We'd like to push some stats from web inspector to the host.
Comment 1 Pavel Feldman 2009-11-24 06:44:54 PST
Created attachment 43766 [details]
[PATCH] 

Note that I should land it after implementation lands in Chromium.
Comment 2 Adam Barth 2009-11-24 07:56:29 PST
Comment on attachment 43766 [details]
[PATCH] 

Marking cq- so we don't accidentally commit this before the downstream change.
Comment 3 Dimitri Glazkov (Google) 2009-11-24 10:12:43 PST
Comment on attachment 43766 [details]
[PATCH] 

ok.
Comment 4 Eric Seidel (no email) 2009-11-24 10:17:11 PST
Comment on attachment 43766 [details]
[PATCH] 

Where is this ever used?  Why would we add this virtual function w/o it ever being used anywhere...  This change is not required before the downstream one since you can always add a virtual method which is never called.  In fact, this change would break the downstream version if checked in, so I'm not sure I understand why this is being added at all w/o ever being used.

I guess I don't understand the patch or the r+.
Comment 5 Pavel Feldman 2009-11-24 10:19:14 PST
> I guess I don't understand the patch or the r+.

http://codereview.chromium.org/435019/show
Comment 6 Pavel Feldman 2009-11-24 10:38:00 PST
Created attachment 43787 [details]
[PATCH] Darin suggests that it is exposed to whole WebKit.
Comment 7 Darin Fisher (:fishd, Google) 2009-11-24 10:41:15 PST
Comment on attachment 43787 [details]
[PATCH] Darin suggests that it is exposed to whole WebKit.

> +    // By default, histogram is exponential, so that min=1, max=1000000, bucketCound=50 would do. Setting

nit: s/bucketCound/bucketCount/
Comment 8 Darin Fisher (:fishd, Google) 2009-11-24 10:45:40 PST
if you add a default implementation, then you don't have to worry about a painful two sided landing

virtual void histogramCounts(...) { }
Comment 9 Pavel Feldman 2009-11-24 11:39:32 PST
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebKit/chromium/ChangeLog
	M	WebKit/chromium/public/WebKitClient.h
Committed r51345

Landed with drive-by default implementations for the whole client itself.