Web MIDI: Ask permissions of the content layer always

content/renderer/media/midi_dispatcher.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/media/midi_dispatcher.h"
 
 #include "base/bind.h"
 #include "content/public/common/service_registry.h"
 #include "content/public/renderer/render_frame.h"
 #include "third_party/WebKit/public/platform/WebString.h"

[yhirano, 2016/01/25 11:20:16]

+#include ".../WebMidiOptions.h"

[Takashi Toyoshima, 2016/01/26 07:24:52]

Done.

 #include "third_party/WebKit/public/web/WebMIDIPermissionRequest.h"
 #include "third_party/WebKit/public/web/WebSecurityOrigin.h"
 #include "third_party/WebKit/public/web/WebUserGestureIndicator.h"
 
 using blink::WebMIDIPermissionRequest;
+using blink::WebMIDIOptions;
 using blink::WebSecurityOrigin;
 
 namespace content {
 
 MidiDispatcher::MidiDispatcher(RenderFrame* render_frame)
     : RenderFrameObserver(render_frame) {
 }
 
 MidiDispatcher::~MidiDispatcher() {}
 
-void MidiDispatcher::requestSysexPermission(
-      const WebMIDIPermissionRequest& request) {
+void MidiDispatcher::requestPermission(const WebMIDIPermissionRequest& request,
+                                       const WebMIDIOptions& options) {
+  // TODO(crbug.com/535181): Expose all requests to the content layer.
+  if (!options.sysex)
+    return WebMIDIPermissionRequest(request).setIsAllowed(true);

[mlamouri (slow - plz ping), 2016/01/21 11:50:57]

What about doing:
PermissionName permission_name = options.sysex ? PERMISSION_NAME_MIDI_SYSEX :
PERMISSION_NAME_MIDI;

(The non-sysex midi permission is already part of the permission system so you
really just need to use it here.)

[Takashi Toyoshima, 2016/01/22 04:33:22]

Thank you for jumping in! I thought I will need your help in the next changes.

I tried PERMISSION_NAME_MIDI, but it makes webmidi layout tests fail. Then, I
decided to split changes outside blink for the next patch. But if I can fix it
easily, I'd merge it to this change.

By the way, I'll add one more option 'software' in addition to 'sysex' soon. In
that case, what's the best practice in terms of current psermission service and
content settings's design?
Should I passed an array of permissions via RequestPermissions()?

[Takashi Toyoshima, 2016/01/22 07:30:01]

Some additional notes for this:

- PERMISSION_NAME_MIDI is rejected by default on layout tests, then need to call
testRunner.setPermission.
- setPermission({name:"midi"}) crashes at NOTREACHED in
LayoutTestMessageFiler::OnSetPermission for lack of "midi" handling.
- Even after handling it, setPermission call won't invoke result.onchange of
permissions.query in permissions-helper.js

So, I feel it's reasonable to split the layout test trick change to another CL,
and leave this TODO as is to make review easy. WDYT?

+
   if (!permission_service_.get()) {
     render_frame()->GetServiceRegistry()->ConnectToRemoteService(
         mojo::GetProxy(&permission_service_));
   }
 
   int permission_request_id =
       requests_.Add(new WebMIDIPermissionRequest(request));
 
   permission_service_->RequestPermission(
-      PERMISSION_NAME_MIDI_SYSEX,
-      request.securityOrigin().toString().utf8(),
+      PERMISSION_NAME_MIDI_SYSEX, request.securityOrigin().toString().utf8(),
       blink::WebUserGestureIndicator::isProcessingUserGesture(),
-      base::Bind(&MidiDispatcher::OnSysExPermissionSet,
-                 base::Unretained(this),
+      base::Bind(&MidiDispatcher::OnPermissionSet, base::Unretained(this),
                  permission_request_id));
 }
 
-void MidiDispatcher::cancelSysexPermissionRequest(
+void MidiDispatcher::cancelPermissionRequest(
     const WebMIDIPermissionRequest& request) {
   for (Requests::iterator it(&requests_); !it.IsAtEnd(); it.Advance()) {
     WebMIDIPermissionRequest* value = it.GetCurrentValue();
     if (!value->equals(request))
       continue;
     requests_.Remove(it.GetCurrentKey());
     break;
   }
 }
 
-void MidiDispatcher::OnSysExPermissionSet(int request_id,
-                                          PermissionStatus status) {
+void MidiDispatcher::OnPermissionSet(int request_id, PermissionStatus status) {
   // |request| can be NULL when the request is canceled.
   WebMIDIPermissionRequest* request = requests_.Lookup(request_id);
   if (!request)
     return;
   request->setIsAllowed(status == PERMISSION_STATUS_GRANTED);
   requests_.Remove(request_id);
 }
 
 }  // namespace content