Comment #4 on issue 25677 by [email protected]: move spellchecker to  
renderer
http://code.google.com/p/chromium/issues/detail?id=25677

The following revision refers to this bug:
     http://src.chromium.org/viewvc/chrome?view=rev&revision=31199

------------------------------------------------------------------------
r31199 | [email protected] | 2009-11-05 19:05:46 -0800 (Thu, 05 Nov 2009)  
| 28 lines
Changed paths:
    M  
http://src.chromium.org/viewvc/chrome/trunk/src/base/file_util.h?r1=31199&r2=31198
    M  
http://src.chromium.org/viewvc/chrome/trunk/src/base/file_util_posix.cc?r1=31199&r2=31198
    M  
http://src.chromium.org/viewvc/chrome/trunk/src/build/common.gypi?r1=31199&r2=31198
    M  
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/automation/automation_profile_impl.h?r1=31199&r2=31198
    M  
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/profile.cc?r1=31199&r2=31198
    M  
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/profile.h?r1=31199&r2=31198
    M  
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/browser_render_process_host.cc?r1=31199&r2=31198
    M  
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/browser_render_process_host.h?r1=31199&r2=31198
    A  
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/spellcheck_host.cc
    A  
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/spellcheck_host.h
    M  
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/spellchecker.cc?r1=31199&r2=31198
    M  
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/spellchecker_mac.mm?r1=31199&r2=31198
    M  
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/tab_contents/render_view_context_menu.cc?r1=31199&r2=31198
    M  
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/chrome.gyp?r1=31199&r2=31198
    M  
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/notification_type.h?r1=31199&r2=31198
    M  
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/render_messages_internal.h?r1=31199&r2=31198
    M  
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/renderer/render_thread.cc?r1=31199&r2=31198
    M  
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/renderer/render_thread.h?r1=31199&r2=31198
    M  
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/renderer/render_view.cc?r1=31199&r2=31198
    A  
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/renderer/spellchecker
    A  
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/renderer/spellchecker/spellcheck.cc
    A  
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/renderer/spellchecker/spellcheck.h
    A  
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/renderer/spellchecker/spellcheck_worditerator.cc
    A  
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/renderer/spellchecker/spellcheck_worditerator.h
    M  
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/test/testing_profile.h?r1=31199&r2=31198

Move the spellchecker to the renderer.

The motivation is that this removes the sync IPC on every call to the  
spellchecker. Also, currently we spellcheck in the IO thread, which  
frequently needs to go to disk (in particular, the entire spellcheck  
dictionary starts paged out), so this will block just the single renderer  
when that happens, rather than the whole IO thread.

This breaks the SpellChecker class into two new classes.
1) On the browser side, we have SpellCheckHost. This class handles  
browser-wide tasks, such as keeping the custom words list in sync with the  
on-disk custom words dictionary, downloading missing dictionaries, etc. On  
Posix, it also opens the bdic file since the renderer isn't allowed to open  
files. SpellCheckHost is created and destroyed on the UI thread. It is  
initialized on the file thread.
2) On the renderer side, SpellChecker2. This class will one day be renamed  
SpellChecker. It handles actual checking of the words, memory maps the  
dictionary file, loads hunspell, etc. There is one SpellChecker2 per  
RenderThread (hence one per render process).

My intention is for this patch to move Linux to this new approach, and  
follow up with ports for Windows (which will involve passing a dictionary  
file name rather than a file descriptor through to the renderer) and Mac  
(which will involve adding sync ViewHost IPC callsfor when the platform  
spellchecker is enabled). Note that anyone using the platform spellchecker  
rather than Hunspell will get no benefit out of this refactor.

There should be no loss of functionality for Linux (or any other platform)  
in this patch. The following should all still work:
- dictionary is loaded lazily
- hunspell is initialized lazily, per renderer
- language changes work.
- Dynamic downloading of new dictionaries
- auto spell correct works (as well as toggling it).
- disabling spellcheck works.
- custom words work (including adding in one renderer and immediately  
having it take effect in other renderers, for certain values of "immediate")

TODO:
- move spellchecker unit tests to test SpellCheck2
- add sync IPC for platform spellchecker; port to Mac
- add dictionary location fallback; port to Windows
- remove SpellChecker classes from browser/

BUG=25677

Review URL: http://codereview.chromium.org/357003
------------------------------------------------------------------------


--
You received this message because you are listed in the owner
or CC fields of this issue, or because you starred this issue.
You may adjust your issue notification preferences at:
http://code.google.com/hosting/settings

--~--~---------~--~----~------------~-------~--~----~
Automated mail from issue updates at http://crbug.com/
Subscription options: http://groups.google.com/group/chromium-bugs
-~----------~----~----~----~------~----~------~--~---

Reply via email to