Bug 57009

Summary: Web Inspector: [REGRESSION] console message count is not updated in scripts and resources panels
Product: WebKit Reporter: Pavel Podivilov <podivilov>
Component: Web Inspector (Deprecated)Assignee: Pavel Podivilov <podivilov>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, vsevik, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch yurys: review+

Description Pavel Podivilov 2011-03-24 06:30:31 PDT
Web Inspector: [REGRESSION] console message count is not updated in scripts and resources panels.
Comment 1 Pavel Podivilov 2011-08-11 03:10:42 PDT
Created attachment 103597 [details]
Patch
Comment 2 Vsevolod Vlasov 2011-08-11 07:53:28 PDT
Comment on attachment 103597 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=103597&action=review

This does not work for me.
Sometimes it leads to resource element bubble and source frame message showing different number of messages. I think this happens because some messages are processed twice in SourceFrame because of an extra MessageAdded event.

> Source/WebCore/inspector/front-end/ConsoleView.js:145
> +                    console.dispatchEventToListeners(WebInspector.ConsoleView.Events.MessageAdded, msg);

I believe this could cause side effects
Comment 3 Pavel Podivilov 2011-08-11 08:34:12 PDT
Created attachment 103630 [details]
Patch
Comment 4 Pavel Podivilov 2011-08-11 08:37:19 PDT
(In reply to comment #2)
> (From update of attachment 103597 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=103597&action=review
> 
> This does not work for me.
> Sometimes it leads to resource element bubble and source frame message showing different number of messages. I think this happens because some messages are processed twice in SourceFrame because of an extra MessageAdded event.

Thanks for spotting this! We should use last message totalRepeatCount to update source frame bubble properly.

> 
> > Source/WebCore/inspector/front-end/ConsoleView.js:145
> > +                    console.dispatchEventToListeners(WebInspector.ConsoleView.Events.MessageAdded, msg);
> 
> I believe this could cause side effects

There are two listeners: DebuggerPresentationModel and ResourceTreeModel, both add message to messages list and update corresponding source frame. I don't think there can be any side effects.
Comment 5 Vsevolod Vlasov 2011-08-11 08:50:42 PDT
Looks good to me now.
Comment 6 Yury Semikhatsky 2011-08-11 23:57:52 PDT
Comment on attachment 103630 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=103630&action=review

> Source/WebCore/inspector/front-end/SourceFrame.js:561
> +    _updateMessageRepeatCount: function(rowMessage)

I'd rather introduce RowMessage class and have this method on the new class, then have RowMessage listening to update events propagated by corresponding ConsoleMessage. What do you think?
Comment 7 Pavel Podivilov 2011-08-12 02:22:34 PDT
(In reply to comment #6)
> (From update of attachment 103630 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=103630&action=review
> 
> > Source/WebCore/inspector/front-end/SourceFrame.js:561
> > +    _updateMessageRepeatCount: function(rowMessage)
> 
> I'd rather introduce RowMessage class and have this method on the new class, then have RowMessage listening to update events propagated by corresponding ConsoleMessage. What do you think?

I think it's a good idea, but quite unrelated to this bug. Maybe we could do this as a separate change since it requires a bunch of additional changes in resources and console?
Comment 8 Pavel Podivilov 2011-08-12 08:35:00 PDT
Committed r92964: <http://trac.webkit.org/changeset/92964>