Bug 25342 - Pass MessageLevel and MessageSource into the ChromeClient::addMessageToConsole
Summary: Pass MessageLevel and MessageSource into the ChromeClient::addMessageToConsole
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Feldman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-04-23 07:41 PDT by Pavel Feldman
Modified: 2009-04-30 09:41 PDT (History)
2 users (show)

See Also:

Attachments
patch (12.35 KB, patch)
2009-04-23 07:41 PDT, Pavel Feldman 		eric: review-
Details | Formatted Diff | Diff
patch (12.93 KB, patch)
2009-04-30 09:03 PDT, Pavel Feldman 		dglazkov: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2009-04-23 07:41:02 PDT 
Pass MessageLevel and MessageSource into the ChromeClient::addMessageToConsole
Comment 1 Pavel Feldman 2009-04-23 07:41:46 PDT 
Created attachment 29710 [details]
patch
Comment 2 Dimitri Glazkov (Google) 2009-04-23 08:27:13 PDT 
Adding Timothy to CC. BTW, you don't need to remove "Reviewed by NOBODY (OOPS!)" from the ChangeLog.
Comment 3 Timothy Hatcher 2009-04-23 09:16:38 PDT 
Comment on attachment 29710 [details]
patch

Patch looks fine. But why is this needed?
Comment 4 Pavel Feldman 2009-04-23 12:18:07 PDT 
(In reply to comment #3)
> (From update of attachment 29710 [details] [review])
> Patch looks fine. But why is this needed?
> 

Hm.. I thought I replied to this one. Anyway, posting again.

Our async/out-of-process version of WebInspector is currently not based on InspectorController. The reason was that we did not want to interfere with Dmitry's unforking effort, yet wanted to experiment. So we came up with these agents concept that basically mimic InspectorController, but separating 'agent' nature from the 'transport'. Now that InspectorController is unforked, I am planning to bring these concepts into the WebKit land and use what we have in Chromium as a proof of concept / experimental playground.

It will take me some time to split InspectorController and I don't want Chromium guys to be blocked by that. Hence I want to expose events that are missing in clients, namely: console severity and xmlhttprequest data. I realize that this is somewhat a temporary measure, but I think it will serve us all well.

Does it make sence?
Comment 5 Timothy Hatcher 2009-04-23 12:29:39 PDT 
Yes, makes sense. It would be good to give a brief reason when posting patches so we don't have to ask/wonder. Thanks for explaining!
Comment 6 Eric Seidel (no email) 2009-04-29 13:17:41 PDT 
Comment on attachment 29710 [details]
patch

Please update the patch to include the nice comments you discussed with Timonthy.  That way someone else can land it w/o thinking about it.
Comment 7 Pavel Feldman 2009-04-30 09:03:54 PDT 
Created attachment 29913 [details]
patch
Comment 8 Pavel Feldman 2009-04-30 09:04:11 PDT 
(In reply to comment #6)
> (From update of attachment 29710 [details] [review])
> Please update the patch to include the nice comments you discussed with
> Timonthy.  That way someone else can land it w/o thinking about it.
> 

Done
Comment 9 Dimitri Glazkov (Google) 2009-04-30 09:24:40 PDT 
Comment on attachment 29913 [details]
patch

Don't forget a URL in the ChangeLog. But I'll fix up and land shortly.
Comment 10 Dimitri Glazkov (Google) 2009-04-30 09:41:46 PDT 
Landed as http://trac.webkit.org/changeset/43063.