Bug 72123 – Web Inspector: Application cache status should be updated after swapCache().

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Vsevolod Vlasov]

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.

[Vsevolod Vlasov]

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

[Vsevolod Vlasov]

Created attachment 115016 [details]
Patch

[Pavel Feldman]

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?

[Vsevolod Vlasov]

> > 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.

[Vsevolod Vlasov]

Committed r100454: <http://trac.webkit.org/changeset/100454>

[Steve Block]

Committed r100458: <http://trac.webkit.org/changeset/100458>