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

Issue 816970 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Don't implicitly convert between mojo type string and C++ base::string16

Project Member Reported by estade@google.com, Feb 27 2018

Issue description

We need to remove string_traits_string16.cc/h and change mojom files to use mojo_base.mojom.String16 instead of string for base::string16.

The only thing currently blocking this is the credential manager. Context here: https://chromium-review.googlesource.com/c/chromium/src/+/920688
 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 27 2018

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

commit 86836ab20a6006edcb0c4b5fbb7f3d8e503193ed
Author: Evan Stade <estade@chromium.org>
Date: Tue Feb 27 21:47:54 2018

Don't implicitly convert between mojo type string and C++ base::string16

Update most .mojom files to use mojo_base.mojom.String16. This implicit
conversion adds extra UTF8<->UTF16 conversions and makes mojo definitions
less self documenting.

CredentialManager is the last hold out. Fixing that and removing
string_traits_string16.cc/h is punted for now.

Bug:  816970 
Change-Id: Ibf214e8f4096616291ff493f5ddf5c79c6a16cf4
Reviewed-on: https://chromium-review.googlesource.com/920688
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539553}
[modify] https://crrev.com/86836ab20a6006edcb0c4b5fbb7f3d8e503193ed/chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos_unittest.cc
[modify] https://crrev.com/86836ab20a6006edcb0c4b5fbb7f3d8e503193ed/chrome/browser/ui/views/ime_driver/remote_text_input_client.cc
[modify] https://crrev.com/86836ab20a6006edcb0c4b5fbb7f3d8e503193ed/components/autofill/content/common/BUILD.gn
[modify] https://crrev.com/86836ab20a6006edcb0c4b5fbb7f3d8e503193ed/components/autofill/content/common/autofill_types.mojom
[modify] https://crrev.com/86836ab20a6006edcb0c4b5fbb7f3d8e503193ed/components/autofill/content/common/autofill_types_struct_traits.cc
[modify] https://crrev.com/86836ab20a6006edcb0c4b5fbb7f3d8e503193ed/components/spellcheck/common/DEPS
[modify] https://crrev.com/86836ab20a6006edcb0c4b5fbb7f3d8e503193ed/components/spellcheck/common/spellcheck.mojom
[modify] https://crrev.com/86836ab20a6006edcb0c4b5fbb7f3d8e503193ed/components/spellcheck/common/spellcheck_struct_traits.cc
[modify] https://crrev.com/86836ab20a6006edcb0c4b5fbb7f3d8e503193ed/components/translate/content/common/translate.mojom
[modify] https://crrev.com/86836ab20a6006edcb0c4b5fbb7f3d8e503193ed/components/translate/content/common/translate_struct_traits.cc
[modify] https://crrev.com/86836ab20a6006edcb0c4b5fbb7f3d8e503193ed/services/ui/ime/ime_unittest.cc
[modify] https://crrev.com/86836ab20a6006edcb0c4b5fbb7f3d8e503193ed/services/ui/public/interfaces/ime/ime.mojom
[modify] https://crrev.com/86836ab20a6006edcb0c4b5fbb7f3d8e503193ed/services/ui/public/interfaces/ime/ime_struct_traits.cc
[modify] https://crrev.com/86836ab20a6006edcb0c4b5fbb7f3d8e503193ed/services/ui/public/interfaces/ime/ime_struct_traits.h
[modify] https://crrev.com/86836ab20a6006edcb0c4b5fbb7f3d8e503193ed/services/ui/public/interfaces/ime/ime_struct_traits_unittest.cc
[modify] https://crrev.com/86836ab20a6006edcb0c4b5fbb7f3d8e503193ed/third_party/WebKit/public/platform/modules/notifications/notification.mojom
[modify] https://crrev.com/86836ab20a6006edcb0c4b5fbb7f3d8e503193ed/third_party/WebKit/public/platform/modules/notifications/notification_service.mojom
[modify] https://crrev.com/86836ab20a6006edcb0c4b5fbb7f3d8e503193ed/ui/aura/mus/text_input_client_impl.cc
[modify] https://crrev.com/86836ab20a6006edcb0c4b5fbb7f3d8e503193ed/ui/aura/mus/text_input_client_impl.h
[modify] https://crrev.com/86836ab20a6006edcb0c4b5fbb7f3d8e503193ed/ui/message_center/public/mojo/notification.mojom

Project Member

Comment 2 by bugdroid1@chromium.org, Mar 21 2018

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

commit 882d99b517e02c6188018b4fceda0e32f882874d
Author: Evan Stade <estade@chromium.org>
Date: Wed Mar 21 14:55:06 2018

Make CredentialManager use mojo_base.mojom.String16 instead of string

Remove ability to implicitly convert between mojo type string and
base::string16

See 86836ab20a6006edc

Bug:  816970 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I5633ff8d5b6bba56ea568d0f49a38d1d4157b6b1
Reviewed-on: https://chromium-review.googlesource.com/965453
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544699}
[modify] https://crrev.com/882d99b517e02c6188018b4fceda0e32f882874d/components/password_manager/content/common/credential_manager_mojom_traits.cc
[modify] https://crrev.com/882d99b517e02c6188018b4fceda0e32f882874d/components/password_manager/content/common/credential_manager_mojom_traits.h
[modify] https://crrev.com/882d99b517e02c6188018b4fceda0e32f882874d/components/password_manager/core/common/credential_manager_types.cc
[modify] https://crrev.com/882d99b517e02c6188018b4fceda0e32f882874d/components/password_manager/core/common/credential_manager_types.h
[modify] https://crrev.com/882d99b517e02c6188018b4fceda0e32f882874d/ios/chrome/browser/passwords/credential_manager_util.mm
[modify] https://crrev.com/882d99b517e02c6188018b4fceda0e32f882874d/ios/chrome/browser/passwords/js_credential_manager.mm
[modify] https://crrev.com/882d99b517e02c6188018b4fceda0e32f882874d/mojo/public/cpp/bindings/BUILD.gn
[modify] https://crrev.com/882d99b517e02c6188018b4fceda0e32f882874d/mojo/public/cpp/bindings/lib/serialization.h
[delete] https://crrev.com/96590f23d22e30ff333c3e64ac5a6f3b7a0d30c7/mojo/public/cpp/bindings/lib/string_traits_string16.cc
[delete] https://crrev.com/96590f23d22e30ff333c3e64ac5a6f3b7a0d30c7/mojo/public/cpp/bindings/string_traits_string16.h
[modify] https://crrev.com/882d99b517e02c6188018b4fceda0e32f882874d/third_party/WebKit/LayoutTests/http/tests/credentialmanager/credentialscontainer-create-from-nested-frame-expected.txt
[modify] https://crrev.com/882d99b517e02c6188018b4fceda0e32f882874d/third_party/WebKit/LayoutTests/http/tests/credentialmanager/credentialscontainer-get-from-nested-frame-expected.txt
[modify] https://crrev.com/882d99b517e02c6188018b4fceda0e32f882874d/third_party/WebKit/LayoutTests/http/tests/credentialmanager/resources/credential-helpers.js
[modify] https://crrev.com/882d99b517e02c6188018b4fceda0e32f882874d/third_party/WebKit/LayoutTests/virtual/enable-webauthn/http/tests/credentialmanager/credentialscontainer-create-from-nested-frame-expected.txt
[modify] https://crrev.com/882d99b517e02c6188018b4fceda0e32f882874d/third_party/WebKit/LayoutTests/virtual/enable-webauthn/http/tests/credentialmanager/credentialscontainer-get-from-nested-frame-expected.txt
[modify] https://crrev.com/882d99b517e02c6188018b4fceda0e32f882874d/third_party/WebKit/public/platform/modules/credentialmanager/credential_manager.mojom

Comment 3 by est...@chromium.org, Mar 21 2018

Status: Fixed (was: Assigned)

Sign in to add a comment