Bug 73306 - [Chromium] [WebSocket] Add didUpdateBufferedAmount() callback to WebKit::WebSocketClient
Summary: [Chromium] [WebSocket] Add didUpdateBufferedAmount() callback to WebKit::WebS...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 73290
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-29 04:14 PST by Takashi Toyoshima
Modified: 2011-11-30 23:02 PST (History)
5 users (show)

See Also:


Attachments
Patch (2.78 KB, patch)
2011-11-29 06:26 PST, Takashi Toyoshima
no flags Details | Formatted Diff | Diff
Patch for landing (3.93 KB, patch)
2011-11-30 15:23 PST, Takashi Toyoshima
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Takashi Toyoshima 2011-11-29 04:14:56 PST
Now, the only way to know when bufferedAmount value is changed is to poll by bufferedAmount() API.
This change add update notification callback to WebSocketClient API.
Comment 1 Takashi Toyoshima 2011-11-29 06:26:22 PST
Created attachment 116959 [details]
Patch

This change also depends a chromium side change.
Comment 2 WebKit Review Bot 2011-11-29 06:28:47 PST
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Comment 3 Takashi Toyoshima 2011-11-29 06:39:37 PST
FYI: Chromium side issue tracking is
http://code.google.com/p/chromium/issues/detail?id=105708
Comment 4 Darin Fisher (:fishd, Google) 2011-11-30 10:37:29 PST
Comment on attachment 116959 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=116959&action=review

> Source/WebKit/chromium/public/WebSocketClient.h:50
>      virtual void didReceiveMessage(const WebString& message) = 0;

By the way, we have the convention of providing default implementations for all
WebKit API methods that are intended to be implemented by the embedder.  We don't
use the OVERRIDE macro on the Chromium side.  This is all done to minimize the
pain of changing embedder implemented APIs.

However, it has the drawback that the compiler will not tell you if you forget to
implement a method.  I think we have been happier reducing the amount of back-n-forth
required to make WebKit API changes.  So, please feel free to provide default implementations
for these methods.
Comment 5 Takashi Toyoshima 2011-11-30 15:23:08 PST
Created attachment 117284 [details]
Patch for landing
Comment 6 Takashi Toyoshima 2011-11-30 15:23:57 PST
Comment on attachment 116959 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=116959&action=review

> Source/WebKit/chromium/public/WebSocketClient.h:53
> +    virtual void didUpdateBufferedAmount(unsigned long bufferedAmount) = 0;

I see.
I add default implementations and will remove OVERRIDE macro on the Chromium side.
Comment 7 WebKit Review Bot 2011-11-30 15:25:36 PST
Comment on attachment 117284 [details]
Patch for landing

Rejecting attachment 117284 [details] from commit-queue.

toyoshim@chromium.org does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.
Comment 8 Takashi Toyoshima 2011-11-30 15:29:41 PST
Comment on attachment 117284 [details]
Patch for landing

Oh, I could not use webkit-patch land-safely.
Could anyone who has committer right set it CQ+?
Comment 9 WebKit Review Bot 2011-11-30 23:02:03 PST
Comment on attachment 117284 [details]
Patch for landing

Clearing flags on attachment: 117284

Committed r101614: <http://trac.webkit.org/changeset/101614>
Comment 10 WebKit Review Bot 2011-11-30 23:02:08 PST
All reviewed patches have been landed.  Closing bug.