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

Issue 12096116: Enable dictionary sync by default (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 1 month ago by Rouslan (back on April 10)
Modified:
2 years, 1 month ago
Reviewers:
groby, Nicolas Zea
CC:
chromium-reviews, 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) Patch
M chrome/app/generated_resources.grd View 1 chunk +0 lines, -6 lines 0 comments 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 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 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 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 Download
M chrome/browser/sync/test/integration/dictionary_helper.h View 1 chunk +0 lines, -3 lines 0 comments Download
M chrome/browser/sync/test/integration/dictionary_helper.cc View 1 chunk +0 lines, -5 lines 0 comments Download
M chrome/browser/sync/test/integration/multiple_client_dictionary_sync_test.cc View 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/sync/test/integration/performance/dictionary_sync_perf_test.cc View 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/sync/test/integration/single_client_dictionary_sync_test.cc View 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/sync/test/integration/two_client_dictionary_sync_test.cc View 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/chrome_tests.gypi View 1 2 3 4 5 6 7 8 9 2 chunks +18 lines, -0 lines 0 comments Download
M chrome/common/chrome_switches.h View 1 2 3 4 5 6 7 8 2 chunks +1 line, -1 line 0 comments Download
M chrome/common/chrome_switches.cc View 1 2 3 4 5 6 7 8 2 chunks +3 lines, -3 lines 0 comments Download
Commit: CQ not working?

Messages

Total messages: 19 (0 generated)
Rouslan (back on April 10)
Groby: PTAL
2 years, 1 month ago (2013-02-01 19:08:40 UTC) #1
Rouslan (back on April 10)
Zea: Is this the correct way to avoid registering the dictionary sync data type on ...
2 years, 1 month ago (2013-02-01 19:09:40 UTC) #2
Nicolas Zea
On 2013/02/01 19:09:40, Rouslan Solomakhin wrote: > Zea: Is this the correct way to avoid ...
2 years, 1 month ago (2013-02-01 19:13:14 UTC) #3
Rouslan (back on April 10)
Nicolas: I've added --disable-sync-dictionary flag. PTAL.
2 years, 1 month ago (2013-02-01 19:56:06 UTC) #4
Nicolas Zea
I'd say you don't need to keep an experiment in about flags, the chrome switch ...
2 years, 1 month ago (2013-02-01 19:59:09 UTC) #5
Rouslan (back on April 10)
On 2013/02/01 19:59:09, Nicolas Zea wrote: > I'd say you don't need to keep an ...
2 years, 1 month ago (2013-02-01 20:21:08 UTC) #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 ...
2 years, 1 month ago (2013-02-01 22:27:56 UTC) #7
Rouslan (back on April 10)
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: ...
2 years, 1 month ago (2013-02-01 22:34:29 UTC) #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
2 years, 1 month ago (2013-02-01 22:35:49 UTC) #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
2 years, 1 month ago (2013-02-01 23:21:50 UTC) #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
2 years, 1 month ago (2013-02-04 17:32:22 UTC) #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
2 years, 1 month ago (2013-02-04 20:51:41 UTC) #12
I haz the power (commit-bot)
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
2 years, 1 month ago (2013-02-04 21:32:18 UTC) #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
2 years, 1 month ago (2013-02-04 22:13:57 UTC) #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
2 years, 1 month ago (2013-02-05 16:35:56 UTC) #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
2 years, 1 month ago (2013-02-08 02:17:45 UTC) #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 ...
2 years, 1 month ago (2013-02-08 02:17:49 UTC) #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
2 years, 1 month ago (2013-02-08 02:24:29 UTC) #18
I haz the power (commit-bot)
2 years, 1 month ago (2013-02-08 05:21:37 UTC) #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 cf4c24d