[Webkit-unassigned] [Bug 24529] WebInspector: HTML5 Offline Web Applications Support (ApplicationCache)

Thu Jul 8 00:30:38 PDT 2010

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

--- Comment #75 from Michael Nordman <michaeln at google.com>  2010-07-08 00:30:36 PST ---
> It is easier to land incremental patches.

And so long as we share a reasonably common idea of where we're going, its easier to have multiple people landing patches incrementally too!

Please understand where I'm coming from. I'm not saying it has to be landed in it final form on day one. What I am suggesting is that there is a reasonably clear idea of both what the feature set is and roughly how the code will be arranged to accomplish that prior to landing significant patches. And that we have a common understanding of those things.

My most recent questions (#72) were intended to ferret out that reasonably clear idea of "how". My earlier questions in response to Kavita's patch (#42) were to ferret out that reasonably clear idea of "what".

(In reply to comment #74)
> Reading through the thread, I can say few things:
> - It is a shame ApplicationCache.h was used, but there was really no sign or comment preventing it from being used.

I have tried to communicate this whenever i see changes involving appcache, in this bug see comment #15.

> Here is what I would suggest:
> 1) For all the items Michael enumerated in #72, please file the bugs
> 2) Lets hide appcache from the UI unless it is ready and both parities are happy with it
> 3) Lets fix chromium fast so that we could iterate against working appcache inspection

Kavita's got some work in progress for the chrome side of things and for the WebKit API layer.
http://codereview.chromium.org/2821005/show
http://codereview.chromium.org/2804026/show
It would be good to get those put to bed, so we have real data to provide to the inspector agent. There's nothing blocking progress on these. Until we have a reasonable agent interface to talk to, we can just drop whatever data comes in on the floor for now.

> 4) Lets make sure ApplicationCacheHost.h has a nice interface that Inspector could talk to and move ApplicationCache.h (and rest of the implementation classes) to under WebCore/loader/appcache/internal.

Oooo... relocating stuff to /loader/appcache/internal is a very good idea!

5) Think thru, agree upon, and land the common interfaces sooner rather than later. Once these interfaces are in place, things can go on in parallel more smoothly. In this case the interfaces of interest are:
- The user interface.
- Those methods of class InspectorApplicationCacheAgent intended to be called by appcache/internal to "push" stuff.
- Those methods of class ApplicationCacheHost intended to be called by the InspectorApplicationCacheAgent to "pull" stuff.

Probably would have been good to have just those interfaces go in first. Without code to "push" stuff and returning null in response to "pull" stuff. (Its not too late to revert and do just that).

The "push" oriented methods of InspectorApplicationCacheAgent may want to take an ApplicationCacheHost parameter to the inspector side of things can distinguish between events and state changes in different subframes. Per the earlier discussion, in the UI we eventually want 1 icon per subframe that's actually associated with an appcache (right?).

6) Give people ample opportunity to review patches.

>> - When multiple top-level pages are associated with the same appcache group, it looks
>> like only one (actually could be none) will be notified of update status changes, what
>> about the others? I'm not talking about the nested frames, multiple pages using the
>> same cache. What is your plan for that?
>
> These all deal with multiple application caches

This one in particular is a little different. It doesn't have anything todo with multiple application cache instances. Rather multiple documents associated to the same cache. They way you're plumbing update status changes from the ApplicationCacheGroup to the InspectorAgent is flawed. Take a look at how m_frame is used. It only gets you to the context that initiated the update. It might be better to siphon off these state changes to the agent where the events are fired into the DOM ... see ApplicationCacheHost::notifyDOMApplicationCache.

>> - Can you separate populating the list view on the left-hand-side from
>> the detailed grid view on the right-hand-side and defer fetching the
>> resource list until needed? That could help make the interface more responsive.
>
>This is how it works right now. When you select the ApplicationCache from
>the left-hand-side it then fetches the data lazily.

This method looks like it returns info used to populate the list on the left as well as info used to populate the grid. So then what is the basis for the list on the left-hand-side prior to lazily fetching? How do you know whether or not to put an icon there and what labels to put on that icon? Or do you just always plot an icon with generic labeling on it?

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