Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(5)

Issue 614663003: Change ExternalApplicationRegistrar::Register API (Closed)

Created:
6 years, 2 months ago by Chris Masone
Modified:
6 years, 1 month ago
Reviewers:
DaveMoore
CC:
chromium-reviews, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, darin (slow to review), ben+mojo_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Change ExternalApplicationRegistrar::Register API The initial implementation of ExternalApplicationRegistrar::Register took a Shell& that couldn't be used until the Register call was acked. This also meant that the Shell passed to Register() had to be bound to a Pipe at or before the point-of-call, on the thread where the call was made. This is problematic for handing this Shell off to a new application that's going to want to spin up its own main thread. Since the Shell can't be used at the point of call anyway, this change alters the Register() API to simply pass back a Shell via a callback, instead of taking a Shell&. BUG=407785 TEST=unit tests R=davemoore

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -20 lines) Patch
M mojo/shell/external_application_listener_posix.cc View 2 chunks +7 lines, -7 lines 0 comments Download
M mojo/shell/external_application_listener_unittest.cc View 2 chunks +10 lines, -2 lines 0 comments Download
M mojo/shell/external_application_registrar.mojom View 2 chunks +3 lines, -2 lines 0 comments Download
M mojo/shell/external_application_registrar_connection.h View 1 chunk +1 line, -5 lines 0 comments Download
M mojo/shell/external_application_registrar_connection.cc View 1 chunk +2 lines, -4 lines 0 comments Download

Messages

Total messages: 1 (0 generated)
Chris Masone
6 years, 2 months ago (2014-10-03 22:47:10 UTC) #1
Here's the Register API changed and the tests fixed, if you want to use it.

Powered by Google App Engine
This is Rietveld 408576698