Added Instant preference syncing and tests.

Added Instant preference syncing and tests.

BUG=77207
TEST=Enabled Instant checkbox in one browser instance with synced profile.
  Launch another instance of the browser with the same profile and ensure
  the checkbox has been properly changed.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=101568

[csharp, 2011-09-14 20:13:22]

Changelist to add the instant preference option to preference sync

http://codereview.chromium.org/7888047/diff/1/chrome/browser/sync/test/integr...
File chrome/browser/sync/test/integration/two_client_preferences_sync_test.cc
(right):

http://codereview.chromium.org/7888047/diff/1/chrome/browser/sync/test/integr...
chrome/browser/sync/test/integration/two_client_preferences_sync_test.cc:180:
IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, kEnableInstant) {
I'm not sure if this test is correctly setup, I just tried to follow the style
of the other tests.
Also, prefs::kInstanceEnabledTime isn't tested because I could find the int64
versions of PrefMatches and Change*Pref, do they exist?

[Nicolas Zea, 2011-09-14 21:32:12]

http://codereview.chromium.org/7888047/diff/1/chrome/browser/instant/instant_...
File chrome/browser/instant/instant_controller.cc (right):

http://codereview.chromium.org/7888047/diff/1/chrome/browser/instant/instant_...
chrome/browser/instant/instant_controller.cc:95: PrefService::SYNCABLE_PREF);
Have you verified that this value is cross-platform compatible? (all platforms
will understand it the same way?)

http://codereview.chromium.org/7888047/diff/1/chrome/browser/sync/test/integr...
File chrome/browser/sync/test/integration/two_client_preferences_sync_test.cc
(right):

http://codereview.chromium.org/7888047/diff/1/chrome/browser/sync/test/integr...
chrome/browser/sync/test/integration/two_client_preferences_sync_test.cc:180:
IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, kEnableInstant) {
On 2011/09/14 20:13:22, csharp wrote:
> I'm not sure if this test is correctly setup, I just tried to follow the style
> of the other tests.
> Also, prefs::kInstanceEnabledTime isn't tested because I could find the int64
> versions of PrefMatches and Change*Pref, do they exist?
> 

They do not yet, feel free to add them though. See
chrome/browser/sync/test/integration/preferences_helper.[h,cc] for examples

[csharp, 2011-09-15 13:39:12]

http://codereview.chromium.org/7888047/diff/1/chrome/browser/instant/instant_...
File chrome/browser/instant/instant_controller.cc (right):

http://codereview.chromium.org/7888047/diff/1/chrome/browser/instant/instant_...
chrome/browser/instant/instant_controller.cc:95: PrefService::SYNCABLE_PREF);
I have not, I had thought that it would be cross-platform compatible because
this is the same method used for other preferences (which I assumed are
cross-platform).

What should I read or do to ensure compatible?

On 2011/09/14 21:32:12, nzea wrote:
> Have you verified that this value is cross-platform compatible? (all platforms
> will understand it the same way?)

http://codereview.chromium.org/7888047/diff/1/chrome/browser/sync/test/integr...
File chrome/browser/sync/test/integration/two_client_preferences_sync_test.cc
(right):

http://codereview.chromium.org/7888047/diff/1/chrome/browser/sync/test/integr...
chrome/browser/sync/test/integration/two_client_preferences_sync_test.cc:180:
IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, kEnableInstant) {
They have been added.

On 2011/09/14 21:32:12, nzea wrote:
> On 2011/09/14 20:13:22, csharp wrote:
> > I'm not sure if this test is correctly setup, I just tried to follow the
style
> > of the other tests.
> > Also, prefs::kInstanceEnabledTime isn't tested because I could find the
int64
> > versions of PrefMatches and Change*Pref, do they exist?
> > 
> 
> They do not yet, feel free to add them though. See
> chrome/browser/sync/test/integration/preferences_helper.[h,cc] for examples

[Nicolas Zea, 2011-09-15 19:48:57]

LGTM with sync trybot runs (linux_sync, win_sync, mac_sync)

http://codereview.chromium.org/7888047/diff/1/chrome/browser/instant/instant_...
File chrome/browser/instant/instant_controller.cc (right):

http://codereview.chromium.org/7888047/diff/1/chrome/browser/instant/instant_...
chrome/browser/instant/instant_controller.cc:95: PrefService::SYNCABLE_PREF);
On 2011/09/15 13:39:12, csharp wrote:
> I have not, I had thought that it would be cross-platform compatible because
> this is the same method used for other preferences (which I assumed are
> cross-platform).
> 
> What should I read or do to ensure compatible?
> 
> On 2011/09/14 21:32:12, nzea wrote:
> > Have you verified that this value is cross-platform compatible? (all
platforms
> > will understand it the same way?)
> 

Looking below, it's just a base::Time::Now().ToInternalValue. Just for the sake
of thoroughness, it would be good for you to get familiar with the base/time.h
format. Offhand I'm pretty sure we store everything internally using the windows
epoch, no matter the platform, so it should be fine.

[csharp, 2011-09-16 17:58:10]

Patch submitted to try-bots twice.
First time was submitted by stevet@chromium.org and ran against all bots, only
error was on mac, error was related to nacl.
Problem was:
[SRPC:HOST:17857,2694382912:16:26:53.102655] NaClSrpcRpcWait(channel=0x1599f0):
EOF is received instead of response. Probably, the other side (usually, nacl
module or browser plugin) crashed.
[17856:2307:8306338677883:INFO:CONSOLE(1)] "NativeClient: NaCl module load
failed: ServiceRuntime: command channel creation failed", source:  (1)

[17856:2307:8306347519947:INFO:CONSOLE(1)] "NativeClient: Log message that
should show up on the JavaScript console", source:  (1)

Without any changes the patch was submitted a second time by mad@chromium.org
for mac-sync, win-sync, linux-sync and mac. All tests passed this time.

[Nicolas Zea, 2011-09-16 17:59:48]

Cool, LGTM

[commit-bot: I haz the power, 2011-09-16 21:07:31]

Change committed as 101568