[webkit-reviews] review requested: [Bug 24529] WebInspector: HTML5 Offline Web Applications Support (ApplicationCache) : [Attachment 60499] [PATCH] Part 1: Backend -> Frontend Messages. ApplicationCache Status and Connectivity Status.

[bugzilla-daemon at webkit.org]

Joseph Pecoraro <joepeck at webkit.org> has asked	for review:
Bug 24529: WebInspector: HTML5 Offline Web Applications Support
(ApplicationCache)
https://bugs.webkit.org/show_bug.cgi?id=24529

Attachment 60499: [PATCH] Part 1: Backend -> Frontend Messages.
ApplicationCache Status and Connectivity Status.
https://bugs.webkit.org/attachment.cgi?id=60499&action=review

------- Additional Comments from Joseph Pecoraro <joepeck at webkit.org>
> > WebCore/loader/appcache/ApplicationCacheGroup.cpp:386
> >  +		   
applicationCacheAgent->updateApplicationCacheStatus(static_cast<int>(status));
> > How about we include the status from the new agent instead of losing the
semantics here?
>
> You mean keep the enum all the way through? I'll give that a shot.

Done.


> > WebCore/page/Page.cpp:103
> >  +	#if ENABLE(INSPECTOR) && ENABLE(OFFLINE_WEB_APPLICATIONS)
> > How do we further encapsulate inspector complexity into the controller?
> > I don't like this new code in the Page. How about we add a
> > eventNames().onlineEvent / eventNames().offlineEvent listeners to the
document
> 
> So, keep as much Inspector code in WebCore/inspector as possible?
> That sounds reasonable. I'll give it a shot.

Hmm, this seems like it might be more difficult than its worth. The
document will change as the page is navigated. So I would have to
keep track and keep adding event listeners to the document. I could
do something in NetworkStateNotifier, but I think the current
approach is lightweight. Its also where I would look to find the code.