New issue
Advanced search Search tips

Issue 915912 link

Starred by 1 user
Status: Fixed
Owner:
rockot@google.com
Closed: Dec 19
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Support test-only mojoms without security review

Project Member Reported by rockot@google.com, Dec 17 
As discussed briefly with dcheng@ - we should support a .test-mojom file extension in mojom.gni, which works just like .mojom except it requires that the GN mojom rule set testonly = true. Then these files can be exempt from the PRESUBMIT rule which enforces SECURITY_OWNERS coverage.
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 19 

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

commit 8e6635a1fc020deda7b09779f8f8ecb03931056f
Author: Ken Rockot <rockot@google.com>
Date: Wed Dec 19 09:00:07 2018

[mojo] Support .test-mojom extension

This changes the mojom GN rule logic in general to work with arbitrary
input file extensions instead of assuming .mojom. It then adds an
explicit constraint to require that all files are either .mojom or
.test-mojom, and if the latter, that the target has testonly=true set.

This provides a nice clean way for developers to define test-only mojoms
without incurring the burden of SECURITY_OWNERS coverage.

Also applies the new shiny thing to Service Manager unittests.

Bug:  915912 
Change-Id: Iebaa7521b02c68b90b9a753358c286e4098c311b
Reviewed-on: https://chromium-review.googlesource.com/c/1381243
Commit-Queue: Ken Rockot <rockot@google.com>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617773}
[modify] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/mojo/public/tools/bindings/mojom.gni
[modify] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/background/tests/BUILD.gn
[delete] https://crrev.com/772250a93f51155cb5f434b6fbe0e03c7261cf2d/services/service_manager/background/tests/OWNERS
[rename] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/background/tests/background.test-mojom
[modify] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/background/tests/background_service_manager_unittest.cc
[modify] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/background/tests/test_service.cc
[modify] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/tests/connect/BUILD.gn
[delete] https://crrev.com/772250a93f51155cb5f434b6fbe0e03c7261cf2d/services/service_manager/tests/connect/OWNERS
[rename] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/tests/connect/connect.test-mojom
[modify] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/tests/connect/connect_test_app.cc
[modify] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/tests/connect/connect_test_class_app.cc
[modify] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/tests/connect/connect_test_exe.cc
[modify] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/tests/connect/connect_test_package.cc
[modify] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/tests/connect/connect_unittest.cc
[modify] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/tests/lifecycle/BUILD.gn
[delete] https://crrev.com/772250a93f51155cb5f434b6fbe0e03c7261cf2d/services/service_manager/tests/lifecycle/OWNERS
[modify] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/tests/lifecycle/app_client.h
[rename] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/tests/lifecycle/lifecycle.test-mojom
[modify] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/tests/lifecycle/lifecycle_unittest.cc
[modify] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/tests/lifecycle/package.cc
[modify] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/tests/lifecycle/parent.cc
[modify] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/tests/service_manager/BUILD.gn
[delete] https://crrev.com/772250a93f51155cb5f434b6fbe0e03c7261cf2d/services/service_manager/tests/service_manager/OWNERS
[rename] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/tests/service_manager/service_manager.test-mojom
[modify] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/tests/service_manager/service_manager_unittest.cc
[modify] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/tests/service_manager/target.cc
[modify] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/tests/service_manager/test_manifests.cc
[modify] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/tests/shutdown/BUILD.gn
[delete] https://crrev.com/772250a93f51155cb5f434b6fbe0e03c7261cf2d/services/service_manager/tests/shutdown/OWNERS
[rename] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/tests/shutdown/shutdown.test-mojom
[modify] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/tests/shutdown/shutdown_client_app.cc
[modify] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/tests/shutdown/shutdown_service_app.cc
[modify] https://crrev.com/8e6635a1fc020deda7b09779f8f8ecb03931056f/services/service_manager/tests/shutdown/shutdown_unittest.cc

Comment 2 by rockot@google.com, Dec 19

Status: Fixed (was: Started)
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 2 

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

commit 7d5d7f1888e01d0a67e08e03176c09a85730a11a
Author: James Cook <jamescook@chromium.org>
Date: Wed Jan 02 19:16:16 2019

chromeos: Rename ash test-only mojoms to .test-mojom

This allows test-only APIs to be added without SECURITY_OWNERS review.

TBR=bartfab@chromium.org

Bug:  915912 
Test: compiles
Change-Id: I9f8fb2e30b15f71aeddc778da571560b0a60d2a9
Reviewed-on: https://chromium-review.googlesource.com/c/1384926
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619451}
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/ash/login/login_screen_test_api.h
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/ash/metrics/time_to_first_present_recorder_test_api.h
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/ash/mojo_test_interface_factory.cc
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/ash/public/interfaces/BUILD.gn
[rename] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/ash/public/interfaces/login_screen_test_api.test-mojom
[rename] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/ash/public/interfaces/shelf_test_api.test-mojom
[rename] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/ash/public/interfaces/shell_test_api.test-mojom
[rename] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/ash/public/interfaces/status_area_widget_test_api.test-mojom
[rename] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/ash/public/interfaces/system_tray_test_api.test-mojom
[rename] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/ash/public/interfaces/time_to_first_present_recorder_test_api.test-mojom
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/ash/shelf/shelf_test_api.h
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/ash/shell_test_api.h
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/ash/system/status_area_widget_test_api.h
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/ash/system/unified/unified_system_tray_test_api.h
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/chrome/browser/chromeos/accessibility/select_to_speak_browsertest.cc
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/chrome/browser/chromeos/first_run/chromeos_first_run_browsertest.cc
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/chrome/browser/chromeos/login/lock/screen_locker_tester.cc
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/chrome/browser/chromeos/login/login_shelf_test_helper.h
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/chrome/browser/chromeos/policy/device_system_use_24hour_clock_browsertest.cc
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/chrome/browser/chromeos/shutdown_policy_browsertest.cc
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/chrome/browser/chromeos/system/tray_accessibility_browsertest.cc
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/chrome/browser/ui/ash/accelerator_commands_browsertest.cc
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/chrome/browser/ui/ash/network/networking_config_chromeos_browsertest.cc
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/chrome/browser/ui/ash/shelf_browsertest.cc
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/chrome/browser/ui/ash/system_tray_client_browsertest.cc
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/chrome/browser/ui/ash/system_tray_tray_cast_browsertest_media_router_chromeos.cc
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/chrome/browser/ui/ash/tablet_mode_client_test_util.cc
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/chrome/browser/ui/ash/time_to_first_present_recorder_browsertest.cc
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/chrome/browser/ui/views/frame/immersive_mode_controller_ash_browsertest.cc
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/chrome/browser/ui/views/select_file_dialog_extension_browsertest.cc
[modify] https://crrev.com/7d5d7f1888e01d0a67e08e03176c09a85730a11a/chrome/browser/ui/webui/chromeos/system_web_dialog_browsertest.cc
►

Sign in to add a comment