Bug 29336

Summary: WebInspector.log() function not protected if console not yet created
Product: WebKit Reporter: Patrick Mueller <pmuellr>
Component: Web Inspector (Deprecated)Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aroben, commit-queue, kmccullough, timothy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
proposed patch - 2009/09/21
timothy: review-
proposed patch - 2009/09/22 none

Description Patrick Mueller 2009-09-17 09:53:41 PDT
The code for WebInspector.log, in inspector.js, looks like this:

    {
       var msg = new WebInspector.ConsoleMessage(...);
       this.console.addMessage();
    }

Unfortunately, this code can be invoked before this.console is set, and so an exception will occur.

Suggest we figure out a way to queue the message if the console is not set, and then do the this.console() on them when it's safe to do so.

Probably means checking for queued messages within this method itself, as well as doing a setInterval() to check.
Comment 1 Patrick Mueller 2009-09-18 06:50:44 PDT
Here's the code I'm currently playing with, didn't want to lose it, due to other changes I'm making in the file:

Also note this fixes the broken repeat-count feature that these messages didn't really support.

WebInspector.log = function(message)
{
    // if we can't log the message, queue it
    if (!WebInspector.ConsoleMessage || !this.console) {
        if (!WebInspector.log.queued)
            WebInspector.log.queued = [];
        WebInspector.log.queued.push(message);
        
        // check for log liveness on a timer
        if (!WebInspector.log.interval) {
            var func = function() {WebInspector.log(null)};
            WebInspector.log.interval = setInterval(func, 1000);
        }
            
        return;
    }
    
    // clear the timer
    if (WebInspector.log.interval) {
        clearInterval(WebInspector.log.interval);
        WebInspector.log.interval = null;
    }
    
    // if there are queued messages, log them
    if (WebInspector.log.queued) {
        var queued = WebInspector.log.queued;
        WebInspector.log.queued = null;
        for (var i = 0; i < queued.length; ++i) {
            WebInspector.log(queued[i]);
        }
    }
    
    // ignore null messages, specifically the one sent by queued timer
    if (!message)
        return;
        
    // calculate the repeat count
    var repeatCount = 1;
    if (message == WebInspector.log.lastMessage) {
        repeatCount = WebInspector.log.repeatCount + 1;
    }

    WebInspector.log.lastMessage = message;
    WebInspector.log.repeatCount = repeatCount;
    
    // post the message
    var msg = new WebInspector.ConsoleMessage(
        WebInspector.ConsoleMessage.MessageSource.Other,
        WebInspector.ConsoleMessage.MessageType.Log,
        WebInspector.ConsoleMessage.MessageLevel.Debug,
        -1,
        null,
        null,
        repeatCount,
        message);

    this.console.addMessage(msg);
}
Comment 2 Patrick Mueller 2009-09-21 06:18:34 PDT
Created attachment 39848 [details]
proposed patch - 2009/09/21

fixes problem of using WebInspector.log() early.  Also fixes repeatCount problem for repeated messages.
Comment 3 Timothy Hatcher 2009-09-21 14:54:00 PDT
Comment on attachment 39848 [details]
proposed patch - 2009/09/21


> +            var func = function() {WebInspector.log(null)};
> +            WebInspector.log.interval = setInterval(func, 1000);

> +    // ignore null messages, specifically the one sent by queued timer
> +    if (!message)
> +        return;

I am not sure passing "null" is the best way to check. What if I want to log somthing and it happens to be undefined/null, I would like you know that. Just make the check function do the "WebInspector.ConsoleMessage && this.console" test and then replay the messages if that passes.

Also just pass the function inline to setInterval or give the variable a better name than func. Put spaces inside the curly braces too.


> +        WebInspector.log.interval = null;

A better way to write this is:

delete WebInspector.log.interval;


> +        for (var i = 0; i < queued.length; ++i) {
> +            WebInspector.log(queued[i]);
> +        }

No need for the braces.
Comment 4 Patrick Mueller 2009-09-22 10:24:35 PDT
Created attachment 39928 [details]
proposed patch - 2009/09/22

issues resolved; split out the function into a number of inner functions for reuse

ConsoleView changed recently so that it requires the messages being logged to be proxy objects, so you'll see that I'm creating a proxy now for the message passed into WebInspector.log() before sending it along to WebInspector.ConsoleMessage()
Comment 5 WebKit Commit Bot 2009-09-22 10:41:52 PDT
Comment on attachment 39928 [details]
proposed patch - 2009/09/22

Clearing flags on attachment: 39928

Committed r48641: <http://trac.webkit.org/changeset/48641>
Comment 6 WebKit Commit Bot 2009-09-22 10:41:56 PDT
All reviewed patches have been landed.  Closing bug.