Bug 20919 - Constrain the number of messages the inspector shows
Summary: Constrain the number of messages the inspector shows
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P3 Normal
Assignee: Pavel Feldman
Keywords: InRadar
Depends on:
Reported: 2008-09-18 10:35 PDT by Kevin McCullough
Modified: 2009-12-20 04:41 PST (History)
5 users (show)

See Also:

[PATCH] Proposed fix (6.14 KB, patch)
2009-12-19 11:22 PST, Pavel Feldman
darin: review-
Details | Formatted Diff | Diff
[PATCH] Review comments addressed (7.23 KB, patch)
2009-12-19 11:50 PST, Pavel Feldman
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kevin McCullough 2008-09-18 10:35:45 PDT
Related to 20904.

One solution to too many console messages is to have a cap on how many total messages we show and let it be changeable by the user.

Comment 1 Ojan Vafai 2008-09-18 12:35:40 PDT
Just to be clear, the cap is on how many messages we store as the primary goal here is to cap memory usage.
Comment 2 Matt Perry 2009-12-18 15:18:31 PST
This affects chromium extensions significantly, because they are essentially web pages that run for the lifetime of the browser. I'm going to take a crack at a patch.
Comment 3 Ojan Vafai 2009-12-18 15:26:48 PST
This might be more complexity than it's worth, but we could have both date and time caps (e.g. message more than 2 days old get dropped). I like the idea of saving as much memory as we can here.

Of course, if we do either of these, the value needs to be configurable via a preference that is exposed in the inspector UI in some way.
Comment 4 Pavel Feldman 2009-12-19 11:22:06 PST
Created attachment 45242 [details]
[PATCH] Proposed fix
Comment 5 Darin Adler 2009-12-19 11:29:47 PST
Comment on attachment 45242 [details]
[PATCH] Proposed fix

> +        m_expiredConsoleMessageCount += 100;
> +        for (size_t i = 0; i < 100; ++i)
> +            delete m_consoleMessages[i];
> +        m_consoleMessages.remove(0, 100);

Should use a named constant for 100 instead of repeating it 3 times. In fact, I think it would be good to put that constant up at the top of the file along with maximumConsoleMessages.

> +void InspectorFrontend::updateConsoleMessageExpiredCount(const int count)

The const here should be left out. All it does is prevent the function from modifying its local copy of the argument internally. This is not something we should do in WebKit code.

> +        void updateConsoleMessageExpiredCount(const int count);


review- so you can fix these minor quibbles
Comment 6 Darin Adler 2009-12-19 11:30:00 PST
Comment on attachment 45242 [details]
[PATCH] Proposed fix

Also seems the argument should be unsigned rather than int.
Comment 7 Pavel Feldman 2009-12-19 11:50:28 PST
Created attachment 45243 [details]
[PATCH] Review comments addressed
Comment 8 Pavel Feldman 2009-12-20 04:41:28 PST
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/English.lproj/localizedStrings.js
	M	WebCore/inspector/InspectorController.cpp
	M	WebCore/inspector/InspectorController.h
	M	WebCore/inspector/InspectorFrontend.cpp
	M	WebCore/inspector/InspectorFrontend.h
	M	WebCore/inspector/front-end/inspector.js
Committed r52415