[Webkit-unassigned] [Bug 24529] Add support in Web Inspector for examining/changing data for HTML5 offline-Web-applications (application cache)

[bugzilla-daemon at webkit.org]

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





--- Comment #24 from Joseph Pecoraro <joepeck at webkit.org>  2010-04-25 12:10:59 PST ---
(In reply to comment #22)
> We used to indent these. Did it change?

Fixed.

> You should limit the lifetime of this agent to the lifetime of the front-end.
> You would not need to check for frontend existence then. In fact I don't see
> where you clear this field, so the check does not work anyways.

Fixed but can be improved. Yes this is the lifetime of the front-end, but not
necessarily the lifetime of an ApplicationCache which is not always needed.


> This should instead be in the disconnectFrontend. Lifetime of this agent is
> similar to the InspectorDOMAgent. You could further generalize releaseDOMAgent
> to releaseAgents and do the tear down there. That way you can safely reference
> front-end from within the agent.

Fixed. Changed releaseDOMAgent to releaseFrontendLifetimeAgents.


> (now that you touched it - it should have been called cookies back then (and
> the other guy rawCookies) with no get prefix.)

These changes muddy the waters of an already large patch. How about a follow up
patch?


> [Online] [Idle] confuses me on the second screenshot. Should one of the
> statuses go to the tree view or to the table header?

That may be possible, I will experiment:

  - Online / Offline is the connectivity indicator. This could probably
    go someplace prominent, as it is not specifically related to the
    applicationCache.
  - IDLE / etc. is the applicationCache status. This could probably
    go into the tree view or table header.


> We should distinguish these by domain when we have multiple iframes loaded,
> right? So it should be just domain name under the Applications Cache section
> item? (Just as cookies?)

Sounds good. Not done yet.


> WebCore/loader/appcache/ApplicationCacheGroup.cpp:476
>  +      // Because willSendRequest only gets called during redirects, we
> initialize
> Is this appcache loader's specifics?

This is actually coded in ResourceHandleMac. It quick breaks, before delegating
to the ResourceClient's willSendRequest. It was a check added for performance
reasons due to a Leopard change in CFNetwork that started spitting out more
notifications.

I think what you're asking is should the Inspector notifications in
ResourceLoader do the same thing. Apparently not. I think it triggers the first
on its own, separate from the ResourceClient delegates. I had tracked it down
at one point, but I've since forgotten. I think it was in ResourceLoader
initialization though.


>> page->inspectorController()->willSendRequest(m_currentResourceIdentifier, request, redirectResponse);
>
> Pardon my ignorance, but what exactly this loader is loading. Options are:
> 1) it fetches manifest file
> 2) it fetches and caches resources in manifest
> 3) it loads cached resources mentioned in manifest

Ahh. I actually haven't fully tested this code. My understanding of it is that
it is both (1) and (2). Initially it is the manifest, and then for each
resource specified to be fetched in the manifest it goes though here
(serially?). You' were right to r- this, I actually haven't tested this enough.
Specifically I have to test this where I actually download a number of files to
see how they show up in the Resources Panel.


> Given that the use of this guy is limited to the Page and AppCache class, I'd
> inline access path in Page and get it via controller in AppCache subsystem. In
> contrast, timeline has a convenience getter here because it is used all over
> the place in WebCore

Fixed. I've removed the changes to Page.h and always go through the inspector
controller.

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