Bug 72123 - Web Inspector: Application cache status should be updated after swapCache().
Summary: Web Inspector: Application cache status should be updated after swapCache().
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: Vsevolod Vlasov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-11 06:28 PST by Vsevolod Vlasov
Modified: 2011-11-16 09:01 PST (History)
14 users (show)

See Also:


Attachments
Patch (40.02 KB, patch)
2011-11-14 13:13 PST, Vsevolod Vlasov
pfeldman: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Vsevolod Vlasov 2011-11-11 06:28:27 PST
Application cache status should be updated on swap.

Refresh button could be deleted as part if this change, since there is no any need in it.
Comment 1 Vsevolod Vlasov 2011-11-11 10:40:55 PST
Application cache view should be updated automatically 
 - when shown
 - in the following cases if view is already shown
   - when there are no resources yet and new status is IDLE
   - when old status was UPDATEREADY and new status is IDLE
Comment 2 Vsevolod Vlasov 2011-11-14 13:13:18 PST
Created attachment 115016 [details]
Patch
Comment 3 Pavel Feldman 2011-11-16 00:11:04 PST
Comment on attachment 115016 [details]
Patch

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

r+ given the macros are removed prior to landing.

> Source/WebCore/ChangeLog:3
> +        Web Inspector: Application cache status should be updated on swap.

What is "swap" could you please add more details into the changelog?

> Source/WebCore/inspector/front-end/ApplicationCacheItemsView.js:130
> +        if (this.isShowing() && this._status === applicationCache.IDLE && (oldStatus === applicationCache.UPDATEREADY || !this._resources))

I noticed that you are doing unconditional update upon wasShown. You could instead set a dirty flag here and only update in case of dirty.

> Source/WebCore/loader/appcache/ApplicationCacheHost.cpp:244
> +#if ENABLE(INSPECTOR)

You don't need this macro here. Instrumentation should take care of overhead, while frame getter and != check are not expensive.

> Source/WebCore/loader/appcache/ApplicationCacheHost.cpp:459
> +#if ENABLE(INSPECTOR)

ditto.

> LayoutTests/http/tests/inspector/resource-tree/appcache-test.js:3
>  function createAndNavigateIFrame(url)

Drive-by: why is this file under the resource-tree folder?
Comment 4 Vsevolod Vlasov 2011-11-16 08:07:22 PST
> > Source/WebCore/ChangeLog:3
> > +        Web Inspector: Application cache status should be updated on swap.
> 
> What is "swap" could you please add more details into the changelog?
swapCache() is a method on applicationCache object. I changed bug summary and updated changelog.

 
> > Source/WebCore/inspector/front-end/ApplicationCacheItemsView.js:130
> > +        if (this.isShowing() && this._status === applicationCache.IDLE && (oldStatus === applicationCache.UPDATEREADY || !this._resources))
> 
> I noticed that you are doing unconditional update upon wasShown. You could instead set a dirty flag here and only update in case of dirty.

Done.

> > Source/WebCore/loader/appcache/ApplicationCacheHost.cpp:244
> > +#if ENABLE(INSPECTOR)
> 
> You don't need this macro here. Instrumentation should take care of overhead, while frame getter and != check are not expensive.
> 
> > Source/WebCore/loader/appcache/ApplicationCacheHost.cpp:459
> > +#if ENABLE(INSPECTOR)
> 
> ditto.
Done.

> > LayoutTests/http/tests/inspector/resource-tree/appcache-test.js:3
> >  function createAndNavigateIFrame(url)
> 
> Drive-by: why is this file under the resource-tree folder?
Moved inspector appcache tests to their own folder.
Comment 5 Vsevolod Vlasov 2011-11-16 08:10:03 PST
Committed r100454: <http://trac.webkit.org/changeset/100454>
Comment 6 Steve Block 2011-11-16 09:01:19 PST
Committed r100458: <http://trac.webkit.org/changeset/100458>