Use the Blink PermissionStatus variant for notifications in Blink

Use the Blink PermissionStatus variant for notifications in Blink

Blink shouldn't use the non-Blink variant types, so update these to use
the appropriate variant instead.

BUG=595685
Committed: https://crrev.com/bd303897bfe40025be26eaaf729568429373ef79
Cr-Commit-Position: refs/heads/master@{#393916}

[Peter Beverloo, 2016-05-13 13:52:20]

Description was changed from

==========
Use blink::mojom::blink::PermissionStatus for notifications in Blink

BUG=
==========

to

==========
Use blink::mojom::blink::PermissionStatus for notifications in Blink

BUG=595685
==========

[Peter Beverloo, 2016-05-13 13:52:20]

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

[Michael van Ouwerkerk, 2016-05-13 13:58:19]

lgtm

[Peter Beverloo, 2016-05-13 13:58:45]

TBR=mkwst for the seven-character rename in /web

[Peter Beverloo, 2016-05-13 13:58:45]

TBR=mkwst for the seven-character rename in /web

[Peter Beverloo, 2016-05-13 13:59:06]

Description was changed from

==========
Use blink::mojom::blink::PermissionStatus for notifications in Blink

BUG=595685
==========

to

==========
Use blink::mojom::blink::PermissionStatus for notifications in Blink

TBR=mkwst@chromium.org (rename in Source/web)
BUG=595685
==========

[Peter Beverloo, 2016-05-13 13:59:06]

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

[Peter Beverloo, 2016-05-13 13:59:14]

The CQ bit was checked by peter@chromium.org

[commit-bot: I haz the power, 2016-05-13 13:59:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1979593002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1979593002/20001

[commit-bot: I haz the power, 2016-05-13 14:02:43]

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

[commit-bot: I haz the power, 2016-05-13 14:02:44]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[Peter Beverloo, 2016-05-13 14:53:11]

Description was changed from

==========
Use blink::mojom::blink::PermissionStatus for notifications in Blink

TBR=mkwst@chromium.org (rename in Source/web)
BUG=595685
==========

to

==========
Use the Blink PermissionStatus variant for notifications in Blink

Blink shouldn't use the non-Blink variant types, so update these to use
the appropriate variant instead.

BUG=595685
==========

[Peter Beverloo, 2016-05-13 14:54:43]

peter@chromium.org changed reviewers:
+ esprehn@chromium.org
- mkwst@chromium.org

[Peter Beverloo, 2016-05-13 14:54:44]

+esprehn

[esprehn, 2016-05-13 17:24:07]

Hmm something isn't right if you need to write blink::mojom::blink::Foo. You
don't need to nest the blink namespace twice.

[Peter Beverloo, 2016-05-13 17:28:44]

I'm using it in Chromium.

The permission service lives in Chromium, and thus receives the regular Mojo
variant. Blink now requires that only the Blink variant is used, so we have to
do this conversion somewhere.

[esprehn, 2016-05-13 17:43:33]

On 2016/05/13 at 17:28:44, peter wrote:
> I'm using it in Chromium.
> 
> The permission service lives in Chromium, and thus receives the regular Mojo
variant. Blink now requires that only the Blink variant is used, so we have to
do this conversion somewhere.

I meant I was pretty sure we were code generating it into blink::mojo::Foo there
shouldn't be another nested namespace for blink?

[esprehn, 2016-05-13 17:59:48]

hmm it does seem the blink bindings are nested twice inside blink::mojom::blink,
we should fix that separately. lgtm

[haraken, 2016-05-14 00:29:41]

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

[haraken, 2016-05-14 00:29:42]

+yzshen FYI, we should remove blink::mojom::blink::Foo. blink:: shouldn't nest.

[yzshen1, 2016-05-16 16:36:59]

On 2016/05/14 00:29:42, haraken wrote:
> +yzshen FYI, we should remove blink::mojom::blink::Foo. blink:: shouldn't
nest.

An extra layer of namespace is necessary to avoid name collision with the
chromium variant:
Chromium side: blink::mojom::Foo
Blink side: blink::mojom::blink::Foo.

(The prefix "blink::mojom" here is chosen by the interface authors, not by the
mojo bindings generator. Have we considered to rename that part?)

We had this discussion before:
https://groups.google.com/a/chromium.org/forum/#!topic/platform-architecture-...

In that thread, it seems we agreed to use "blink" as the extra layer of
namespace for the blink variant.

On this newer thread:
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-mojo/8smILScq7O4
Ken and John seemed to prefer switching "blink" back to "wtf".

Would you please join that thread to provide your suggestions? Or we could chat
about this naming issue the next time we meet on the onion soup meeting.




Maybe we should get together to chat about this naming issue. (Or

[yzshen1, 2016-05-16 16:38:10]

> Maybe we should get together to chat about this naming issue. (Or

(Please ignore this part in the previous mail...)

[Peter Beverloo, 2016-05-16 19:10:49]

The CQ bit was checked by peter@chromium.org

[Peter Beverloo, 2016-05-16 19:10:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
mvanouwerkerk@chromium.org, esprehn@chromium.org
Link to the patchset: https://codereview.chromium.org/1979593002/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2016-05-16 19:11:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1979593002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1979593002/60001

[commit-bot: I haz the power, 2016-05-16 21:08:38]

Description was changed from

==========
Use the Blink PermissionStatus variant for notifications in Blink

Blink shouldn't use the non-Blink variant types, so update these to use
the appropriate variant instead.

BUG=595685
==========

to

==========
Use the Blink PermissionStatus variant for notifications in Blink

Blink shouldn't use the non-Blink variant types, so update these to use
the appropriate variant instead.

BUG=595685
==========

[commit-bot: I haz the power, 2016-05-16 21:08:39]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-16 21:10:32]

Description was changed from

==========
Use the Blink PermissionStatus variant for notifications in Blink

Blink shouldn't use the non-Blink variant types, so update these to use
the appropriate variant instead.

BUG=595685
==========

to

==========
Use the Blink PermissionStatus variant for notifications in Blink

Blink shouldn't use the non-Blink variant types, so update these to use
the appropriate variant instead.

BUG=595685

Committed: https://crrev.com/bd303897bfe40025be26eaaf729568429373ef79
Cr-Commit-Position: refs/heads/master@{#393916}
==========

[commit-bot: I haz the power, 2016-05-16 21:10:33]

Patchset 4 (id:??) landed as
https://crrev.com/bd303897bfe40025be26eaaf729568429373ef79
Cr-Commit-Position: refs/heads/master@{#393916}