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

Issue 770034 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Combine various ARC++ typemaps for gfx::Rect

Project Member Reported by dcheng@chromium.org, Sep 29 2017

Issue description

https://cs.chromium.org/search/?q=file:components/arc+file:typemap+%22gfx::Rect%22&sq=package:chromium&type=cs

ARC++ currently has two structs typemapped to gfx::Rect. It should just be the same typemap if possible. This reduces the amount of duplication among struct traits validation logic.

I also notice that https://cs.chromium.org/chromium/src/components/arc/ime/arc_ime_struct_traits.cc?rcl=19a84daf8a8b078d9476a2a4d80cc6a5eb7adf4e&l=15 currently forgets to validate that width / height are non-negative, while https://cs.chromium.org/chromium/src/components/arc/common/app_struct_traits.cc?rcl=19a84daf8a8b078d9476a2a4d80cc6a5eb7adf4e&l=9 does.
 
Labels: OS-Chrome
Owner: elijahtaylor@chromium.org
Status: Assigned (was: Untriaged)
Elijah for routing/assignment.
Cc: yhanada@chromium.org
Owner: hidehiko@chromium.org
Hidehiko, seems like a good cleanup, is this something you can help with?  +yhanada original author of the IME version
Status: Started (was: Assigned)
For long term, we'd like to migrate to ui/gfx/geometry/mojo/geometry.mojom.
However, currently rolled libmojo is too old to work on this, IMHO.
So, let's revisit here once it's done. crbug.com/723224 is the tracking bug.

Meanwhile, I'd update the code to use same logic.

crbug.com/723224 is related. I tried to use gfx::Rect in geometry.mojom directly, but gave up to do that because it's difficult to sync mojom files between chromium and Android.
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 13 2017

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

commit 20b9165c44879c024b71e881a3600c65a8d8cc13
Author: Hidehiko Abe <hidehiko@chromium.org>
Date: Fri Oct 13 05:00:27 2017

Migrate Rect/Range into gfx.mojom.

We defined such structs in each mojom as needed.
This CL extracts them into gfx.mojom.

BUG= 770034 
TEST=Ran on DUT. Ran bots.

Change-Id: I04b4e4530927fc59b3f334d74f0cde7536d177a2
Reviewed-on: https://chromium-review.googlesource.com/700179
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Kazuhiro Inaba <kinaba@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508598}
[modify] https://crrev.com/20b9165c44879c024b71e881a3600c65a8d8cc13/chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_arc_home_service.cc
[modify] https://crrev.com/20b9165c44879c024b71e881a3600c65a8d8cc13/chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_arc_home_service_browsertest.cc
[modify] https://crrev.com/20b9165c44879c024b71e881a3600c65a8d8cc13/components/arc/BUILD.gn
[modify] https://crrev.com/20b9165c44879c024b71e881a3600c65a8d8cc13/components/arc/common/accessibility_helper.mojom
[modify] https://crrev.com/20b9165c44879c024b71e881a3600c65a8d8cc13/components/arc/common/app.mojom
[delete] https://crrev.com/b09b37a9eeb8731ea31cce852ee55925a89ca83c/components/arc/common/app.typemap
[delete] https://crrev.com/b09b37a9eeb8731ea31cce852ee55925a89ca83c/components/arc/common/app_struct_traits.cc
[delete] https://crrev.com/b09b37a9eeb8731ea31cce852ee55925a89ca83c/components/arc/common/app_struct_traits.h
[add] https://crrev.com/20b9165c44879c024b71e881a3600c65a8d8cc13/components/arc/common/arc_gfx_struct_traits.cc
[add] https://crrev.com/20b9165c44879c024b71e881a3600c65a8d8cc13/components/arc/common/arc_gfx_struct_traits.h
[add] https://crrev.com/20b9165c44879c024b71e881a3600c65a8d8cc13/components/arc/common/gfx.mojom
[add] https://crrev.com/20b9165c44879c024b71e881a3600c65a8d8cc13/components/arc/common/gfx.typemap
[modify] https://crrev.com/20b9165c44879c024b71e881a3600c65a8d8cc13/components/arc/common/ime.mojom
[delete] https://crrev.com/b09b37a9eeb8731ea31cce852ee55925a89ca83c/components/arc/common/ime.typemap
[delete] https://crrev.com/b09b37a9eeb8731ea31cce852ee55925a89ca83c/components/arc/common/screen_rect.mojom
[modify] https://crrev.com/20b9165c44879c024b71e881a3600c65a8d8cc13/components/arc/common/typemaps.gni
[modify] https://crrev.com/20b9165c44879c024b71e881a3600c65a8d8cc13/components/arc/common/voice_interaction_arc_home.mojom
[modify] https://crrev.com/20b9165c44879c024b71e881a3600c65a8d8cc13/components/arc/common/voice_interaction_framework.mojom
[modify] https://crrev.com/20b9165c44879c024b71e881a3600c65a8d8cc13/components/arc/ime/arc_ime_bridge_impl.cc
[modify] https://crrev.com/20b9165c44879c024b71e881a3600c65a8d8cc13/components/arc/ime/arc_ime_bridge_impl.h
[delete] https://crrev.com/b09b37a9eeb8731ea31cce852ee55925a89ca83c/components/arc/ime/arc_ime_struct_traits.cc
[delete] https://crrev.com/b09b37a9eeb8731ea31cce852ee55925a89ca83c/components/arc/ime/arc_ime_struct_traits.h

Status: Fixed (was: Started)

Comment 7 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 8 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)

Sign in to add a comment