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

Issue 662510 link

Starred by 3 users
Status: Fixed
Owner:
yusukes@chromium.org
Closed: Nov 2016
Cc:
█ lhchavez@chromium.org
█ yzshen@chromium.org
yusukes@chromium.org
elijahtaylor@chromium.org
hidehiko@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue 624136



Sign in to add a comment

Switch mojom targets in components/arc to use STL types.

Project Member Reported by yzshen@chromium.org, Nov 4 2016 
At the moment, arc_bindings have the following flag:
  use_new_wrapper_types = false

That maps mojo string/array/map to mojo::String/Array/Map.
The new way is to remove this flag. And the generator will use STL types, mapping mojo string/array/map to
std::string/std::vector/std::unordered_map (or base::Optional<std::string>/... if the types are nullable).

You could take a look at this document for more details:
https://docs.google.com/document/d/1dutUW6XUlbSJzsqmtmPnN-J7R8z-iEL-QJ_GR9hWKHg/edit#heading=h.8kjqdetcrav

Comment 1 by yzshen@chromium.org, Nov 4 2016 

Luis: would you please find an owner for this? Thanks!

This change won't affect the wire format of mojo messages. Therefore, it won't introduce compatibility issues if not all parties are updated atomically.

Comment 2 by elijahtaylor@chromium.org, Nov 11 2016

Cc: hidehiko@chromium.org elijahtaylor@chromium.org yusukes@chromium.org
Status: Available (was: Untriaged)
+ a few ARC folks, if anyone can take this cleanup task to help the mojo team.  Should be a straightforward based on the CLs in  issue 624136

Comment 3 by yusukes@chromium.org, Nov 11 2016

Owner: yusukes@chromium.org
Status: Started (was: Available)

Comment 4 by yusukes@chromium.org, Nov 11 2016

Owner: lhchavez@chromium.org
turned out Luis already has an (initial) patch.

Comment 5 by lhchavez@chromium.org, Nov 12 2016

Owner: yusukes@chromium.org
Initial changes that introduce a temporary intermediate target landed. yusukes@ please help with the intent_helper code and friends. Feel free to split this bug so that the original authors finish the migration of their services.
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 16 2016 

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

commit 035067294b27c4fc3ef2e10db62037c8f3f50359
Author: yusukes <yusukes@chromium.org>
Date: Wed Nov 16 06:27:44 2016

arc: enable use_new_wrapper_types for {scale_factor,screen_rect}.mojom

BUG= 662510 
BUG= 624136 
TEST=try

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

[modify] https://crrev.com/035067294b27c4fc3ef2e10db62037c8f3f50359/components/arc/BUILD.gn
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 16 2016 

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

commit e6db796a2f93afe3913d9c84c1760b51d1652360
Author: yusukes <yusukes@chromium.org>
Date: Wed Nov 16 15:44:44 2016

arc: enable use_new_wrapper_types for intent_helper.mojom

BUG= 662510 
BUG= 624136 
TEST=try

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

[modify] https://crrev.com/e6db796a2f93afe3913d9c84c1760b51d1652360/chrome/browser/chromeos/arc/fileapi/arc_content_file_system_async_file_util_unittest.cc
[modify] https://crrev.com/e6db796a2f93afe3913d9c84c1760b51d1652360/chrome/browser/chromeos/arc/fileapi/arc_content_file_system_file_stream_reader_unittest.cc
[modify] https://crrev.com/e6db796a2f93afe3913d9c84c1760b51d1652360/chrome/browser/chromeos/arc/intent_helper/arc_external_protocol_dialog.cc
[modify] https://crrev.com/e6db796a2f93afe3913d9c84c1760b51d1652360/chrome/browser/chromeos/arc/intent_helper/arc_external_protocol_dialog.h
[modify] https://crrev.com/e6db796a2f93afe3913d9c84c1760b51d1652360/chrome/browser/chromeos/arc/intent_helper/arc_external_protocol_dialog_unittest.cc
[modify] https://crrev.com/e6db796a2f93afe3913d9c84c1760b51d1652360/chrome/browser/chromeos/arc/intent_helper/arc_navigation_throttle.cc
[modify] https://crrev.com/e6db796a2f93afe3913d9c84c1760b51d1652360/chrome/browser/chromeos/arc/intent_helper/arc_navigation_throttle.h
[modify] https://crrev.com/e6db796a2f93afe3913d9c84c1760b51d1652360/chrome/browser/chromeos/arc/intent_helper/arc_navigation_throttle_unittest.cc
[modify] https://crrev.com/e6db796a2f93afe3913d9c84c1760b51d1652360/chrome/browser/chromeos/file_manager/arc_file_tasks.cc
[modify] https://crrev.com/e6db796a2f93afe3913d9c84c1760b51d1652360/components/arc/BUILD.gn
[modify] https://crrev.com/e6db796a2f93afe3913d9c84c1760b51d1652360/components/arc/intent_helper/activity_icon_loader.cc
[modify] https://crrev.com/e6db796a2f93afe3913d9c84c1760b51d1652360/components/arc/intent_helper/activity_icon_loader.h
[modify] https://crrev.com/e6db796a2f93afe3913d9c84c1760b51d1652360/components/arc/intent_helper/arc_intent_helper_bridge.cc
[modify] https://crrev.com/e6db796a2f93afe3913d9c84c1760b51d1652360/components/arc/intent_helper/arc_intent_helper_bridge.h
[modify] https://crrev.com/e6db796a2f93afe3913d9c84c1760b51d1652360/components/arc/intent_helper/arc_intent_helper_bridge_unittest.cc
[modify] https://crrev.com/e6db796a2f93afe3913d9c84c1760b51d1652360/components/arc/intent_helper/intent_filter.cc
[modify] https://crrev.com/e6db796a2f93afe3913d9c84c1760b51d1652360/components/arc/intent_helper/intent_filter_unittest.cc
[modify] https://crrev.com/e6db796a2f93afe3913d9c84c1760b51d1652360/components/arc/intent_helper/link_handler_model_impl.cc
[modify] https://crrev.com/e6db796a2f93afe3913d9c84c1760b51d1652360/components/arc/intent_helper/link_handler_model_impl.h
[modify] https://crrev.com/e6db796a2f93afe3913d9c84c1760b51d1652360/components/arc/intent_helper/local_activity_resolver.cc
[modify] https://crrev.com/e6db796a2f93afe3913d9c84c1760b51d1652360/components/arc/intent_helper/local_activity_resolver.h
[modify] https://crrev.com/e6db796a2f93afe3913d9c84c1760b51d1652360/components/arc/intent_helper/local_activity_resolver_unittest.cc
[modify] https://crrev.com/e6db796a2f93afe3913d9c84c1760b51d1652360/components/arc/test/fake_intent_helper_instance.cc
[modify] https://crrev.com/e6db796a2f93afe3913d9c84c1760b51d1652360/components/arc/test/fake_intent_helper_instance.h

Comment 11 by yusukes@chromium.org, Nov 16 2016 

Landed:
intent_helper.mojom
notifications.mojom
scale_factor.mojom
screen_rect.mojom

Waiting for OWNER approval:
app.mojom https://codereview.chromium.org/2503633003/
enterprise_reporting.mojom https://codereview.chromium.org/2504013003/
video_accelerator.mojom https://codereview.chromium.org/2505733003/

Not yet started:
bitmap.mojom
bluetooth.mojom
net.mojom

Comment 15 by yusukes@chromium.org, Nov 17 2016

Status: Fixed (was: Started)

Comment 17 by yzshen@chromium.org, Nov 17 2016 

Woohoo! Thanks!

Comment 18 by lhchavez@chromium.org, Dec 15 2016 

the very last one in components/arc: net.mojom is also done: https://chromium.googlesource.com/chromium/src.git/+/cc2b33c4b4af32e831b227f81ccd0be0a04c056d

This is now 100% fixed for realsies on ToT.
►

Sign in to add a comment