New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 635680 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug

Blocking:
issue 643262



Sign in to add a comment

Enable new VariationsService URL for Chrome Android

Project Member Reported by asvitk...@chromium.org, Aug 8 2016

Issue description

Enable new VariationsService URL for Chrome Android
 
Cc: k...@chromium.org
ktam@, I have the CL ready for this. Let me know when I can start testing (i.e. when the new URL has been pushed.) Thanks!

Comment 2 by k...@chromium.org, Aug 12 2016

Great thanks. I will let you know if/once we get approved/DNS routes get set up.
Labels: -M-54 M-55
Moving to M55 per chat with ktam@.
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 31 2016

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

commit 1078363787edb725a21dd94993a1b8fdacd346cb
Author: asvitkine <asvitkine@chromium.org>
Date: Wed Aug 31 20:58:07 2016

Change variations service domain for Android Chrome.

The new domain is clientservices.googleapis.com.

Also, removes testing config for VarationsServiceControl
30m interval trial, since that's the client default.
Updates the parameter code for the fetch interval to fix
the typo in the trial name.

Note: Should not be submitted until we verify that the new
URL is working.

BUG= 635680 

Review-Url: https://codereview.chromium.org/2223263002
Cr-Commit-Position: refs/heads/master@{#415750}

[modify] https://crrev.com/1078363787edb725a21dd94993a1b8fdacd346cb/components/variations/variations_request_scheduler.cc
[modify] https://crrev.com/1078363787edb725a21dd94993a1b8fdacd346cb/components/variations/variations_url_constants.cc
[modify] https://crrev.com/1078363787edb725a21dd94993a1b8fdacd346cb/testing/variations/fieldtrial_testing_config_android.json
[modify] https://crrev.com/1078363787edb725a21dd94993a1b8fdacd346cb/testing/variations/fieldtrial_testing_config_chromeos.json
[modify] https://crrev.com/1078363787edb725a21dd94993a1b8fdacd346cb/testing/variations/fieldtrial_testing_config_ios.json
[modify] https://crrev.com/1078363787edb725a21dd94993a1b8fdacd346cb/testing/variations/fieldtrial_testing_config_linux.json
[modify] https://crrev.com/1078363787edb725a21dd94993a1b8fdacd346cb/testing/variations/fieldtrial_testing_config_mac.json
[modify] https://crrev.com/1078363787edb725a21dd94993a1b8fdacd346cb/testing/variations/fieldtrial_testing_config_win.json

Status: Fixed (was: Started)

Comment 6 by k...@chromium.org, Sep 1 2016

Blocking: 643262
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 18 2016

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

commit 04519a516180a807b2afde7c51156951753fceff
Author: asvitkine <asvitkine@chromium.org>
Date: Tue Oct 18 21:38:43 2016

Change variations service URL on the Java side.

This codepath is taken during first run seed fetching.
In https://codereview.chromium.org/2223263002, I changed
the URL of the regular seed fetch for Android, but forgot
to also change this one.

BUG= 635680 

Review-Url: https://codereview.chromium.org/2433453002
Cr-Commit-Position: refs/heads/master@{#426059}

[modify] https://crrev.com/04519a516180a807b2afde7c51156951753fceff/components/variations/android/java/src/org/chromium/components/variations/firstrun/VariationsSeedService.java

Comment 8 by k...@chromium.org, Oct 18 2016

Will this affect first-run? If so, can we merge to M-55? Would be good to ensure startup experience works.
I don't think it's worth merging this part - since this affects the first run support where Chrome is pre-installed on the device and the user goes through the device set up wizard on a new device.

It's a pretty unlikely case and more support will come under crbug.com/632199, but let me know if you disagree.

Comment 10 by k...@chromium.org, Oct 19 2016

Ah OK. If it's only for pre-installed, I think we're fine.
Status: Started (was: Fixed)
Appears that https://codereview.chromium.org/2223263002 didn't work as expected because build_config.h wasn't included - which is required for the defined(OS_ANDROID) check.

Landing a CL to fix.
Project Member

Comment 12 by bugdroid1@chromium.org, Nov 5 2016

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

commit 0292659d45ce31e698ea8329f8aef66c8ac1c9e0
Author: asvitkine <asvitkine@chromium.org>
Date: Sat Nov 05 02:38:02 2016

Include build_config.h in variations_url_constants.cc.

The file checks #if defined(OS_ANDROID), but that's not set
unless build_config.h is included.

This change should result in the URL for variations seed
fetches being actually changed for Android Chrome.

BUG= 635680 

Review-Url: https://codereview.chromium.org/2474313005
Cr-Commit-Position: refs/heads/master@{#430129}

[modify] https://crrev.com/0292659d45ce31e698ea8329f8aef66c8ac1c9e0/components/variations/variations_url_constants.cc

Labels: Merge-Request-55
Requesting merge of https://codereview.chromium.org/2474313005 to M55.

Comment 14 by dimu@chromium.org, Nov 7 2016

Labels: -Merge-Request-55 Merge-Approved-55 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M55 (branch: 2883)
Project Member

Comment 15 by bugdroid1@chromium.org, Nov 7 2016

Labels: -merge-approved-55 merge-merged-2883
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/58621a9010d93459e681e634068b225b0f5fceb5

commit 58621a9010d93459e681e634068b225b0f5fceb5
Author: Alexei Svitkine <asvitkine@chromium.org>
Date: Mon Nov 07 18:00:28 2016

Include build_config.h in variations_url_constants.cc.

The file checks #if defined(OS_ANDROID), but that's not set
unless build_config.h is included.

This change should result in the URL for variations seed
fetches being actually changed for Android Chrome.

BUG= 635680 

Review-Url: https://codereview.chromium.org/2474313005
Cr-Commit-Position: refs/heads/master@{#430129}
(cherry picked from commit 0292659d45ce31e698ea8329f8aef66c8ac1c9e0)

Review URL: https://codereview.chromium.org/2482943002 .

Cr-Commit-Position: refs/branch-heads/2883@{#476}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/58621a9010d93459e681e634068b225b0f5fceb5/components/variations/variations_url_constants.cc

Status: Fixed (was: Started)
Merged to M55.

Sign in to add a comment