Bug 57009 - Web Inspector: [REGRESSION] console message count is not updated in scripts and resources panels
Summary: Web Inspector: [REGRESSION] console message count is not updated in scripts a...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Podivilov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-24 06:30 PDT by Pavel Podivilov
Modified: 2011-08-12 08:35 PDT (History)
10 users (show)

See Also:


Attachments
Patch (5.02 KB, patch)
2011-08-11 03:10 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff
Patch (5.10 KB, patch)
2011-08-11 08:34 PDT, Pavel Podivilov
yurys: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>