[webkit-reviews] review granted: [Bug 37602] AppCache Progress Events - need to include additional info per the spec : [Attachment 59303] chromium webkit api for the extra info

[bugzilla-daemon at webkit.org]

Dumitru Daniliuc <dumi at chromium.org> has granted Michael Nordman
<michaeln at google.com>'s request for review:
Bug 37602: AppCache Progress Events - need to include additional info per the
spec
https://bugs.webkit.org/show_bug.cgi?id=37602

Attachment 59303: chromium webkit api for the extra info
https://bugs.webkit.org/attachment.cgi?id=59303&action=review

------- Additional Comments from Dumitru Daniliuc <dumi at chromium.org>
> Index: WebKit/chromium/ChangeLog
> ===================================================================
> --- WebKit/chromium/ChangeLog (revision 61565)
> +++ WebKit/chromium/ChangeLog (working copy)
> @@ -1,3 +1,17 @@
> +2010-06-21  Michael Nordman	<michaeln at google.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Additional API to pass additional info needed for AppCache progress
events.
> +	   https://bugs.webkit.org/show_bug.cgi?id=37602
> +
> +	   Also, delete an obsolete method from the worker API.
> +
> +	   * public/WebApplicationCacheHostClient.h: define a new method
> +	   * public/WebSharedWorker.h: remove an obsolte method
> +	   * src/ApplicationCacheHostInternal.h: implement the new method
> +	  
(WebCore::ApplicationCacheHostInternal::notifyProgressEventListener):
> +
>  2010-06-21  Kent Tamura  <tkent at chromium.org>
>  
>	   Reviewed by Dimitri Glazkov.
> Index: WebKit/chromium/public/WebApplicationCacheHostClient.h
> ===================================================================
> --- WebKit/chromium/public/WebApplicationCacheHostClient.h	(revision
61562)
> +++ WebKit/chromium/public/WebApplicationCacheHostClient.h	(working copy)
> @@ -39,8 +39,9 @@ namespace WebKit {
>  // This interface is used by the embedder to call into webkit.
>  class WebApplicationCacheHostClient {
>  public:
> -    // Called to fire the event in the scriptable interface.
> +    // Called to fire events in the scriptable interface.
>      virtual void notifyEventListener(WebApplicationCacheHost::EventID) = 0;
> +    virtual void notifyProgressEventListener(const WebURL&, int num_total,
int num_complete) = 0;
>  
>  protected:
>      // Should not be deleted by the embedder.
> Index: WebKit/chromium/public/WebSharedWorker.h
> ===================================================================
> --- WebKit/chromium/public/WebSharedWorker.h	(revision 61562)
> +++ WebKit/chromium/public/WebSharedWorker.h	(working copy)
> @@ -63,15 +63,6 @@ public:
>				       const WebString& sourceCode,
>				       long long scriptResourceAppCacheID) = 0;

>  
> -    // FIXME(michaeln): Remove this after the roll and adjusting to it.
> -    virtual void startWorkerContext(const WebURL& scriptURL,
> -				       const WebString& name,
> -				       const WebString& userAgent,
> -				       const WebString& sourceCode)
> -    {
> -	   startWorkerContext(scriptURL, name, userAgent, sourceCode, 0);
> -    }
> -
>      class ConnectListener {
>      public:
>	   // Invoked once the connect event has been sent so the caller can
free this object.
> Index: WebKit/chromium/src/ApplicationCacheHostInternal.h
> ===================================================================
> --- WebKit/chromium/src/ApplicationCacheHostInternal.h	(revision
61562)
> +++ WebKit/chromium/src/ApplicationCacheHostInternal.h	(working copy)
> @@ -39,6 +39,7 @@
>  #include "WebFrameImpl.h"
>  #include "WebKit.h"
>  #include "WebKitClient.h"
> +#include "WebURL.h"
>  
>  namespace WebCore {
>  
> @@ -57,6 +58,17 @@ public:
>	  
m_innerHost->notifyDOMApplicationCache(static_cast<ApplicationCacheHost::EventI
D>(eventID));
>      }
>  
> +    virtual void notifyProgressEventListener(const WebKit::WebURL&, int
num_total, int num_complete) 
> +    {
> +	   // FIXME: Modify webcore's progress event handling to carry the
extra info and alter the
> +	   // layout tests to not fail when the more recently specified 'final'
event is raised.
> +	   // For now, we're eating the extra info and that last event.
> +	   // See https://bugs.webkit.org/show_bug.cgi?id=37602
> +	   if (num_complete == num_total)
> +	       return;
> +	   notifyEventListener(WebKit::WebApplicationCacheHost::ProgressEvent);

> +    }
> +
>      static WebKit::WebApplicationCacheHost*
toWebApplicationCacheHost(ApplicationCacheHost* innerHost)
>      {
>	   if (innerHost && innerHost->m_internal.get())

WebKit/chromium/public/WebApplicationCacheHostClient.h:44
 +	virtual void notifyProgressEventListener(const WebURL&, int num_total,
int num_complete) = 0;
is ApplicationCacheHostInternal is the only class that inherits from
WebApplicationCacheHostClient?

WebKit/chromium/src/ApplicationCacheHostInternal.h:63
 +	    // FIXME: Modify webcore's progress event handling to carry the
extra info and alter the
minor: s/webcore/WebCore/