New issue
Advanced search Search tips

Issue 714480 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Mojo components/spellcheck/common/spellcheck_messages.h

Project Member Reported by noel@chromium.org, Apr 24 2017

Issue description

The IPC in components/spellcheck/common/spellcheck_messages.h should be converted to mojo <eom>.
 

Comment 1 by noel@chromium.org, May 1 2017

Components: UI>Browser>Spellcheck UI>Browser>Language>Spellcheck Internals>Mojo
Project Member

Comment 2 by bugdroid1@chromium.org, May 11 2017

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

commit e3d7e30c8eb15dd89116d863b9d293dd22979a7c
Author: Noel Gordon <noel@chromium.org>
Date: Thu May 11 23:48:33 2017

Convert render process component common Spellcheck IPC to mojo

Add a render process mojo interface exposed to the browser for
receiving process-wide spellcheck control and updates from the
browser process.

Fold the enable IPC into the Initialize() mojo API (the enable
IPC was only ever called right after an Init IPC) by adding an
|enable| argument. Mojo does not support set<> so use vector<>
instead on the mojo APIs. Use a binding set on the server-side
to automate mojo request disconnect and clean-up.

Update the browser test to use mojo instead of IPC in the mock
renderer and add a test for a custom dictionary update. Change
the InitSpellcheck helper: spellcheck->InitForRenderer() isn't
needed (creating a SpellcheckService does that for us).

Bug:714480

Change-Id: I5365c42fc10c7766054c5b80fb366ceaa4d59159
Reviewed-on: https://chromium-review.googlesource.com/487341
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Martin Barbella <mbarbella@chromium.org>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#471125}
[modify] https://crrev.com/e3d7e30c8eb15dd89116d863b9d293dd22979a7c/chrome/browser/chrome_content_renderer_manifest_overlay.json
[modify] https://crrev.com/e3d7e30c8eb15dd89116d863b9d293dd22979a7c/chrome/browser/spellchecker/spellcheck_service.cc
[modify] https://crrev.com/e3d7e30c8eb15dd89116d863b9d293dd22979a7c/chrome/browser/spellchecker/spellcheck_service.h
[modify] https://crrev.com/e3d7e30c8eb15dd89116d863b9d293dd22979a7c/chrome/browser/spellchecker/spellcheck_service_browsertest.cc
[modify] https://crrev.com/e3d7e30c8eb15dd89116d863b9d293dd22979a7c/components/spellcheck/common/BUILD.gn
[modify] https://crrev.com/e3d7e30c8eb15dd89116d863b9d293dd22979a7c/components/spellcheck/common/OWNERS
[add] https://crrev.com/e3d7e30c8eb15dd89116d863b9d293dd22979a7c/components/spellcheck/common/spellcheck.mojom
[delete] https://crrev.com/6b54cbfc3f6586b238744d9bd30f0bd49bda1dce/components/spellcheck/common/spellcheck_bdict_language.h
[modify] https://crrev.com/e3d7e30c8eb15dd89116d863b9d293dd22979a7c/components/spellcheck/common/spellcheck_messages.h
[modify] https://crrev.com/e3d7e30c8eb15dd89116d863b9d293dd22979a7c/components/spellcheck/renderer/BUILD.gn
[modify] https://crrev.com/e3d7e30c8eb15dd89116d863b9d293dd22979a7c/components/spellcheck/renderer/DEPS
[modify] https://crrev.com/e3d7e30c8eb15dd89116d863b9d293dd22979a7c/components/spellcheck/renderer/spellcheck.cc
[modify] https://crrev.com/e3d7e30c8eb15dd89116d863b9d293dd22979a7c/components/spellcheck/renderer/spellcheck.h

Project Member

Comment 3 by bugdroid1@chromium.org, May 29 2017

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

commit 10ae3c4b866897e7097ee9606d35b71f789e8532
Author: Noel Gordon <noel@chromium.org>
Date: Mon May 29 23:48:56 2017

Convert Spellcheck host MessageFilter IPC to mojo

Remove chrome/browser/spellchecker/spellcheck_message_filter and its
unittest. Replace it with a mojo implementation of same that exposes
an interface to renderers for calling spell-check-host API available
in the browser process:

  - RequestDictionary, NotifyChecked, CallSpellingService.

Update browser tests / unit tests to use the new mojo implementation
and add a mojom::SpellCheckHost unit test:

  - chrome_site_per_process_browsertest.cc
  - spellcheck_service_browsertest.cc
  - spellcheck_provider_hunspell_unittest.cc
  - spell_check_host_impl_unittest.cc

Bug:  714480 
Change-Id: I7e6a46245a908fe4600b1d3d86f2d9dc884f79ea
Reviewed-on: https://chromium-review.googlesource.com/505648
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: Johan Tibell <tibell@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#475395}
[modify] https://crrev.com/10ae3c4b866897e7097ee9606d35b71f789e8532/chrome/browser/BUILD.gn
[modify] https://crrev.com/10ae3c4b866897e7097ee9606d35b71f789e8532/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/10ae3c4b866897e7097ee9606d35b71f789e8532/chrome/browser/chrome_content_browser_manifest_overlay.json
[modify] https://crrev.com/10ae3c4b866897e7097ee9606d35b71f789e8532/chrome/browser/chrome_site_per_process_browsertest.cc
[add] https://crrev.com/10ae3c4b866897e7097ee9606d35b71f789e8532/chrome/browser/spellchecker/spell_check_host_impl.cc
[add] https://crrev.com/10ae3c4b866897e7097ee9606d35b71f789e8532/chrome/browser/spellchecker/spell_check_host_impl.h
[add] https://crrev.com/10ae3c4b866897e7097ee9606d35b71f789e8532/chrome/browser/spellchecker/spell_check_host_impl_unittest.cc
[delete] https://crrev.com/958dbb23d9fb6ef967f76d9cda0153a6cb05f8cd/chrome/browser/spellchecker/spellcheck_message_filter.cc
[delete] https://crrev.com/958dbb23d9fb6ef967f76d9cda0153a6cb05f8cd/chrome/browser/spellchecker/spellcheck_message_filter.h
[delete] https://crrev.com/958dbb23d9fb6ef967f76d9cda0153a6cb05f8cd/chrome/browser/spellchecker/spellcheck_message_filter_unittest.cc
[modify] https://crrev.com/10ae3c4b866897e7097ee9606d35b71f789e8532/chrome/browser/spellchecker/spellcheck_service_browsertest.cc
[modify] https://crrev.com/10ae3c4b866897e7097ee9606d35b71f789e8532/chrome/test/BUILD.gn
[modify] https://crrev.com/10ae3c4b866897e7097ee9606d35b71f789e8532/components/spellcheck/common/OWNERS
[modify] https://crrev.com/10ae3c4b866897e7097ee9606d35b71f789e8532/components/spellcheck/common/spellcheck.mojom
[add] https://crrev.com/10ae3c4b866897e7097ee9606d35b71f789e8532/components/spellcheck/common/spellcheck.typemap
[modify] https://crrev.com/10ae3c4b866897e7097ee9606d35b71f789e8532/components/spellcheck/common/spellcheck_messages.h
[modify] https://crrev.com/10ae3c4b866897e7097ee9606d35b71f789e8532/components/spellcheck/renderer/hunspell_engine.cc
[modify] https://crrev.com/10ae3c4b866897e7097ee9606d35b71f789e8532/components/spellcheck/renderer/spellcheck.cc
[modify] https://crrev.com/10ae3c4b866897e7097ee9606d35b71f789e8532/components/spellcheck/renderer/spellcheck.h
[modify] https://crrev.com/10ae3c4b866897e7097ee9606d35b71f789e8532/components/spellcheck/renderer/spellcheck_provider.cc
[modify] https://crrev.com/10ae3c4b866897e7097ee9606d35b71f789e8532/components/spellcheck/renderer/spellcheck_provider.h
[modify] https://crrev.com/10ae3c4b866897e7097ee9606d35b71f789e8532/components/spellcheck/renderer/spellcheck_provider_hunspell_unittest.cc
[modify] https://crrev.com/10ae3c4b866897e7097ee9606d35b71f789e8532/components/spellcheck/renderer/spellcheck_provider_test.cc
[modify] https://crrev.com/10ae3c4b866897e7097ee9606d35b71f789e8532/components/spellcheck/renderer/spellcheck_provider_test.h
[modify] https://crrev.com/10ae3c4b866897e7097ee9606d35b71f789e8532/components/typemaps.gni

Comment 4 by noel@chromium.org, May 30 2017

Related  bug 712395  re the OSX spelling panel.
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 15 2017

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

commit 08fdafe21e81711142065b8251af2e5ecd268707
Author: Noel Gordon <noel@chromium.org>
Date: Thu Jun 15 10:39:24 2017

Convert SpellCheckPanel SpellCheckPanelHost IPC to mojo

The spelling and grammar checking panel is an OSX feature [1],
available in Mac Chrome via BUILDFLAG(HAS_SPELLCHECK_PANEL),
used as a spell-correction aid for contentEditable, <input>
and <textarea>, and designMode document content that has the
focus on the current web page.

- Replace the spell check panel host and client chrome IPCs
  with mojo ( bug 714480 ).
- Move the spelling panel client interface from RenderView to
  RenderFrame, so that it works with OOPIF ( bug 712395 ).
- Update the browser Mac render view delegate to send spelling
  panel client messages to the focused RenderFrame.
- In Blink, move the WebSpellCheckClient off WebView and make
  it WebLocalFrame associated so it works with OOPIF.
- Rename it to WebSpellCheckPanelHostClient and remove its API
  from the SpellCheckerClientImpl class to address TODO.
- Make WebSpellCheckPanelHostClient pure virtual, add an empty
  impl of it to EmptyClients.h to satisfy LocalFrameClient.
- Add an OOPIF browser test to verify that the spelling panel
  can be shown from an OOPIF frame ( bug 712395 ).

[1] https://support.apple.com/kb/PH18451?locale=en_US

Bug:  714480 , 712395 
Change-Id: I964111685b2bf22bf54fd4e6e20f0861edf68b51
Reviewed-on: https://chromium-review.googlesource.com/515323
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Rachel Blum <groby@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Johan Tibell <tibell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479664}
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/chrome/browser/BUILD.gn
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/chrome/browser/chrome_content_browser_manifest_overlay.json
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/chrome/browser/chrome_content_renderer_manifest_overlay.json
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/chrome/browser/chrome_site_per_process_browsertest.cc
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/chrome/browser/renderer_host/chrome_render_widget_host_view_mac_delegate.mm
[add] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/chrome/browser/spellchecker/spell_check_panel_host_impl.cc
[add] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/chrome/browser/spellchecker/spell_check_panel_host_impl.h
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/chrome/browser/spellchecker/spellcheck_message_filter_platform_mac.cc
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/components/spellcheck/browser/spellcheck_message_filter_platform.h
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/components/spellcheck/browser/spellcheck_message_filter_platform_android.cc
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/components/spellcheck/common/BUILD.gn
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/components/spellcheck/common/spellcheck_messages.h
[add] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/components/spellcheck/common/spellcheck_panel.mojom
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/components/spellcheck/renderer/spellcheck_panel.cc
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/components/spellcheck/renderer/spellcheck_panel.h
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckRequester.cpp
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.h
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckerClient.h
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckerClientImpl.cpp
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckerClientImpl.h
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/third_party/WebKit/Source/core/exported/WebViewBase.h
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/third_party/WebKit/Source/core/frame/LocalFrameClient.h
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/third_party/WebKit/Source/core/loader/EmptyClients.h
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/third_party/WebKit/Source/web/LocalFrameClientImpl.cpp
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/third_party/WebKit/Source/web/LocalFrameClientImpl.h
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/third_party/WebKit/Source/web/WebViewImpl.cpp
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/third_party/WebKit/Source/web/WebViewImpl.h
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/third_party/WebKit/public/BUILD.gn
[add] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/third_party/WebKit/public/platform/WebSpellCheckPanelHostClient.h
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/third_party/WebKit/public/web/WebLocalFrame.h
[delete] https://crrev.com/56f1632a383ddb22bbbdf8b25000d36032ff1231/third_party/WebKit/public/web/WebSpellCheckClient.h
[modify] https://crrev.com/08fdafe21e81711142065b8251af2e5ecd268707/third_party/WebKit/public/web/WebView.h

Comment 6 by noel@chromium.org, Jun 16 2017

Cc: xiaoche...@chromium.org
xiaochengh@ the above patch fixed the spelling panel on Mac ( issue 712395 ) that you pointed out to me.
 
Took a lot longer time-wise to review than expected, and I need to move to other things.  Would you be willing to take over the current bug and mojo the last of the spelling IPC's?


Comment 7 by noel@chromium.org, Jun 16 2017

Cc: -xiaoche...@chromium.org noel@chromium.org
Owner: xiaoche...@chromium.org

Comment 8 by noel@chromium.org, Jun 16 2017

> Would you be willing to take over the current bug and mojo the last of the spelling IPC's?

Answer was yes as discussed in chat, FTR.  Thanks xiaochengh@ and please ping if you have questions or need help.

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 21 2017

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

commit f4ed16a5b5ec4d178ef71791b5ea82d7026cb26b
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Mon Aug 21 20:38:59 2017

Use mojo typemap for spellcheck mojom

This patch changes the spellcheck mojom from using IPC struct traits to
using mojo struct traits. This is a preparation step for migrating the
remaining spellcheck IPC to mojo.

Bug:  714480 
Change-Id: Ie09f1193ac3295b5a08c7e528e2680c1b11ff647
Reviewed-on: https://chromium-review.googlesource.com/618305
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496052}
[modify] https://crrev.com/f4ed16a5b5ec4d178ef71791b5ea82d7026cb26b/components/spellcheck/common/BUILD.gn
[modify] https://crrev.com/f4ed16a5b5ec4d178ef71791b5ea82d7026cb26b/components/spellcheck/common/OWNERS
[modify] https://crrev.com/f4ed16a5b5ec4d178ef71791b5ea82d7026cb26b/components/spellcheck/common/spellcheck.mojom
[modify] https://crrev.com/f4ed16a5b5ec4d178ef71791b5ea82d7026cb26b/components/spellcheck/common/spellcheck.typemap
[add] https://crrev.com/f4ed16a5b5ec4d178ef71791b5ea82d7026cb26b/components/spellcheck/common/spellcheck_struct_traits.cc
[add] https://crrev.com/f4ed16a5b5ec4d178ef71791b5ea82d7026cb26b/components/spellcheck/common/spellcheck_struct_traits.h

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 28 2017

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

commit b8c3418b5af02264abe0970ac50e6d0a7bbadd27
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Mon Aug 28 22:10:28 2017

Stop compiling Mac-only code in Android SpellCheckMessageFilterPlatform

This patch makes the above change to reduce binary size on Android.

It is also a preparation for mojofying remaining spellcheck messages.

Bug:  714480 
Change-Id: Iee061513ba40a0a6b9f898151540ad0e1d67c1b4
Reviewed-on: https://chromium-review.googlesource.com/637184
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497895}
[modify] https://crrev.com/b8c3418b5af02264abe0970ac50e6d0a7bbadd27/components/spellcheck/browser/spellcheck_message_filter_platform.h
[modify] https://crrev.com/b8c3418b5af02264abe0970ac50e6d0a7bbadd27/components/spellcheck/browser/spellcheck_message_filter_platform_android.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Jan 11 2018

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

commit 85abcd8b6add47e835386d0e8f32d91c8c0d39a7
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Thu Jan 11 02:36:56 2018

Rename SpellCheckHostImpl to SpellCheckHostChromeImpl

This patch renames class SpellCheckHostImpl to
SpellCheckHostChromeImpl.

The purpose of the renaming is to clear an obstacle in the
mojofication of Android spellcheck messages: Android WebView
should use an impl of SpellCheckHost instead of IPC, but
SpellCheckHostImpl is in chrome/, and can't be componentized
due to use of other chrome/ classes.

In followup patches, we will:
- Add a new SpellCheckHostImpl in components/, and make
  SpellCheckHostChromeImpl a subclass of it
- Implement Android-specific APIs of SpellCheckHost in
  SpellCHeckHostImpl (in components/)

Bug:  714480 
Change-Id: Ib53e8aca3b8ff48c9367659c716c85c8b10430e8
Reviewed-on: https://chromium-review.googlesource.com/639431
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Rachel Blum <groby@chromium.org>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528537}
[modify] https://crrev.com/85abcd8b6add47e835386d0e8f32d91c8c0d39a7/chrome/browser/BUILD.gn
[modify] https://crrev.com/85abcd8b6add47e835386d0e8f32d91c8c0d39a7/chrome/browser/chrome_service.cc
[rename] https://crrev.com/85abcd8b6add47e835386d0e8f32d91c8c0d39a7/chrome/browser/spellchecker/spell_check_host_chrome_impl.cc
[rename] https://crrev.com/85abcd8b6add47e835386d0e8f32d91c8c0d39a7/chrome/browser/spellchecker/spell_check_host_chrome_impl.h
[rename] https://crrev.com/85abcd8b6add47e835386d0e8f32d91c8c0d39a7/chrome/browser/spellchecker/spell_check_host_chrome_impl_unittest.cc
[modify] https://crrev.com/85abcd8b6add47e835386d0e8f32d91c8c0d39a7/chrome/browser/spellchecker/spellcheck_service_browsertest.cc
[modify] https://crrev.com/85abcd8b6add47e835386d0e8f32d91c8c0d39a7/chrome/test/BUILD.gn

Project Member

Comment 12 by bugdroid1@chromium.org, Feb 16 2018

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

commit 3125666425d0bee78d5172da57351d89a89b7d0b
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Fri Feb 16 03:02:47 2018

Use a dummy LocalInterfaceProvider for spellcheck test code

Several spellcheck classes hold a LocalInterfaceProvider pointer,
which is always non-null in production code, but can be null in unit
tests. This makes the production code performing unnecessary nullptr
checks and doing hacky things.

This patch adds a dummy LocalInterfaceProvider that doesn't bind any
remote application, and uses it in test code, so that spellcheck
production code no longer needs to perform nullptr checks.

Bug:  714480 
Change-Id: I30a9c82efd4298a62bda52d357fcfa9a6c6d991b
Reviewed-on: https://chromium-review.googlesource.com/910650
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537163}
[modify] https://crrev.com/3125666425d0bee78d5172da57351d89a89b7d0b/components/spellcheck/renderer/BUILD.gn
[add] https://crrev.com/3125666425d0bee78d5172da57351d89a89b7d0b/components/spellcheck/renderer/empty_local_interface_provider.cc
[add] https://crrev.com/3125666425d0bee78d5172da57351d89a89b7d0b/components/spellcheck/renderer/empty_local_interface_provider.h
[modify] https://crrev.com/3125666425d0bee78d5172da57351d89a89b7d0b/components/spellcheck/renderer/hunspell_engine.cc
[modify] https://crrev.com/3125666425d0bee78d5172da57351d89a89b7d0b/components/spellcheck/renderer/platform_spelling_engine.cc
[modify] https://crrev.com/3125666425d0bee78d5172da57351d89a89b7d0b/components/spellcheck/renderer/spellcheck.cc
[modify] https://crrev.com/3125666425d0bee78d5172da57351d89a89b7d0b/components/spellcheck/renderer/spellcheck_multilingual_unittest.cc
[modify] https://crrev.com/3125666425d0bee78d5172da57351d89a89b7d0b/components/spellcheck/renderer/spellcheck_panel.cc
[modify] https://crrev.com/3125666425d0bee78d5172da57351d89a89b7d0b/components/spellcheck/renderer/spellcheck_provider.cc
[modify] https://crrev.com/3125666425d0bee78d5172da57351d89a89b7d0b/components/spellcheck/renderer/spellcheck_provider_test.cc
[modify] https://crrev.com/3125666425d0bee78d5172da57351d89a89b7d0b/components/spellcheck/renderer/spellcheck_provider_test.h
[modify] https://crrev.com/3125666425d0bee78d5172da57351d89a89b7d0b/components/spellcheck/renderer/spellcheck_unittest.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Feb 16 2018

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

commit d9085ee1dd4bcd9907ffc348679bd5bb2628e631
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Fri Feb 16 22:32:30 2018

Mojofy spellcheck messages

Currently, spellchecker uses mojo only on Linux and Window, and still
uses IPC on Mac and Android. This patch finishes the mojofication on
Mac and Android.

Details:

1. For each spellcheck message, a new API in SpellCheckHost mojom
   is defined. Their original senders are changed to call the new
   mojo APIs.

2. To allow spellchecking on WebView, a new impl of SpellCheckHost
   named SpellCheckHostImpl is created in components/spellcheck. The
   existing impl SpellCheckHostChromeImpl is changed to inherit
   SpellCheckHostImpl for code sharing.

3. The existing chrome/browser/spellcheck_message_filter_platform_mac.cc
   is changed to Mac-only implementation of SpellCheckHostChromeImpl.

4. The existing message filter implementation on Android, which is
   basically a wrapper class of SpellCheckerSessionBrige, is replaced
   by new wrapping of SpellCheckerSessionBridge in the Android-only
   implementation of SpellCheckHostImpl.

5. Remaining changes are test changes.

To reduce patch size, there are some leftovers that will be done
by follow-up patches:
- Remove the spellcheck message files
- Rename certain files according to TODOs left in this patch

Bug:  714480 
Change-Id: I1a25307284b8973f0b39cae58dfa46b27232349a
Reviewed-on: https://chromium-review.googlesource.com/846370
Reviewed-by: Martin Barbella <mbarbella@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Tim Volodine <timvolodine@chromium.org>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537434}
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/android_webview/browser/aw_browser_manifest_overlay.json
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/android_webview/browser/aw_content_browser_client.cc
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/android_webview/renderer/aw_content_renderer_client.cc
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/chrome/browser/chrome_site_per_process_browsertest.cc
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/chrome/browser/spellchecker/spell_check_host_chrome_impl.cc
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/chrome/browser/spellchecker/spell_check_host_chrome_impl.h
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/chrome/browser/spellchecker/spellcheck_message_filter_platform_mac.cc
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/chrome/browser/spellchecker/spellcheck_message_filter_platform_mac_browsertest.cc
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/chrome/browser/spellchecker/spellcheck_message_filter_platform_mac_unittest.cc
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/components/spellcheck/browser/BUILD.gn
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/components/spellcheck/browser/DEPS
[add] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/components/spellcheck/browser/spell_check_host_impl.cc
[add] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/components/spellcheck/browser/spell_check_host_impl.h
[delete] https://crrev.com/3d7e2b409e4abdf6a7ba879a0135d4c168609908/components/spellcheck/browser/spellcheck_message_filter_platform.h
[delete] https://crrev.com/3d7e2b409e4abdf6a7ba879a0135d4c168609908/components/spellcheck/browser/spellcheck_message_filter_platform_android.cc
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/components/spellcheck/browser/spellcheck_platform.h
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/components/spellcheck/browser/spellcheck_platform_mac.mm
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/components/spellcheck/browser/spellchecker_session_bridge_android.cc
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/components/spellcheck/browser/spellchecker_session_bridge_android.h
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/components/spellcheck/common/BUILD.gn
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/components/spellcheck/common/spellcheck.mojom
[delete] https://crrev.com/3d7e2b409e4abdf6a7ba879a0135d4c168609908/components/spellcheck/common/spellcheck_message_generator.cc
[delete] https://crrev.com/3d7e2b409e4abdf6a7ba879a0135d4c168609908/components/spellcheck/common/spellcheck_message_generator.h
[delete] https://crrev.com/3d7e2b409e4abdf6a7ba879a0135d4c168609908/components/spellcheck/common/spellcheck_messages.h
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/components/spellcheck/common/spellcheck_result.h
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/components/spellcheck/renderer/platform_spelling_engine.cc
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/components/spellcheck/renderer/platform_spelling_engine.h
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/components/spellcheck/renderer/spellcheck_panel.cc
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/components/spellcheck/renderer/spellcheck_provider.cc
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/components/spellcheck/renderer/spellcheck_provider.h
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/components/spellcheck/renderer/spellcheck_provider_mac_unittest.cc
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/components/spellcheck/renderer/spellcheck_provider_test.cc
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/components/spellcheck/renderer/spellcheck_provider_test.h
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/tools/ipc_fuzzer/message_lib/all_messages.h
[modify] https://crrev.com/d9085ee1dd4bcd9907ffc348679bd5bb2628e631/tools/ipc_fuzzer/message_tools/message_list.cc

Project Member

Comment 14 by bugdroid1@chromium.org, Feb 16 2018

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

commit 384fe0f5b0c591cbd364c934c4d9b1c9846a121d
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Fri Feb 16 23:46:56 2018

Rename spellcheck filter files to spellcheck host files

Spellcheck was mojofied in crrev.com/c/846370, but relevant files
are left without being renamed, so that we don't break git revision
log. This patch finishes the renaming.

Bug:  714480 
Change-Id: If6df9357871a4a2e353f3ab336878d3be891c596
Reviewed-on: https://chromium-review.googlesource.com/898529
Reviewed-by: Rachel Blum <groby@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537461}
[modify] https://crrev.com/384fe0f5b0c591cbd364c934c4d9b1c9846a121d/chrome/browser/BUILD.gn
[rename] https://crrev.com/384fe0f5b0c591cbd364c934c4d9b1c9846a121d/chrome/browser/spellchecker/spell_check_host_chrome_impl_mac.cc
[rename] https://crrev.com/384fe0f5b0c591cbd364c934c4d9b1c9846a121d/chrome/browser/spellchecker/spell_check_host_chrome_impl_mac_browsertest.cc
[rename] https://crrev.com/384fe0f5b0c591cbd364c934c4d9b1c9846a121d/chrome/browser/spellchecker/spell_check_host_chrome_impl_mac_unittest.cc
[modify] https://crrev.com/384fe0f5b0c591cbd364c934c4d9b1c9846a121d/chrome/test/BUILD.gn

Status: Fixed (was: Started)

Sign in to add a comment