[PresentationAPI] on-session-message handler for binary messages (Blink side).

[PresentationAPI] on-session-message handler for binary messages.

Implements the onmessage event handler of PresentationSession idl to support
ArrayBuffer and Blob data types.

Chromium CL: https://codereview.chromium.org/1194123003/

BUG=459008
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=198654

[USE s.singapati at gmail.com, 2015-06-23 12:41:16]

s.singapati@samsung.com changed reviewers:
+ habib.virji@samsung.com

[Habib Virji, 2015-06-23 13:10:42]

Only two things will suggest considering :
- Some description will be useful. Though it is self explanatory. ArrayBuffer
and Blob changes if they are based on the idl, please add them. Someway to be
clear what your changes are, as bug id you have referenced is closed.
- There are no tests to show your changes, please add them.

[USE s.singapati at gmail.com, 2015-06-23 14:55:36]

On 2015/06/23 13:10:42, Habib Virji wrote:
> Only two things will suggest considering :
> - Some description will be useful. Though it is self explanatory. ArrayBuffer
> and Blob changes if they are based on the idl, please add them. Someway to be
> clear what your changes are, as bug id you have referenced is closed.
> - There are no tests to show your changes, please add them.

Hi, Thanks for the comments. 
Looks like bug id has been closed mistakenly. About tests, currently it is hard
to write Layout tests as there is no tests for base functionality yet. I think
this will come in near future. Manual testing has been done from Presentation
class (implementing onmessage from idl) and by sending messages from
PresentationDispatcher::OnSessionMessagesReceived().

onmessage handler from a TEST page:
 presentation.onmessage = function (event) {
  if (event.data instanceof ArrayBuffer) {
    var bytearray = new Uint8Array(event.data);
    for (var i = 0; i < bytearray.length; i++)
      console.log(bytearray[i]);
    
  } else if (event.data instanceof Blob) {
    var reader = new FileReader();
    reader.onload = function(event) {
      console.log(reader.result);
    }; 
    reader.readAsText(event.data);

  } else if (typeof event.data === "string") {
     console.log(event.data);
  }
 };

[USE s.singapati at gmail.com, 2015-06-23 15:21:40]

s.singapati@samsung.com changed reviewers:
+ haibinlu@chromium.org, imcheng@chromium.org, mfoltz@chromium.org

[USE s.singapati at gmail.com, 2015-06-23 15:21:56]

PTAL.

[USE s.singapati at gmail.com, 2015-06-25 15:32:05]

And, It is bit unclear that the use of "binaryType" attribute of
PresentationSession for onmessage implementation. Otherwise "binaryType" and
getter & setter need to be added.

[USE s.singapati at gmail.com, 2015-06-29 15:12:24]

Any comments on this patch?

[mark a. foltz, 2015-06-29 22:37:04]

On 2015/06/29 at 15:12:24, s.singapati wrote:
> Any comments on this patch?

The binaryType is used to determine how the PresentationSession wishes to
receive binary messages.

http://w3c.github.io/presentation-api/#receiving-a-message-through-presentati...

It seems to me that PresentationSession should have a generic interface to
receive a binary message, then use the session's binaryType to construct a Blob
or ArrayBuffer depending on the value.

[mark a. foltz, 2015-06-29 22:40:17]

On 2015/06/29 at 22:37:04, mark a. foltz wrote:
> On 2015/06/29 at 15:12:24, s.singapati wrote:
> > Any comments on this patch?
> 
> The binaryType is used to determine how the PresentationSession wishes to
receive binary messages.
> 
>
http://w3c.github.io/presentation-api/#receiving-a-message-through-presentati...
> 
> It seems to me that PresentationSession should have a generic interface to
receive a binary message, then use the session's binaryType to construct a Blob
or ArrayBuffer depending on the value.

See, for example, the didReceiveBinaryMessage method in
WebSocketChannelClient.h:

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

And the implementation in DOMWebSocket.cpp:

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

[USE s.singapati at gmail.com, 2015-06-30 14:32:30]

PTAL.

https://codereview.chromium.org/1206513004/diff/20001/Source/modules/presenta...
File Source/modules/presentation/PresentationSession.idl (right):

https://codereview.chromium.org/1206513004/diff/20001/Source/modules/presenta...
Source/modules/presentation/PresentationSession.idl:25: attribute BinaryType
binaryType;
"enum BinaryType" is already defined in WebSocket.idl, so defining again here
causes the build failure (Exception: Enumeration BinaryType has multiple
definitions). Otherwise, defining binaryType for eg., as DOMString is needed to
avoid dependency from WebSocket.idl.

[USE s.singapati at gmail.com, 2015-07-01 07:17:47]

https://codereview.chromium.org/1206513004/diff/20001/Source/modules/presenta...
File Source/modules/presentation/PresentationSession.h (right):

https://codereview.chromium.org/1206513004/diff/20001/Source/modules/presenta...
Source/modules/presentation/PresentationSession.h:124: BinaryType m_binaryType;
Need to give default value to m_binaryType in PresentationSession ctor.

[USE s.singapati at gmail.com, 2015-07-02 15:33:29]

PTAL.

[USE s.singapati at gmail.com, 2015-07-03 07:38:18]

s.singapati@samsung.com changed reviewers:
+ avayvod@chromium.org

[USE s.singapati at gmail.com, 2015-07-06 16:03:49]

Any comments please? Thanks.

[mark a. foltz, 2015-07-06 21:19:45]

Sorry for the delay, the last several days are a holiday weekend in the US.

lgtm after refactoring to be consistent with
https://codereview.chromium.org/1212423003

https://codereview.chromium.org/1206513004/diff/1/Source/modules/presentation...
File Source/modules/presentation/PresentationSession.cpp (right):

https://codereview.chromium.org/1206513004/diff/1/Source/modules/presentation...
Source/modules/presentation/PresentationSession.cpp:246:
memcpy(rawData->mutableData()->data(), data, length);
Can you just call BlobData::appendBytes(data, length)?

https://codereview.chromium.org/1206513004/diff/60001/Source/modules/presenta...
File Source/modules/presentation/Presentation.cpp (right):

https://codereview.chromium.org/1206513004/diff/60001/Source/modules/presenta...
Source/modules/presentation/Presentation.cpp:179:
PresentationSession::dispose(sessionClient);
This method will be going away soon; see
https://codereview.chromium.org/1212423003.  

Please replace with 'delete sessionClient'

https://codereview.chromium.org/1206513004/diff/60001/Source/modules/presenta...
File Source/modules/presentation/PresentationController.cpp (right):

https://codereview.chromium.org/1206513004/diff/60001/Source/modules/presenta...
Source/modules/presentation/PresentationController.cpp:110:
PresentationSession::dispose(sessionClient);
Please refactor to use the form suggested in
https://codereview.chromium.org/1212423003

[USE s.singapati at gmail.com, 2015-07-07 15:00:33]

https://codereview.chromium.org/1206513004/diff/1/Source/modules/presentation...
File Source/modules/presentation/PresentationSession.cpp (right):

https://codereview.chromium.org/1206513004/diff/1/Source/modules/presentation...
Source/modules/presentation/PresentationSession.cpp:246:
memcpy(rawData->mutableData()->data(), data, length);
On 2015/07/06 21:19:44, mark a. foltz wrote:
> Can you just call BlobData::appendBytes(data, length)?

Done. Manual tested.

https://codereview.chromium.org/1206513004/diff/60001/Source/modules/presenta...
File Source/modules/presentation/Presentation.cpp (right):

https://codereview.chromium.org/1206513004/diff/60001/Source/modules/presenta...
Source/modules/presentation/Presentation.cpp:179:
PresentationSession::dispose(sessionClient);
On 2015/07/06 21:19:45, mark a. foltz wrote:
> This method will be going away soon; see
> https://codereview.chromium.org/1212423003.  
> 
> Please replace with 'delete sessionClient'

Done.

https://codereview.chromium.org/1206513004/diff/60001/Source/modules/presenta...
File Source/modules/presentation/PresentationController.cpp (right):

https://codereview.chromium.org/1206513004/diff/60001/Source/modules/presenta...
Source/modules/presentation/PresentationController.cpp:110:
PresentationSession::dispose(sessionClient);
On 2015/07/06 21:19:45, mark a. foltz wrote:
> Please refactor to use the form suggested in
> https://codereview.chromium.org/1212423003

Done.

[USE s.singapati at gmail.com, 2015-07-07 15:00:51]

The CQ bit was checked by s.singapati@samsung.com

[USE s.singapati at gmail.com, 2015-07-07 15:00:51]

The patchset sent to the CQ was uploaded after l-g-t-m from mfoltz@chromium.org
Link to the patchset: https://codereview.chromium.org/1206513004/#ps80001
(title: "refactor deleting sessionClient and using BlobData::appendBytes")

[commit-bot: I haz the power, 2015-07-07 15:01:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1206513004/80001

[USE s.singapati at gmail.com, 2015-07-07 15:13:22]

The CQ bit was unchecked by s.singapati@samsung.com

[USE s.singapati at gmail.com, 2015-07-07 15:37:06]

The CQ bit was checked by s.singapati@samsung.com

[commit-bot: I haz the power, 2015-07-07 15:37:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1206513004/80001

[commit-bot: I haz the power, 2015-07-07 15:43:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-07-07 15:43:53]

Try jobs failed on following builders:
  blink_presubmit on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/3...)

[USE s.singapati at gmail.com, 2015-07-07 16:37:22]

The CQ bit was checked by s.singapati@samsung.com

[USE s.singapati at gmail.com, 2015-07-07 16:37:22]

The patchset sent to the CQ was uploaded after l-g-t-m from mfoltz@chromium.org
Link to the patchset: https://codereview.chromium.org/1206513004/#ps100001
(title: "merge")

[commit-bot: I haz the power, 2015-07-07 16:37:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1206513004/100001

[commit-bot: I haz the power, 2015-07-07 17:02:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-07-07 17:03:01]

Try jobs failed on following builders:
  win_blink_compile_dbg on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/win_blink_compile_dbg/bu...)

[USE s.singapati at gmail.com, 2015-07-08 13:07:47]

The CQ bit was checked by s.singapati@samsung.com to run a CQ dry run

[USE s.singapati at gmail.com, 2015-07-08 13:07:48]

The patchset sent to the CQ was uploaded after l-g-t-m from mfoltz@chromium.org
Link to the patchset: https://codereview.chromium.org/1206513004/#ps120001
(title: "add typedef uint8_t for WIN32")

[commit-bot: I haz the power, 2015-07-08 13:08:04]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1206513004/120001

[USE s.singapati at gmail.com, 2015-07-08 13:46:09]

s.singapati@samsung.com changed reviewers:
+ jochen@chromium.org

[USE s.singapati at gmail.com, 2015-07-08 13:52:21]

Hello jochen, Could you please review public/platform/WebCommon.h? Thanks.

Needed to fix builds on windows.
e:\b\build\slave\win_layout\build\src\third_party\webkit\public\platform\modules\presentation\webpresentationcontroller.h(32)
: error C4430: missing type specifier - int assumed. Note: C++ does not support
default-int
e:\b\build\slave\win_layout\build\src\third_party\webkit\public\platform\modules\presentation\webpresentationcontroller.h(32)
: error C2143: syntax error : missing ',' before '*'

[Peter Beverloo, 2015-07-08 14:00:22]

peter@chromium.org changed reviewers:
+ peter@chromium.org

[Peter Beverloo, 2015-07-08 14:00:22]

https://codereview.chromium.org/1206513004/diff/120001/public/platform/WebCom...
File public/platform/WebCommon.h (right):

https://codereview.chromium.org/1206513004/diff/120001/public/platform/WebCom...
public/platform/WebCommon.h:88: #if defined(WIN32)
Removing lines 88-97 and 99 (in a separate patch) would be a better solution -
stdint.h is commonly available now.

[commit-bot: I haz the power, 2015-07-08 14:27:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-07-08 14:27:19]

Dry run: This issue passed the CQ dry run.

[USE s.singapati at gmail.com, 2015-07-08 14:39:56]

Now this CL waits on a new CL https://codereview.chromium.org/1223843017/

https://codereview.chromium.org/1206513004/diff/120001/public/platform/WebCom...
File public/platform/WebCommon.h (right):

https://codereview.chromium.org/1206513004/diff/120001/public/platform/WebCom...
public/platform/WebCommon.h:88: #if defined(WIN32)
On 2015/07/08 14:00:22, Peter Beverloo wrote:
> Removing lines 88-97 and 99 (in a separate patch) would be a better solution -
> stdint.h is commonly available now.

Done. New CL: https://codereview.chromium.org/1223843017

[USE s.singapati at gmail.com, 2015-07-10 02:33:42]

The CQ bit was checked by s.singapati@samsung.com

[USE s.singapati at gmail.com, 2015-07-10 02:33:43]

The patchset sent to the CQ was uploaded after l-g-t-m from mfoltz@chromium.org
Link to the patchset: https://codereview.chromium.org/1206513004/#ps140001
(title: "Revert "add typedef uint8_t for WIN32"")

[commit-bot: I haz the power, 2015-07-10 02:34:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1206513004/140001

[commit-bot: I haz the power, 2015-07-10 03:43:14]

Committed patchset #8 (id:140001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=198654