Chromium Code Reviews
Help | Chromium Project | Sign in
(424)

Issue 12096116: Enable dictionary sync by default (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 2 months ago by Rouslan Solomakhin
Modified:
1 year, 2 months ago
Reviewers:
groby, Nicolas Zea
CC:
chromium-reviews_chromium.org, Raghu Simha, haitaol1, akalin, timsteele, rpetterson
Base URL:
http://git.chromium.org/chromium/src.git@master
Visibility:
Public.

Description

Enable dictionary sync by default

This CL enables dictionary sync by default on all platforms except Android and
Mac, which do not use the Chrome custom spelling dictionary file.

BUG=72910
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=181430

Patch Set 1 #

Patch Set 2 : Add --disable-sync-dictionary flag #

Patch Set 3 : Remove experiment from about flags #

Total comments: 2

Patch Set 4 : Explicit OS check #

Patch Set 5 : Fix unit test #

Patch Set 6 : Disable dictionary sync integration tests on Mac #

Patch Set 7 : Fix unittest on mac #

Patch Set 8 : Merge master #

Patch Set 9 : Fix win_rel bot #

Patch Set 10 : Merge master #

Unified diffs Side-by-side diffs Delta from patch set Stats (+30 lines, -45 lines) Lint Patch
M chrome/app/generated_resources.grd View 1 chunk +0 lines, -6 lines 0 comments ? errors Download
M chrome/browser/about_flags.cc View 1 2 3 4 5 6 7 8 9 1 chunk +0 lines, -7 lines 0 comments ? errors Download
M chrome/browser/spellchecker/spellcheck_service.cc View 1 2 3 4 5 6 7 8 1 chunk +1 line, -1 line 0 comments ? errors Download
M chrome/browser/sync/profile_sync_components_factory_impl.cc View 1 2 3 4 5 6 7 8 1 chunk +4 lines, -3 lines 0 comments 0 errors Download
M chrome/browser/sync/profile_sync_components_factory_impl_unittest.cc View 1 2 3 4 5 6 1 chunk +3 lines, -0 lines 0 comments 0 errors Download
M chrome/browser/sync/test/integration/dictionary_helper.h View 1 chunk +0 lines, -3 lines 0 comments ? errors Download
M chrome/browser/sync/test/integration/dictionary_helper.cc View 1 chunk +0 lines, -5 lines 0 comments ? errors Download
M chrome/browser/sync/test/integration/multiple_client_dictionary_sync_test.cc View 1 chunk +0 lines, -4 lines 0 comments ? errors Download
M chrome/browser/sync/test/integration/performance/dictionary_sync_perf_test.cc View 1 chunk +0 lines, -4 lines 0 comments ? errors Download
M chrome/browser/sync/test/integration/single_client_dictionary_sync_test.cc View 1 chunk +0 lines, -4 lines 0 comments ? errors Download
M chrome/browser/sync/test/integration/two_client_dictionary_sync_test.cc View 1 chunk +0 lines, -4 lines 0 comments ? errors Download
M chrome/chrome_tests.gypi View 1 2 3 4 5 6 7 8 9 2 chunks +18 lines, -0 lines 0 comments ? errors Download
M chrome/common/chrome_switches.h View 1 2 3 4 5 6 7 8 2 chunks +1 line, -1 line 0 comments ? errors Download
M chrome/common/chrome_switches.cc View 1 2 3 4 5 6 7 8 2 chunks +3 lines, -3 lines 0 comments ? errors Download
Commit:

Messages

Total messages: 19
Rouslan Solomakhin
Groby: PTAL
1 year, 2 months ago #1
Rouslan Solomakhin
Zea: Is this the correct way to avoid registering the dictionary sync data type on ...
1 year, 2 months ago #2
Nicolas Zea
On 2013/02/01 19:09:40, Rouslan Solomakhin wrote: > Zea: Is this the correct way to avoid ...
1 year, 2 months ago #3
Rouslan Solomakhin
Nicolas: I've added --disable-sync-dictionary flag. PTAL.
1 year, 2 months ago #4
Nicolas Zea
I'd say you don't need to keep an experiment in about flags, the chrome switch ...
1 year, 2 months ago #5
Rouslan Solomakhin
On 2013/02/01 19:59:09, Nicolas Zea wrote: > I'd say you don't need to keep an ...
1 year, 2 months ago #6
groby
LGTM minus one nit https://codereview.chromium.org/12096116/diff/8002/chrome/browser/sync/profile_sync_components_factory_impl.cc File chrome/browser/sync/profile_sync_components_factory_impl.cc (right): https://codereview.chromium.org/12096116/diff/8002/chrome/browser/sync/profile_sync_components_factory_impl.cc#newcode238 chrome/browser/sync/profile_sync_components_factory_impl.cc:238: #if !defined(OS_ANDROID) && !defined(OS_MACOSX) I'd ...
1 year, 2 months ago #7
Rouslan Solomakhin
https://codereview.chromium.org/12096116/diff/8002/chrome/browser/sync/profile_sync_components_factory_impl.cc File chrome/browser/sync/profile_sync_components_factory_impl.cc (right): https://codereview.chromium.org/12096116/diff/8002/chrome/browser/sync/profile_sync_components_factory_impl.cc#newcode238 chrome/browser/sync/profile_sync_components_factory_impl.cc:238: #if !defined(OS_ANDROID) && !defined(OS_MACOSX) On 2013/02/01 22:27:56, groby wrote: ...
1 year, 2 months ago #8
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rouslan@chromium.org/12096116/19001
1 year, 2 months ago #9
I haz the power (commit-bot)
Retried try job too often on linux_aura for step(s) unit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura&number=13952
1 year, 2 months ago #10
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rouslan@chromium.org/12096116/24001
1 year, 2 months ago #11
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rouslan@chromium.org/12096116/30016
1 year, 2 months ago #12
I haz the power (commit-bot)
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
1 year, 2 months ago #13
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rouslan@chromium.org/12096116/42002
1 year, 2 months ago #14
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rouslan@chromium.org/12096116/42002
1 year, 2 months ago #15
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rouslan@chromium.org/12096116/50001
1 year, 2 months ago #16
I haz the power (commit-bot)
Failed to apply patch for chrome/app/generated_resources.grd: While running patch -p1 --forward --force --no-backup-if-mismatch; patching file ...
1 year, 2 months ago #17
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rouslan@chromium.org/12096116/53001
1 year, 2 months ago #18
I haz the power (commit-bot)
1 year, 2 months ago #19
Message was sent while issue was closed.
Change committed as 181430
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6