Issue 6745015: Make PPAPI PostMessage behave asynchronously.

Issue 6745015: Make PPAPI PostMessage behave asynchronously. (Closed)

Created:
9 years, 9 months ago by dmichael(do not use this one)

Modified:
9 years, 7 months ago

Reviewers:
brettw, piman

CC:
chromium-reviews, darin-cc_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src/

Visibility:
Public.

Description

Make PPAPI PostMessage behave asynchronously. Add PostMessage test to ppapi_uitest.cc. BUG=None TEST=test_post_message.cc Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=79497

Patch Set 4 : '' #

Created: 9 years, 9 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+91 lines, -15 lines)			Patch
M	chrome/test/ui/ppapi_uitest.cc	View	1 2 3	1 chunk	+4 lines, -0 lines	0 comments	Download
M	ppapi/tests/test_post_message.cc	View	1 2 3	6 chunks	+20 lines, -4 lines	0 comments	Download
M	webkit/plugins/ppapi/message_channel.h	View	1 2 3	3 chunks	+18 lines, -0 lines	0 comments	Download
M	webkit/plugins/ppapi/message_channel.cc	View	1 2 3	7 chunks	+49 lines, -11 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

piman

Looks good mostly, but asserting on not-currently-supported data is rather rude. http://codereview.chromium.org/6745015/diff/8/webkit/plugins/ppapi/message_channel.cc File webkit/plugins/ppapi/message_channel.cc (right): ...

9 years, 9 months ago (2011-03-25 23:21:10 UTC) #2

dmichael(do not use this one)

http://codereview.chromium.org/6745015/diff/8/webkit/plugins/ppapi/message_channel.cc File webkit/plugins/ppapi/message_channel.cc (right): http://codereview.chromium.org/6745015/diff/8/webkit/plugins/ppapi/message_channel.cc#newcode101 webkit/plugins/ppapi/message_channel.cc:101: DCHECK(false); On 2011/03/25 23:21:10, piman wrote: > NOTIMPLEMENTED() instead ...

9 years, 9 months ago (2011-03-26 02:22:56 UTC) #3

brettw

LGTM, just some style nits. http://codereview.chromium.org/6745015/diff/4003/webkit/plugins/ppapi/message_channel.cc File webkit/plugins/ppapi/message_channel.cc (right): http://codereview.chromium.org/6745015/diff/4003/webkit/plugins/ppapi/message_channel.cc#newcode105 webkit/plugins/ppapi/message_channel.cc:105: if (!string) { Be ...

9 years, 9 months ago (2011-03-26 17:34:16 UTC) #4

dmichael(do not use this one)

9 years, 9 months ago (2011-03-26 20:57:08 UTC) #5

Thanks for the reviews!  Committed 79497.

http://codereview.chromium.org/6745015/diff/4003/webkit/plugins/ppapi/message...
File webkit/plugins/ppapi/message_channel.cc (right):

http://codereview.chromium.org/6745015/diff/4003/webkit/plugins/ppapi/message...
webkit/plugins/ppapi/message_channel.cc:105: if (!string) {
On 2011/03/26 17:34:16, brettw wrote:
> Be consistent, so no {} for single-line conditionals.
Argh...  old habits die hard.  Thanks.

http://codereview.chromium.org/6745015/diff/4003/webkit/plugins/ppapi/message...
File webkit/plugins/ppapi/message_channel.h (right):

http://codereview.chromium.org/6745015/diff/4003/webkit/plugins/ppapi/message...
webkit/plugins/ppapi/message_channel.h:89: // A function which evaluates the
JavaScript code for onmessage_invoker_ and
On 2011/03/26 17:34:16, brettw wrote:
> Delete "A function which"
Done.

http://codereview.chromium.org/6745015/diff/4003/webkit/plugins/ppapi/message...
webkit/plugins/ppapi/message_channel.h:101: // Hold a
ScopedRunnableMethodFactory so that when this MessageChannel is
On 2011/03/26 17:34:16, brettw wrote:
> I think something shorter like "Ensures pending tasks will not fire after this
> object is destroyed." or something
Done.

Expand Messages | Collapse Messages