Reland: Web MIDI: use mojom::blink::PermissionService directly to ask permission

Reland: Web MIDI: use mojom::blink::PermissionService directly to ask permission

Changes from the first attempt:

- AwPermissionManager::RequestPermissions() was implemented for
  Android WebView in a separate CL.
- Workarounds to avoid touching NOTIMPLEMENTED() was removed

Original description follows:

SystemWebViewShellLayoutTest failed because of the same reason with
other modified tests in this CL. Tests are modified to pass, but now
it does not cover {sysex:true} case that needs
AwPermissionManager::RequestPermissions implementation.
It will be implemented in the next CL soon.

Original description follows:
Web MIDI asked permissions via public/web interfaces.
But now that PermissionService is available in Blink,
use the service to ask permissions.

This migration makes it possible to remove all MIDI
related public/web interfaces.

BUG=582328
Committed: https://crrev.com/15c1a1ffd1f4c620b89191209dc76ef5557dd8fc
Cr-Commit-Position: refs/heads/master@{#404136}

TEST=${OUT}/bin/run_system_webview_shell_layout_test_apk # with a built SystemWebViewGoogle.apk
TEST=git cl try

Committed: https://crrev.com/8c21bb977b580edb009eb2fe0d3f511fd5ca42f4
Cr-Commit-Position: refs/heads/master@{#408621}

[Takashi Toyoshima, 2016-07-01 12:41:12]

Description was changed from

==========
Web MIDI: use mojom::blink::PermissionService directly to ask permission

BUG=582328
==========

to

==========
Web MIDI: use mojom::blink::PermissionService directly to ask permission

Web MIDI asked permissions via public/web interfaces.
But now that PermissionService is available in Blink,
use the service to ask permissions.

This migration makes it possible to remove all MIDI
related public/web interfaces.

BUG=582328
==========

[Takashi Toyoshima, 2016-07-01 12:41:12]

toyoshim@chromium.org changed reviewers:
+ yukishiino@chromium.org

[Takashi Toyoshima, 2016-07-01 12:44:51]

Hi Shiino-san, can you take a look, or help me to find a reviewer for this
change?

https://codereview.chromium.org/2116763002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webmidi/MIDIAccessInitializer.cpp
(right):

https://codereview.chromium.org/2116763002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webmidi/MIDIAccessInitializer.cpp:1: //
Copyright 2014 The Chromium Authors. All rights reserved.
Major logic changes are in this file.

https://codereview.chromium.org/2116763002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webmidi/MIDIAccessInitializer.cpp:54:
permissions.PassStorage(),
I don't have enough confidence on this mojo:WTFArray usage.

[Yuki, 2016-07-04 06:42:36]

yukishiino@chromium.org changed reviewers:
+ sammc@chromium.org

[Yuki, 2016-07-04 06:42:36]

+R=sammc@,

Sam, could you review this CL as a Mojo expert?

[Sam McNally, 2016-07-05 01:17:55]

https://codereview.chromium.org/2116763002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webmidi/MIDIAccessInitializer.cpp
(right):

https://codereview.chromium.org/2116763002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webmidi/MIDIAccessInitializer.cpp:54:
permissions.PassStorage(),
On 2016/07/01 12:44:51, toyoshim wrote:
> I don't have enough confidence on this mojo:WTFArray usage.

WTFArray is "move" constructible from a WTF::Vector
(https://cs.chromium.org/chromium/src/mojo/public/cpp/bindings/wtf_array.h?l=41),
so you can just use a Vector and move it into the RequestPermissions() call.

https://codereview.chromium.org/2116763002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webmidi/MIDIAccessInitializer.cpp:123: for
(size_t i = 0; i < status.size(); ++i) {
I think this would be nicer as a foreach loop. WTFArray doesn't seem to support
begin()/end() but you can get the Vector out with WTFArray::storage().

[Takashi Toyoshima, 2016-07-05 08:03:19]

https://codereview.chromium.org/2116763002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webmidi/MIDIAccessInitializer.cpp
(right):

https://codereview.chromium.org/2116763002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webmidi/MIDIAccessInitializer.cpp:54:
permissions.PassStorage(),
On 2016/07/05 01:17:54, Sam McNally wrote:
> On 2016/07/01 12:44:51, toyoshim wrote:
> > I don't have enough confidence on this mojo:WTFArray usage.
> 
> WTFArray is "move" constructible from a WTF::Vector
>
(https://cs.chromium.org/chromium/src/mojo/public/cpp/bindings/wtf_array.h?l=41),
> so you can just use a Vector and move it into the RequestPermissions() call.

Done.

https://codereview.chromium.org/2116763002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webmidi/MIDIAccessInitializer.cpp:123: for
(size_t i = 0; i < status.size(); ++i) {
On 2016/07/05 01:17:54, Sam McNally wrote:
> I think this would be nicer as a foreach loop. WTFArray doesn't seem to
support
> begin()/end() but you can get the Vector out with WTFArray::storage().

Done.

[Takashi Toyoshima, 2016-07-05 09:53:44]

cc: Lalit who wrote
android_webview/javatests/src/org/chromium/android_webview/test/AwPermissionManagerTest.java

org.chromium.android_webview.test.AwPermissionManagerTest#testRequestMultiple is
still failing.
Lalit, can you help me to debug this test?

In this change, requestMIDIAccess({sysex:true}) is changed to ask permission for
both MIDI and MIDI_SYSEX in tests.
I think this behavior change breaks the test. This behavior change was applied
for production before, but is slightly different in tests.

[Takashi Toyoshima, 2016-07-06 14:10:29]

I learned how to run AndroidWebViewTest on the real device, and find what
happens in the test.

In AndroidWebView, RequestPermissions isn't implemented.

android_webview/browser/aw_permission_manager.cc, in
AwPermissionManager::RequestPermissions;
NOTIMPLEMENTED() << "RequestPermissions has not been implemented in WebView"; 

I will implement this.

[Takashi Toyoshima, 2016-07-06 14:50:33]

In this patch, I just added a workaround for the AndroidWebView test because it
would be better to split to another CL to get better reviews on mojo, and
WebView respectively.

[Takashi Toyoshima, 2016-07-06 17:12:26]

FYI, WebView side change is here, https://codereview.chromium.org/2125893002/. I
will ask code review once this CL is submitted.

[Takashi Toyoshima, 2016-07-07 03:49:00]

Hum... Android tests fail randomly, but it looks not related to this CL.
Sam, can you take another look?

[Sam McNally, 2016-07-07 04:47:10]

LGTM

https://codereview.chromium.org/2116763002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webmidi/MIDIAccessInitializer.cpp
(right):

https://codereview.chromium.org/2116763002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webmidi/MIDIAccessInitializer.cpp:25: 
using mojom::blink::PermissionName;
using mojom::blink::PermissionStatus;

That should allow you to shorten some of these long enum type and value names.

https://codereview.chromium.org/2116763002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webmidi/MIDIAccessInitializer.cpp:55:
mojo::WTFArray<mojom::blink::PermissionName>(std::move(permissions)),
Can you use an initializer list to construct the WTFArray?

[Takashi Toyoshima, 2016-07-07 05:28:26]

https://codereview.chromium.org/2116763002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webmidi/MIDIAccessInitializer.cpp
(right):

https://codereview.chromium.org/2116763002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webmidi/MIDIAccessInitializer.cpp:25: 
On 2016/07/07 04:47:09, Sam McNally wrote:
> using mojom::blink::PermissionName;
> using mojom::blink::PermissionStatus;
> 
> That should allow you to shorten some of these long enum type and value names.

Done.

https://codereview.chromium.org/2116763002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webmidi/MIDIAccessInitializer.cpp:55:
mojo::WTFArray<mojom::blink::PermissionName>(std::move(permissions)),
WTFArray does not seem to have a constructor for initializer_list<T> like
Vector. So, I change this to use an initializer list to construct Vector<> and
move it to WTFArray<>.

[Takashi Toyoshima, 2016-07-07 05:29:11]

toyoshim@chromium.org changed reviewers:
+ kinuko@chromium.org

[Takashi Toyoshima, 2016-07-07 05:29:47]

toyoshim@chromium.org changed reviewers:
+ tkent@chromium.org

[Takashi Toyoshima, 2016-07-07 05:30:20]

toyoshim@chromium.org changed reviewers:
+ torne@chromium.org

[Takashi Toyoshima, 2016-07-07 05:31:48]

Add some OWNERS.

kinuko for content
tkent for WebKit API
torne for Android WebView

Please take a look.

[tkent, 2016-07-07 05:34:53]

third_party/WebKit/public and third_party/WebKit/Source/web lgtm

[kinuko, 2016-07-07 06:07:29]

content/ lgtm

[haraken, 2016-07-07 08:21:57]

haraken@chromium.org changed reviewers:
+ esprehn@chromium.org, haraken@chromium.org

[haraken, 2016-07-07 08:21:58]

The mojo interaction LGTM. This looks like a great simplification!

[Takashi Toyoshima, 2016-07-07 09:54:16]

toyoshim@chromium.org changed reviewers:
+ tobiasjs@chromium.org

[Takashi Toyoshima, 2016-07-07 09:55:30]

This is a nice takeaway from the last BlinkOn :)

+tobiasjs for WebView because torne is ooo.

[Tobias Sargeant, 2016-07-07 10:50:29]

On 2016/07/07 09:55:30, toyoshim wrote:
> This is a nice takeaway from the last BlinkOn :)
> 
> +tobiasjs for WebView because torne is ooo.

android_webview/ LGTM but maybe add a comment to explain/TODO to resolve the
workaround?

[Takashi Toyoshima, 2016-07-07 11:03:11]

https://codereview.chromium.org/2116763002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webmidi/MIDIAccessInitializer.cpp
(right):

https://codereview.chromium.org/2116763002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webmidi/MIDIAccessInitializer.cpp:60: //
TODO(toyoshim): Merge this MIDI only code path to the other one for
>Tobias

Here is TODO.
AwPermissionManagerTest.java's change is not a workaround, but needed even after
a right fix. This is because if sysex is requested, two permissions, MIDI and
MIDI_SYSEX, are requested in the new implementation. Before this change, MIDI
permission is short-circuited to be granted without client decision. That's why
{sysex: true} is specified in the test. But after this change, it isn't needed,
and should not have to work correctly.

[Tobias Sargeant, 2016-07-07 11:06:52]

On 2016/07/07 11:03:11, toyoshim wrote:
>
https://codereview.chromium.org/2116763002/diff/120001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/modules/webmidi/MIDIAccessInitializer.cpp
> (right):
> 
>
https://codereview.chromium.org/2116763002/diff/120001/third_party/WebKit/Sou...
> third_party/WebKit/Source/modules/webmidi/MIDIAccessInitializer.cpp:60: //
> TODO(toyoshim): Merge this MIDI only code path to the other one for
> >Tobias
> 
> Here is TODO.
> AwPermissionManagerTest.java's change is not a workaround, but needed even
after
> a right fix. This is because if sysex is requested, two permissions, MIDI and
> MIDI_SYSEX, are requested in the new implementation. Before this change, MIDI
> permission is short-circuited to be granted without client decision. That's
why
> {sysex: true} is specified in the test. But after this change, it isn't
needed,
> and should not have to work correctly.

Great. Thanks for the explanation.

[Takashi Toyoshima, 2016-07-07 11:08:38]

The CQ bit was checked by toyoshim@chromium.org

[Takashi Toyoshima, 2016-07-07 11:08:39]

The patchset sent to the CQ was uploaded after l-g-t-m from sammc@chromium.org
Link to the patchset: https://codereview.chromium.org/2116763002/#ps120001
(title: "review #13")

[commit-bot: I haz the power, 2016-07-07 11:08:50]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-07 12:33:01]

Description was changed from

==========
Web MIDI: use mojom::blink::PermissionService directly to ask permission

Web MIDI asked permissions via public/web interfaces.
But now that PermissionService is available in Blink,
use the service to ask permissions.

This migration makes it possible to remove all MIDI
related public/web interfaces.

BUG=582328
==========

to

==========
Web MIDI: use mojom::blink::PermissionService directly to ask permission

Web MIDI asked permissions via public/web interfaces.
But now that PermissionService is available in Blink,
use the service to ask permissions.

This migration makes it possible to remove all MIDI
related public/web interfaces.

BUG=582328
==========

[commit-bot: I haz the power, 2016-07-07 12:33:02]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-07-07 12:33:10]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-07 12:35:40]

Description was changed from

==========
Web MIDI: use mojom::blink::PermissionService directly to ask permission

Web MIDI asked permissions via public/web interfaces.
But now that PermissionService is available in Blink,
use the service to ask permissions.

This migration makes it possible to remove all MIDI
related public/web interfaces.

BUG=582328
==========

to

==========
Web MIDI: use mojom::blink::PermissionService directly to ask permission

Web MIDI asked permissions via public/web interfaces.
But now that PermissionService is available in Blink,
use the service to ask permissions.

This migration makes it possible to remove all MIDI
related public/web interfaces.

BUG=582328

Committed: https://crrev.com/15c1a1ffd1f4c620b89191209dc76ef5557dd8fc
Cr-Commit-Position: refs/heads/master@{#404136}
==========

[commit-bot: I haz the power, 2016-07-07 12:35:41]

Patchset 7 (id:??) landed as
https://crrev.com/15c1a1ffd1f4c620b89191209dc76ef5557dd8fc
Cr-Commit-Position: refs/heads/master@{#404136}

[hush (inactive), 2016-07-07 18:04:13]

A revert of this CL (patchset #7 id:120001) has been created in
https://codereview.chromium.org/2129913005/ by hush@chromium.org.

The reason for reverting is: This CL is suspected to break
org.chromium.webview_shell.test.WebViewLayoutTest#testRequestMIDIAccessDenyPermission
org.chromium.webview_shell.test.WebViewLayoutTest#testRequestMIDIAccess

https://uberchromegw.corp.google.com/i/chromium.android/builders/Android%20We....

[Takashi Toyoshima, 2016-07-08 04:05:24]

Description was changed from

==========
Web MIDI: use mojom::blink::PermissionService directly to ask permission

Web MIDI asked permissions via public/web interfaces.
But now that PermissionService is available in Blink,
use the service to ask permissions.

This migration makes it possible to remove all MIDI
related public/web interfaces.

BUG=582328

Committed: https://crrev.com/15c1a1ffd1f4c620b89191209dc76ef5557dd8fc
Cr-Commit-Position: refs/heads/master@{#404136}
==========

to

==========
Web MIDI: use mojom::blink::PermissionService directly to ask permission

Web MIDI asked permissions via public/web interfaces.
But now that PermissionService is available in Blink,
use the service to ask permissions.

This migration makes it possible to remove all MIDI
related public/web interfaces.

BUG=582328

Committed: https://crrev.com/15c1a1ffd1f4c620b89191209dc76ef5557dd8fc
Cr-Commit-Position: refs/heads/master@{#404136}
==========

[Takashi Toyoshima, 2016-07-08 04:10:45]

reopen to fix broken tests and to submit again.

[Takashi Toyoshima, 2016-07-08 07:22:48]

Description was changed from

==========
Web MIDI: use mojom::blink::PermissionService directly to ask permission

Web MIDI asked permissions via public/web interfaces.
But now that PermissionService is available in Blink,
use the service to ask permissions.

This migration makes it possible to remove all MIDI
related public/web interfaces.

BUG=582328

Committed: https://crrev.com/15c1a1ffd1f4c620b89191209dc76ef5557dd8fc
Cr-Commit-Position: refs/heads/master@{#404136}
==========

to

==========
Reland: Web MIDI: use mojom::blink::PermissionService directly to ask permission

SystemWebViewShellLayoutTest failed because of the same reason with
other modified tests in this CL. Tests are modified to pass, but now
it does not cover {sysex:true} case that needs
AwPermissionManager::RequestPermissions implementation.
It will be implemented in the next CL soon.

Original description follows:
Web MIDI asked permissions via public/web interfaces.
But now that PermissionService is available in Blink,
use the service to ask permissions.

This migration makes it possible to remove all MIDI
related public/web interfaces.

BUG=582328

Committed: https://crrev.com/15c1a1ffd1f4c620b89191209dc76ef5557dd8fc
Cr-Commit-Position: refs/heads/master@{#404136}
==========

[Takashi Toyoshima, 2016-07-08 07:26:53]

Tobias, I updated WebView code with workaround.
Right fix to implement RequestPermissions is ready for review and is expected to
be submitted soon; https://codereview.chromium.org/2125893002/

https://codereview.chromium.org/2116763002/diff/140001/android_webview/tools/...
File
android_webview/tools/system_webview_shell/layout_tests/src/org/chromium/webview_shell/test/WebViewLayoutTest.java
(right):

https://codereview.chromium.org/2116763002/diff/140001/android_webview/tools/...
android_webview/tools/system_webview_shell/layout_tests/src/org/chromium/webview_shell/test/WebViewLayoutTest.java:215:
"blink-apis/webmidi/requestmidiaccess-permission-denied-expected.txt");
fix an existing mistake.

https://codereview.chromium.org/2116763002/diff/140001/android_webview/tools/...
File
android_webview/tools/system_webview_shell/test/data/blink-apis/webmidi/requestmidiaccess.html
(right):

https://codereview.chromium.org/2116763002/diff/140001/android_webview/tools/...
android_webview/tools/system_webview_shell/test/data/blink-apis/webmidi/requestmidiaccess.html:21:
// TODO(toyoshim): Add another version to request with {sysex:true}.
This is a workaround. I will add {sysex:true} case again in the next CL.

[Takashi Toyoshima, 2016-07-11 05:34:36]

ping: torne and tobias

[Tobias Sargeant, 2016-07-11 15:26:12]

On 2016/07/11 05:34:36, toyoshim wrote:
> ping: torne and tobias

I ran system_webview_shell_layout_test_apk locally, and there were still
failures.

bin/run_system_webview_shell_layout_test_apk -f
'org.chromium.webview_shell.test.WebViewLayoutTest#testRequestMIDIAccessDenyPermission'
...
junit.framework.ComparisonFailure: expected:<...cess API in WebView
[]PASS: requestMIDIAcc...> but was:<...cess API in WebView
[onPermissionRequest: android.webkit.resource.MIDI_SYSEX
request denied
]PASS: requestMIDIAcc...>

[Takashi Toyoshima, 2016-07-12 05:45:37]

Did you replace SystemWebViewGoogle.apk with one of this patch applied?
Your results looks like one of patched test running with non-patched
SystemWebView.

This is the instruction I followed;
https://www.chromium.org/developers/how-tos/build-instructions-android-webview.
I built system_webview_apk with an internal checkout, and replaced SystemWebView
with built SystemWebViewGoogle.apk after uninstalling and removing default
WebViews. Then, run run_system_webview_shell_layout_test_apk.

I want to double-check this now, but I do not have a device that runs
non-production Android build that allows replacing SystemWebView today.

[Takashi Toyoshima, 2016-07-12 07:19:42]

I flashed a userdebug build to a Nexus 4, and followed steps below.

% adb uninstall com.google.android.webview
Failure [DELETE_FAILED_INTERNAL_ERROR]
% adb uninstall com.android.webview
Failure [DELETE_FAILED_INTERNAL_ERROR]
% adb root
% adb remount
remount succeeded
% adb shell stop
% adb shell rm -rf /system/app/webview /system/app/WebViewGoogle
% adb shell start
% ninja -C out/gn -j300 system_webview_apk
(snip)
% adb install -r -d out/gn/apks/SystemWebViewGoogle.apk
3943 KB/s (39185503 bytes in 9.703s)
        pkg: /data/local/tmp/SystemWebViewGoogle.apk
Success
% ninja -C out/gn -j300 system_webview_shell_layout_test_apk
(snip)
% ./out/gn/bin/run_system_webview_shell_layout_test_apk -f
org.chromium.webview_shell.test.WebViewLayoutTest#testRequestMIDIAccessDenyPermission
W    0.056s Main  Unable to find package info for
org.chromium.webview_shell.test
E    2.209s individual_device_set_up(00a45a0655552205)  Couldn't set debug app:
no package info
E    2.412s individual_device_set_up(00a45a0655552205)  Couldn't set flags: no
package info
C    6.475s Main 
********************************************************************************
C    6.475s Main  Summary
C    6.475s Main 
********************************************************************************
C    6.475s Main  [==========] 1 test ran.
C    6.475s Main  [  PASSED  ] 1 test.
C    6.476s Main 
********************************************************************************

When I tried these steps with Nexus5 yesterday, I didn't see such errors on 'adb
uninstall' steps. So, I guess this errors was because of running on clean
install device, and I can ignore here.

[Takashi Toyoshima, 2016-07-12 07:25:32]

Ah, build target and installed apk was unmatched in the log.
But I built both system_webview_apk and system_webview_google_apk, and installed
both SystemWebView.apk and SystemWebViewGoogle.apk before ran the test just in
case.

[Tobias Sargeant, 2016-07-12 13:45:53]

PS8 is missing a definition for createBaseCallback.

[Tobias Sargeant, 2016-07-12 13:57:53]

On 2016/07/12 13:45:53, Tobias Sargeant wrote:
> PS8 is missing a definition for createBaseCallback.

Looks like it was removed in 98807a41bbabd6ece33e2e433b38447f3c092189

[Takashi Toyoshima, 2016-07-13 07:04:45]

PS9 is one simply rebased to compile.

But probably we prefer to revisit this CL without workaronuds after submitting
https://codereview.chromium.org/2143573002/

[Takashi Toyoshima, 2016-07-29 08:55:55]

Description was changed from

==========
Reland: Web MIDI: use mojom::blink::PermissionService directly to ask permission

SystemWebViewShellLayoutTest failed because of the same reason with
other modified tests in this CL. Tests are modified to pass, but now
it does not cover {sysex:true} case that needs
AwPermissionManager::RequestPermissions implementation.
It will be implemented in the next CL soon.

Original description follows:
Web MIDI asked permissions via public/web interfaces.
But now that PermissionService is available in Blink,
use the service to ask permissions.

This migration makes it possible to remove all MIDI
related public/web interfaces.

BUG=582328

Committed: https://crrev.com/15c1a1ffd1f4c620b89191209dc76ef5557dd8fc
Cr-Commit-Position: refs/heads/master@{#404136}
==========

to

==========
Reland: Web MIDI: use mojom::blink::PermissionService directly to ask permission

Changes from the first attempt:

- AwPermissionManager::RequestPermissions() was implemented for
  Android WebView in a separate CL.
- Workarounds to avoid touching NOTIMPLEMENTED() was removed

Original description follows:

SystemWebViewShellLayoutTest failed because of the same reason with
other modified tests in this CL. Tests are modified to pass, but now
it does not cover {sysex:true} case that needs
AwPermissionManager::RequestPermissions implementation.
It will be implemented in the next CL soon.

Original description follows:
Web MIDI asked permissions via public/web interfaces.
But now that PermissionService is available in Blink,
use the service to ask permissions.

This migration makes it possible to remove all MIDI
related public/web interfaces.

BUG=582328

Committed: https://crrev.com/15c1a1ffd1f4c620b89191209dc76ef5557dd8fc
Cr-Commit-Position: refs/heads/master@{#404136}
==========

[Takashi Toyoshima, 2016-07-29 11:06:15]

Description was changed from

==========
Reland: Web MIDI: use mojom::blink::PermissionService directly to ask permission

Changes from the first attempt:

- AwPermissionManager::RequestPermissions() was implemented for
  Android WebView in a separate CL.
- Workarounds to avoid touching NOTIMPLEMENTED() was removed

Original description follows:

SystemWebViewShellLayoutTest failed because of the same reason with
other modified tests in this CL. Tests are modified to pass, but now
it does not cover {sysex:true} case that needs
AwPermissionManager::RequestPermissions implementation.
It will be implemented in the next CL soon.

Original description follows:
Web MIDI asked permissions via public/web interfaces.
But now that PermissionService is available in Blink,
use the service to ask permissions.

This migration makes it possible to remove all MIDI
related public/web interfaces.

BUG=582328

Committed: https://crrev.com/15c1a1ffd1f4c620b89191209dc76ef5557dd8fc
Cr-Commit-Position: refs/heads/master@{#404136}
==========

to

==========
Reland: Web MIDI: use mojom::blink::PermissionService directly to ask permission

Changes from the first attempt:

- AwPermissionManager::RequestPermissions() was implemented for
  Android WebView in a separate CL.
- Workarounds to avoid touching NOTIMPLEMENTED() was removed

Original description follows:

SystemWebViewShellLayoutTest failed because of the same reason with
other modified tests in this CL. Tests are modified to pass, but now
it does not cover {sysex:true} case that needs
AwPermissionManager::RequestPermissions implementation.
It will be implemented in the next CL soon.

Original description follows:
Web MIDI asked permissions via public/web interfaces.
But now that PermissionService is available in Blink,
use the service to ask permissions.

This migration makes it possible to remove all MIDI
related public/web interfaces.

BUG=582328

Committed: https://crrev.com/15c1a1ffd1f4c620b89191209dc76ef5557dd8fc
Cr-Commit-Position: refs/heads/master@{#404136}

TEST=${OUT}/bin/run_system_webview_shell_layout_test_apk # with a built
SystemWebViewGoogle.apk
TEST=git cl try
==========

[Takashi Toyoshima, 2016-07-29 11:06:33]

The CQ bit was checked by toyoshim@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-29 11:06:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Takashi Toyoshima, 2016-07-29 11:11:23]

Tobias: Could you take another look for reland this CL?

Now all workarounds are removed because we now have
AwPermissionManager::RequestPermissions() implementation.
To add one more test to WebViewLayoutTest, I rename the old
testRequestMIDIAccess() to testRequestMIDIAccessWithSysex(), and add a variant
as testRequestMIDIAccess().
I also rename requestmidiaccess-permission-denied-expected.html to
requestmidiaccess-permission-denied-expected.txt, this name looks just a
mistake.

[Tobias Sargeant, 2016-07-29 11:17:14]

On 2016/07/29 11:11:23, toyoshim wrote:
> Tobias: Could you take another look for reland this CL?
> 
> Now all workarounds are removed because we now have
> AwPermissionManager::RequestPermissions() implementation.
> To add one more test to WebViewLayoutTest, I rename the old
> testRequestMIDIAccess() to testRequestMIDIAccessWithSysex(), and add a variant
> as testRequestMIDIAccess().
> I also rename requestmidiaccess-permission-denied-expected.html to
> requestmidiaccess-permission-denied-expected.txt, this name looks just a
> mistake.

LGTM to reland.

[commit-bot: I haz the power, 2016-07-29 12:18:49]

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

[commit-bot: I haz the power, 2016-07-29 12:18:51]

Dry run: This issue passed the CQ dry run.

[Takashi Toyoshima, 2016-07-29 12:22:58]

The CQ bit was checked by toyoshim@chromium.org

[Takashi Toyoshima, 2016-07-29 12:22:59]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org,
kinuko@chromium.org, sammc@chromium.org, haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/2116763002/#ps180001
(title: "for second attempt")

[commit-bot: I haz the power, 2016-07-29 12:23:10]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-29 12:27:20]

Description was changed from

==========
Reland: Web MIDI: use mojom::blink::PermissionService directly to ask permission

Changes from the first attempt:

- AwPermissionManager::RequestPermissions() was implemented for
  Android WebView in a separate CL.
- Workarounds to avoid touching NOTIMPLEMENTED() was removed

Original description follows:

SystemWebViewShellLayoutTest failed because of the same reason with
other modified tests in this CL. Tests are modified to pass, but now
it does not cover {sysex:true} case that needs
AwPermissionManager::RequestPermissions implementation.
It will be implemented in the next CL soon.

Original description follows:
Web MIDI asked permissions via public/web interfaces.
But now that PermissionService is available in Blink,
use the service to ask permissions.

This migration makes it possible to remove all MIDI
related public/web interfaces.

BUG=582328

Committed: https://crrev.com/15c1a1ffd1f4c620b89191209dc76ef5557dd8fc
Cr-Commit-Position: refs/heads/master@{#404136}

TEST=${OUT}/bin/run_system_webview_shell_layout_test_apk # with a built
SystemWebViewGoogle.apk
TEST=git cl try
==========

to

==========
Reland: Web MIDI: use mojom::blink::PermissionService directly to ask permission

Changes from the first attempt:

- AwPermissionManager::RequestPermissions() was implemented for
  Android WebView in a separate CL.
- Workarounds to avoid touching NOTIMPLEMENTED() was removed

Original description follows:

SystemWebViewShellLayoutTest failed because of the same reason with
other modified tests in this CL. Tests are modified to pass, but now
it does not cover {sysex:true} case that needs
AwPermissionManager::RequestPermissions implementation.
It will be implemented in the next CL soon.

Original description follows:
Web MIDI asked permissions via public/web interfaces.
But now that PermissionService is available in Blink,
use the service to ask permissions.

This migration makes it possible to remove all MIDI
related public/web interfaces.

BUG=582328

Committed: https://crrev.com/15c1a1ffd1f4c620b89191209dc76ef5557dd8fc
Cr-Commit-Position: refs/heads/master@{#404136}

TEST=${OUT}/bin/run_system_webview_shell_layout_test_apk # with a built
SystemWebViewGoogle.apk
TEST=git cl try
==========

[commit-bot: I haz the power, 2016-07-29 12:27:22]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-07-29 12:28:35]

Description was changed from

==========
Reland: Web MIDI: use mojom::blink::PermissionService directly to ask permission

Changes from the first attempt:

- AwPermissionManager::RequestPermissions() was implemented for
  Android WebView in a separate CL.
- Workarounds to avoid touching NOTIMPLEMENTED() was removed

Original description follows:

SystemWebViewShellLayoutTest failed because of the same reason with
other modified tests in this CL. Tests are modified to pass, but now
it does not cover {sysex:true} case that needs
AwPermissionManager::RequestPermissions implementation.
It will be implemented in the next CL soon.

Original description follows:
Web MIDI asked permissions via public/web interfaces.
But now that PermissionService is available in Blink,
use the service to ask permissions.

This migration makes it possible to remove all MIDI
related public/web interfaces.

BUG=582328

Committed: https://crrev.com/15c1a1ffd1f4c620b89191209dc76ef5557dd8fc
Cr-Commit-Position: refs/heads/master@{#404136}

TEST=${OUT}/bin/run_system_webview_shell_layout_test_apk # with a built
SystemWebViewGoogle.apk
TEST=git cl try
==========

to

==========
Reland: Web MIDI: use mojom::blink::PermissionService directly to ask permission

Changes from the first attempt:

- AwPermissionManager::RequestPermissions() was implemented for
  Android WebView in a separate CL.
- Workarounds to avoid touching NOTIMPLEMENTED() was removed

Original description follows:

SystemWebViewShellLayoutTest failed because of the same reason with
other modified tests in this CL. Tests are modified to pass, but now
it does not cover {sysex:true} case that needs
AwPermissionManager::RequestPermissions implementation.
It will be implemented in the next CL soon.

Original description follows:
Web MIDI asked permissions via public/web interfaces.
But now that PermissionService is available in Blink,
use the service to ask permissions.

This migration makes it possible to remove all MIDI
related public/web interfaces.

BUG=582328

Committed: https://crrev.com/15c1a1ffd1f4c620b89191209dc76ef5557dd8fc
Cr-Commit-Position: refs/heads/master@{#404136}

TEST=${OUT}/bin/run_system_webview_shell_layout_test_apk # with a built
SystemWebViewGoogle.apk
TEST=git cl try

Committed: https://crrev.com/8c21bb977b580edb009eb2fe0d3f511fd5ca42f4
Cr-Commit-Position: refs/heads/master@{#408621}
==========

[commit-bot: I haz the power, 2016-07-29 12:28:37]

Patchset 10 (id:??) landed as
https://crrev.com/8c21bb977b580edb009eb2fe0d3f511fd5ca42f4
Cr-Commit-Position: refs/heads/master@{#408621}