New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 610421 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocked on:
issue 610415
issue 610417
issue 610418
issue 610420



Sign in to add a comment

Restrict usage of non-blinks style bindings inside blink

Project Member Reported by esprehn@chromium.org, May 9 2016

Issue description

Blink should only use blink style bindings, otherwise there's expensive copies and type conversions inside all the mojo types.

Today blink generates both styles of bindings, only the .mojom-blink.h headers should be includable from inside blink though, not the stl style .mojom.h ones.
 
We need to update the PRESUBMIT to enforce this, no header named .mojom.h should be includable inside blink. Only .mojom-blink.h headers.

Cc: roc...@chromium.org yzshen@chromium.org dcheng@chromium.org
Does anyone have a bandwidth to work on this?

It looks better to fix this bug soon. Otherwise people start using .mojom.h (e.g., https://codereview.chromium.org/1948223004/).

Comment 5 by dcheng@chromium.org, May 13 2016

Owner: dcheng@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 6 by bugdroid1@chromium.org, May 13 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/378010a9e3fcf55ec9b63183e06ddc1848734d28

commit 378010a9e3fcf55ec9b63183e06ddc1848734d28
Author: dcheng <dcheng@chromium.org>
Date: Fri May 13 05:09:46 2016

Add PRESUBMIT check for including non-Blink variant mojoms in Blink.

Blink code should only use the Blink variants of the generated mojom
headers.

BUG= 610421 
R=esprehn@chromium.org,haraken@chromium.org,tkent@chromium.org

Review-Url: https://codereview.chromium.org/1976883002
Cr-Commit-Position: refs/heads/master@{#393449}

[modify] https://crrev.com/378010a9e3fcf55ec9b63183e06ddc1848734d28/third_party/WebKit/PRESUBMIT.py

Comment 7 by dcheng@chromium.org, May 13 2016

Status: Fixed (was: Started)

Comment 8 by dcheng@chromium.org, May 13 2016

Owner: ----
Status: Available (was: Fixed)
(Oh, maybe we want to leave this open as the master bug for the issues that currently exist.)

Comment 9 by tkent@chromium.org, Jun 23 2016

Components: -Blink>Architecture Blink>Internals
Renaming Blink>Architecture to Blink>Internals

Status: Fixed (was: Available)
Project Member

Comment 11 by bugdroid1@chromium.org, Jul 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e4fbef2b7c59d740551a78f1a6c2c035aff7cb07

commit e4fbef2b7c59d740551a78f1a6c2c035aff7cb07
Author: Matt Falkenhagen <falken@chromium.org>
Date: Wed Jul 19 10:03:01 2017

PRESUBMIT: Add more clues to the error about non-Blink variants.

The existing message didn't really give the reader advice about what to
do, if they hadn't heard of "non-Blink variant mojoms" before.

Bug:  610421 
Change-Id: Idce6432a28c613d901c3f33f6d64f03f700238d0
Reviewed-on: https://chromium-review.googlesource.com/576605
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487811}
[modify] https://crrev.com/e4fbef2b7c59d740551a78f1a6c2c035aff7cb07/third_party/WebKit/PRESUBMIT.py

Sign in to add a comment