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

Issue 2113933003: Add mojo interface to load hyphenation dictionaries on Android (Closed)

Created:
4 years, 5 months ago by kojii
Modified:
4 years, 5 months ago
CC:
Aaron Boodman, abarth-chromium, ben+mojo_chromium.org, blink-reviews, blundell+watchlist_chromium.org, chromium-reviews, creis+watch_chromium.org, darin (slow to review), darin-cc_chromium.org, droger+watchlist_chromium.org, nasko+codewatch_chromium.org, qsr+mojo_chromium.org, sdefresne+watchlist_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add mojo interface to load hyphenation dictionaries on Android This patch adds a mojo interface to load hyphenation dictionaries on Android. Calls to Minikin using the loaded dictionary files will be in following patches. BUG=605840 Committed: https://crrev.com/30add38fedb8b9bd44b931aa3adba40d7c79b929 Cr-Commit-Position: refs/heads/master@{#405405}

Patch Set 1 #

Patch Set 2 : Still WIP #

Patch Set 3 : Move mojom to blink #

Patch Set 4 : Minor editorial #

Patch Set 5 : rebase #

Patch Set 6 : single process hack #

Patch Set 7 : Build on Linux #

Patch Set 8 : Cleanup #

Patch Set 9 : Rebase #

Patch Set 10 : Try to fix Android build error #

Patch Set 11 : cleanup #

Patch Set 12 : Moved UMA to renderer to include IPC time #

Patch Set 13 : Revert changes to sync_call_restrictions.h #

Patch Set 14 : Android build fix #

Patch Set 15 : Moved to content/browser #

Total comments: 9

Patch Set 16 : jam/eae review #

Total comments: 9

Patch Set 17 : dcheng review #

Unified diffs Side-by-side diffs Delta from patch set Stats (+322 lines, -19 lines) Patch
M build/common.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 4 chunks +12 lines, -0 lines 0 comments Download
M build/config/features.gni View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +3 lines, -0 lines 0 comments Download
M content/browser/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +8 lines, -0 lines 0 comments Download
A content/browser/hyphenation/DEPS View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +5 lines, -0 lines 0 comments Download
A content/browser/hyphenation/OWNERS View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +4 lines, -0 lines 0 comments Download
A content/browser/hyphenation/hyphenation_impl.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +34 lines, -0 lines 0 comments Download
A content/browser/hyphenation/hyphenation_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +72 lines, -0 lines 0 comments Download
M content/browser/renderer_host/render_process_host_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +12 lines, -2 lines 0 comments Download
M content/content_browser.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +6 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/text/hyphens/can-hyphenate-locale.html View 1 2 3 4 5 6 7 8 9 1 chunk +23 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/text/hyphens/can-hyphenate-locale-expected.txt View 1 2 3 4 5 6 7 8 9 10 1 chunk +6 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/platform/android/fast/text/hyphens/can-hyphenate-locale-expected.txt View 1 2 3 4 5 6 7 8 9 10 1 chunk +6 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/platform/mac/fast/text/hyphens/can-hyphenate-locale-expected.txt View 1 2 3 4 5 6 7 8 9 10 1 chunk +6 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/testing/Internals.h View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/testing/Internals.cpp View 1 2 3 4 5 6 7 1 chunk +5 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/testing/Internals.idl View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 2 chunks +5 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/DEPS View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/blink_platform.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +0 lines, -1 line 0 comments Download
D third_party/WebKit/Source/platform/text/android/HyphenationAndroid.cpp View 1 2 3 4 5 6 1 chunk +0 lines, -14 lines 0 comments Download
A third_party/WebKit/Source/platform/text/hyphenation/HyphenationMinikin.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +93 lines, -0 lines 0 comments Download
M third_party/WebKit/public/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/public/blink.gyp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +1 line, -0 lines 0 comments Download
A third_party/WebKit/public/platform/modules/hyphenation/OWNERS View 1 2 1 chunk +7 lines, -0 lines 0 comments Download
A + third_party/WebKit/public/platform/modules/hyphenation/hyphenation.mojom View 1 2 1 chunk +4 lines, -2 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 33 (10 generated)
kojii
PTAL. jochen@: build, components, content rockot@: mojo (or in another CL is also fine, I'll ...
4 years, 5 months ago (2016-07-07 13:25:11 UTC) #4
Ken Rockot(use gerrit already)
I don't think we should do this SyncCallRestrictions change. Having to do it for every ...
4 years, 5 months ago (2016-07-07 13:37:41 UTC) #5
Ken Rockot(use gerrit already)
I don't think we should do this SyncCallRestrictions change. Having to do it for every ...
4 years, 5 months ago (2016-07-07 13:37:41 UTC) #6
Ken Rockot(use gerrit already)
Should be fine in single-process mode as of https://crrev.com/be559ecbba727fb1a78cbff249195cad244375a6
4 years, 5 months ago (2016-07-07 19:26:34 UTC) #8
kojii
On 2016/07/07 at 19:26:34, rockot wrote: > Should be fine in single-process mode as of ...
4 years, 5 months ago (2016-07-08 03:31:22 UTC) #9
Ken Rockot(use gerrit already)
lgtm
4 years, 5 months ago (2016-07-08 05:01:19 UTC) #10
jochen (gone - plz use gerrit)
why do you need a component, as opposed to just putting this into content?
4 years, 5 months ago (2016-07-08 12:56:35 UTC) #12
kojii
On 2016/07/08 at 12:56:35, jochen wrote: > why do you need a component, as opposed ...
4 years, 5 months ago (2016-07-08 13:56:38 UTC) #13
jam
On 2016/07/08 13:56:38, kojii wrote: > On 2016/07/08 at 12:56:35, jochen wrote: > > why ...
4 years, 5 months ago (2016-07-09 03:04:52 UTC) #14
kojii
On 2016/07/09 at 03:04:52, jam wrote: > On 2016/07/08 13:56:38, kojii wrote: > > On ...
4 years, 5 months ago (2016-07-09 08:23:29 UTC) #15
kojii
PTAL, moved to content/browser. Thank you for your reviews and suggestions. IIUC, what platform-architecture-dev@ was ...
4 years, 5 months ago (2016-07-12 06:57:08 UTC) #16
jam
you'll need to add a security reviewer from ipc/SECURITY_OWNERS for the new mojo file https://codereview.chromium.org/2113933003/diff/280001/content/browser/hyphenation/BUILD.gn ...
4 years, 5 months ago (2016-07-12 14:51:12 UTC) #18
eae
LGTM for Blink https://codereview.chromium.org/2113933003/diff/280001/third_party/WebKit/Source/platform/text/minikin/HyphenationMinikin.cpp File third_party/WebKit/Source/platform/text/minikin/HyphenationMinikin.cpp (right): https://codereview.chromium.org/2113933003/diff/280001/third_party/WebKit/Source/platform/text/minikin/HyphenationMinikin.cpp#newcode1 third_party/WebKit/Source/platform/text/minikin/HyphenationMinikin.cpp:1: // Copyright 2016 The Chromium Authors. ...
4 years, 5 months ago (2016-07-12 16:11:13 UTC) #19
kojii
Thank you for your reviews, jam & eae. > you'll need to add a security ...
4 years, 5 months ago (2016-07-12 17:32:21 UTC) #21
jam
lgtm, thanks
4 years, 5 months ago (2016-07-12 19:26:51 UTC) #22
dcheng
https://codereview.chromium.org/2113933003/diff/300001/content/browser/hyphenation/hyphenation_impl.cc File content/browser/hyphenation/hyphenation_impl.cc (right): https://codereview.chromium.org/2113933003/diff/300001/content/browser/hyphenation/hyphenation_impl.cc#newcode30 content/browser/hyphenation/hyphenation_impl.cc:30: // when std::map::emplace() is fully supported on all configurations. ...
4 years, 5 months ago (2016-07-13 02:56:29 UTC) #23
kojii
https://codereview.chromium.org/2113933003/diff/300001/content/browser/hyphenation/hyphenation_impl.cc File content/browser/hyphenation/hyphenation_impl.cc (right): https://codereview.chromium.org/2113933003/diff/300001/content/browser/hyphenation/hyphenation_impl.cc#newcode30 content/browser/hyphenation/hyphenation_impl.cc:30: // when std::map::emplace() is fully supported on all configurations. ...
4 years, 5 months ago (2016-07-13 05:02:29 UTC) #24
jochen (gone - plz use gerrit)
build lgtm
4 years, 5 months ago (2016-07-13 08:29:26 UTC) #25
dcheng
mojo lgtm https://codereview.chromium.org/2113933003/diff/300001/third_party/WebKit/Source/platform/text/hyphenation/HyphenationMinikin.cpp File third_party/WebKit/Source/platform/text/hyphenation/HyphenationMinikin.cpp (right): https://codereview.chromium.org/2113933003/diff/300001/third_party/WebKit/Source/platform/text/hyphenation/HyphenationMinikin.cpp#newcode33 third_party/WebKit/Source/platform/text/hyphenation/HyphenationMinikin.cpp:33: if (service.is_bound()) On 2016/07/13 05:02:29, kojii wrote: ...
4 years, 5 months ago (2016-07-13 12:08:59 UTC) #26
Steven Holte
On 2016/07/13 12:08:59, dcheng wrote: > mojo lgtm > > https://codereview.chromium.org/2113933003/diff/300001/third_party/WebKit/Source/platform/text/hyphenation/HyphenationMinikin.cpp > File third_party/WebKit/Source/platform/text/hyphenation/HyphenationMinikin.cpp > ...
4 years, 5 months ago (2016-07-13 17:50:55 UTC) #27
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2113933003/320001
4 years, 5 months ago (2016-07-14 01:37:12 UTC) #30
commit-bot: I haz the power
Committed patchset #17 (id:320001)
4 years, 5 months ago (2016-07-14 02:10:47 UTC) #31
commit-bot: I haz the power
4 years, 5 months ago (2016-07-14 02:13:10 UTC) #33
Message was sent while issue was closed.
Patchset 17 (id:??) landed as
https://crrev.com/30add38fedb8b9bd44b931aa3adba40d7c79b929
Cr-Commit-Position: refs/heads/master@{#405405}

Powered by Google App Engine
This is Rietveld 408576698