[Webkit-unassigned] [Bug 26046] Limit the number of resources and console messages cached by the InspectorController

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=26046





------- Comment #11 from pfeldman at chromium.org  2009-05-29 13:52 PDT -------
(In reply to comment #9)
> (From update of attachment 30750 [review])
> Looking good!
> 
> > -    ASSERT(m_frontend);
> > +    ASSERT(m_frontend.get());
> 
> You shoudln't need that change.
>

Done

> >      if (!m_frontend.get())
> >          return;
> 
> No need to call .get().
> 
> > +    if (m_frontend.get())
> > +        m_frontend->resourceTrackingWasEnabled();
> 
> No need to call .get().
> 
> > +    if (m_frontend.get())
> > +        m_frontend->resourceTrackingWasDisabled();
> 
> No need to call .get().
>

Done all

> 
> > +    var enableOption = function(text, checked) {
> 
> I prefer function enableOption() instead.
>

Done

> > +        var caption = document.createElement("text");
> > +        caption.textContent = text;
> > +        caption.addEventListener("mousedown", function() { option.checked = true; }, false);
> > +        li.appendChild(caption);
> 
> You should do: <label><input type="radio"> text</label>, then you wont need the
> event listener.
> 

Done

> > +    alwaysWasChosen: function() {
> > +        return this.enabledAlways.checked;
> >      }
> 
> Maybe a getter for this? get alwaysEnabled()?
>


Done

> > +    _toggleProfiling: function(opt_always)
> > +    _toggleResourceTracking: function(opt_always)
> > +    _toggleDebugging: function(opt_always)
> 
> These variables should not have an underscore and be camelcase, and no
> abbrivation.
>

Done

> 
> > +    text-align: center;
> > +    margin-left: 20px;
> 
> What was this change for?
> 
>

This was to align disclaimer with the radio buttons. 'text-align' was not
required, removed.

> > +.panel-enabler-view text {
> 
> An element named text isn't good, just use a span. But this wont be needed if
> you use by label suggestion.
> 

Done

> > +	color: rgb(6, 6, 6);
> 
> Tab here.
> 
> > +.panel-enabler-view.resources img {
> > +    content: url(Images/scriptsSilhouette.png);
> > +}
> 
> I'll make a new resource silhouette soon, but we could land like this.
> 
> I'll attach the radio button dot soon. You can use multiple background show the
> dot.
>

Thanks!


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.