Bug 20625 - ConsoleMessage should have a "type" attribute
Summary: ConsoleMessage should have a "type" attribute
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Enhancement
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-09-03 07:51 PDT by Keishi Hattori
Modified: 2009-07-12 23:15 PDT (History)
3 users (show)

See Also:


Attachments
patch (23.11 KB, patch)
2008-09-30 05:48 PDT, Keishi Hattori
timothy: review-
Details | Formatted Diff | Diff
Patch to add MessageType to ConsoleMessage (58.60 KB, patch)
2009-07-08 22:50 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
forgot to write WebKit ChangeLog (58.47 KB, patch)
2009-07-09 00:18 PDT, Keishi Hattori
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keishi Hattori 2008-09-03 07:51:38 PDT
ConsoleMessage should have a "type" attribute to separate console message types from console message levels.
Comment 1 Keishi Hattori 2008-09-30 05:48:44 PDT
Created attachment 23938 [details]
patch

Clean up. Moved message types from MessageLevel to MessageType.
Comment 2 Timothy Hatcher 2008-10-02 12:17:38 PDT
Comment on attachment 23938 [details]
patch

This needs some updates to work with TOT.


             // This _format call passes in true for the plainText argument. The result's textContent is
             // used for inline message bubbles in SourceFrames, or other plain-text representations.
-            this.message = this._format(Array.prototype.slice.call(arguments, 6), true).textContent;
+            this.message = this._format(Array.prototype.slice.call(arguments, 7), true).textContent;
 
             // The formatedMessage property is used for the rich and interactive console.
-            this.formattedMessage = this._format(Array.prototype.slice.call(arguments, 6));
+            this.formattedMessage = this._format(Array.prototype.slice.call(arguments, 7));
             break;

We no longer call _format twice.

Otherwise looks good! But r- until it can apply to TOT,
Comment 3 Keishi Hattori 2009-07-08 22:50:23 PDT
Created attachment 32502 [details]
Patch to add MessageType to ConsoleMessage
Comment 4 Keishi Hattori 2009-07-09 00:18:21 PDT
Created attachment 32504 [details]
forgot to write WebKit ChangeLog
Comment 5 Timothy Hatcher 2009-07-09 13:41:50 PDT
Comment on attachment 32504 [details]
forgot to write WebKit ChangeLog


> -            case WebInspector.ConsoleMessage.MessageLevel.StartGroup:
> -                element.addStyleClass("console-group-title-level");
> +        }
> +        
> +        if (this.type === WebInspector.ConsoleMessage.MessageType.StartGroup) {
> +            element.addStyleClass("console-group-title");

Why can't this be in the switch liek before?

> diff --git a/WebCore/workers/GenericWorkerTask.h b/WebCore/workers/GenericWorkerTask.h
> index 3611fea..d6a9994 100644
> --- a/WebCore/workers/GenericWorkerTask.h
> +++ b/WebCore/workers/GenericWorkerTask.h
> @@ -328,6 +328,56 @@ namespace WebCore {
>          P6 m_parameter6;
>          P7 m_parameter7;
>      };
> +    
> +    template<typename P1, typename MP1, typename P2, typename MP2, typename P3, typename MP3, typename P4, typename MP4, typename P5, typename MP5, typename P6, typename MP6, typename P7, typename MP7, typename P8, typename MP8>
> +    class GenericWorkerTask8 : public ScriptExecutionContext::Task {
> +    public:
> +        typedef void (*Method)(ScriptExecutionContext*, MP1, MP2, MP3, MP4, MP5, MP6, MP7, MP8);
> +        typedef GenericWorkerTask8<P1, MP1, P2, MP2, P3, MP3, P4, MP4, P5, MP5, P6, MP6, P7, MP7, P8, MP8> GenericWorkerTask;
> +        typedef typename GenericWorkerTaskTraits<P1>::ParamType Param1;
> +        typedef typename GenericWorkerTaskTraits<P2>::ParamType Param2;
> +        typedef typename GenericWorkerTaskTraits<P3>::ParamType Param3;
> +        typedef typename GenericWorkerTaskTraits<P4>::ParamType Param4;
> +        typedef typename GenericWorkerTaskTraits<P5>::ParamType Param5;
> +        typedef typename GenericWorkerTaskTraits<P6>::ParamType Param6;
> +        typedef typename GenericWorkerTaskTraits<P7>::ParamType Param7;
> +        typedef typename GenericWorkerTaskTraits<P8>::ParamType Param8;
> +        
> +        static PassRefPtr<GenericWorkerTask> create(Method method, Param1 parameter1, Param2 parameter2, Param3 parameter3, Param4 parameter4, Param5 parameter5, Param6 parameter6, Param7 parameter7, Param8 parameter8)
> +        {
> +            return adoptRef(new GenericWorkerTask(method, parameter1, parameter2, parameter3, parameter4, parameter5, parameter6, parameter7, parameter8));
> +        }
> +        
> +    private:
> +        GenericWorkerTask8(Method method, Param1 parameter1, Param2 parameter2, Param3 parameter3, Param4 parameter4, Param5 parameter5, Param6 parameter6, Param7 parameter7, Param8 parameter8)
> +        : m_method(method)
> +        , m_parameter1(parameter1)
> +        , m_parameter2(parameter2)
> +        , m_parameter3(parameter3)
> +        , m_parameter4(parameter4)
> +        , m_parameter5(parameter5)
> +        , m_parameter6(parameter6)
> +        , m_parameter7(parameter7)
> +        , m_parameter8(parameter8)
> +        {
> +        }
> +        
> +        virtual void performTask(ScriptExecutionContext* context)
> +        {
> +            (*m_method)(context, m_parameter1, m_parameter2, m_parameter3, m_parameter4, m_parameter5, m_parameter6, m_parameter7, m_parameter8);
> +        }
> +        
> +    private:
> +        Method m_method;
> +        P1 m_parameter1;
> +        P2 m_parameter2;
> +        P3 m_parameter3;
> +        P4 m_parameter4;
> +        P5 m_parameter5;
> +        P6 m_parameter6;
> +        P7 m_parameter7;
> +        P8 m_parameter8;
> +    };
>  
>      template<typename P1, typename MP1>
>      PassRefPtr<ScriptExecutionContext::Task> createCallbackTask(
> @@ -413,6 +463,21 @@ namespace WebCore {
>                  CrossThreadCopier<P7>::copy(parameter7));
>      }
>  
> +    template<typename P1, typename MP1, typename P2, typename MP2, typename P3, typename MP3, typename P4, typename MP4, typename P5, typename MP5, typename P6, typename MP6, typename P7, typename MP7, typename P8, typename MP8>
> +    PassRefPtr<ScriptExecutionContext::Task> createCallbackTask(
> +                                                                void (*method)(ScriptExecutionContext*, MP1, MP2, MP3, MP4, MP5, MP6, MP7, MP8),
> +                                                                const P1& parameter1, const P2& parameter2, const P3& parameter3, const P4& parameter4, const P5& parameter5, const P6& parameter6, const P7& parameter7, const P8& parameter8)
> +    {
> +        return GenericWorkerTask8<typename CrossThreadCopier<P1>::Type, MP1, typename CrossThreadCopier<P2>::Type, MP2, typename CrossThreadCopier<P3>::Type, MP3,
> +        typename CrossThreadCopier<P4>::Type, MP4, typename CrossThreadCopier<P5>::Type, MP5, typename CrossThreadCopier<P6>::Type, MP6,
> +        typename CrossThreadCopier<P7>::Type, MP7, typename CrossThreadCopier<P8>::Type, MP8>::create(
> +                                                           method,
> +                                                           CrossThreadCopier<P1>::copy(parameter1), CrossThreadCopier<P2>::copy(parameter2),
> +                                                           CrossThreadCopier<P3>::copy(parameter3), CrossThreadCopier<P4>::copy(parameter4),
> +                                                           CrossThreadCopier<P5>::copy(parameter5), CrossThreadCopier<P6>::copy(parameter6),
> +                                                           CrossThreadCopier<P7>::copy(parameter7), CrossThreadCopier<P8>::copy(parameter8));
> +    }
> +
>  } // namespace WebCore
>  
>  #endif // ENABLE(WORKERS)

What is up with the change to WebCore/workers/GenericWorkerTask.h? I don't think that is relivant to this change.
Comment 6 Keishi Hattori 2009-07-09 14:43:19 PDT
(In reply to comment #5)
> (From update of attachment 32504 [details])
> 
> > -            case WebInspector.ConsoleMessage.MessageLevel.StartGroup:
> > -                element.addStyleClass("console-group-title-level");
> > +        }
> > +        
> > +        if (this.type === WebInspector.ConsoleMessage.MessageType.StartGroup) {
> > +            element.addStyleClass("console-group-title");
> 
> Why can't this be in the switch liek before?
>

I only had one case to switch depending on "this.type" so I used an if statement.

> What is up with the change to WebCore/workers/GenericWorkerTask.h? I don't
> think that is relivant to this change.

I added a type argument to "WorkerContext::addMessage"
then to "WorkerMessagingProxy::postConsoleMessageToWorkerObject"
then to "postConsoleMessageTask"

Then "createCallbackTask" needed an additional argument so I added "GenericWorkerTask8".

Is this not necessary? I'm not sure when WorkerContext::addMessage gets called.
Comment 7 Brent Fulgham 2009-07-12 22:09:38 PDT
Patch needed some tiny adjustments to XSSAuditor.cpp, but tests pass after my adjustments so I think it's okay.

Landed in http://trac.webkit.org/changeset/45786.
Comment 8 Brent Fulgham 2009-07-12 23:15:04 PDT
Note that the original patch broke Qt, wx, Gtk+, and Windows support as written.

See:

http://trac.webkit.org/changeset/45789
http://trac.webkit.org/changeset/45790