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

Issue 2828093002: Add plumbing for accessing latest country in variations service in Java (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 week, 2 days ago by Yusuf
Modified:
1 week, 1 day ago
CC:
chromium-reviews, asvitkine+watch_chromium.org, agrieve+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Add plumbing for accessing latest country in variations service in Java Checking what the latest country is according to variations service is an important signal for various features but this is currently not exposed in variations_service code. This adds the required plumbing from variations_service to the corresponding Android side classes. BUG=713381 Review-Url: https://codereview.chromium.org/2828093002 Cr-Commit-Position: refs/heads/master@{#466107} Committed: https://chromium.googlesource.com/chromium/src/+/d855bfda144dd7d99d9b39f779837c3e45d6b1e9

Patch Set 1 #

Total comments: 6

Patch Set 2 : asvitkine@ comments #

Total comments: 5

Patch Set 3 : nits #

Unified diffs Side-by-side diffs Delta from patch set Stats (+40 lines, -0 lines) Patch
M chrome/android/java/src/org/chromium/chrome/browser/ChromeActivitySessionTracker.java View 1 2 2 chunks +9 lines, -0 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/metrics/VariationsSession.java View 1 1 chunk +8 lines, -0 lines 0 comments Download
M chrome/browser/android/metrics/variations_session.cc View 1 1 chunk +15 lines, -0 lines 0 comments Download
M components/variations/service/variations_service.h View 1 1 chunk +4 lines, -0 lines 0 comments Download
M components/variations/service/variations_service.cc View 1 chunk +4 lines, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 14 (5 generated)
Yusuf
Still verifying that this does what I was hoping it would do, but sending this ...
1 week, 2 days ago (2017-04-19 21:39:22 UTC) #2
Alexei Svitkine (slow)
https://codereview.chromium.org/2828093002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/metrics/VariationsSession.java File chrome/android/java/src/org/chromium/chrome/browser/metrics/VariationsSession.java (right): https://codereview.chromium.org/2828093002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/metrics/VariationsSession.java#newcode73 chrome/android/java/src/org/chromium/chrome/browser/metrics/VariationsSession.java:73: * @return The latest country according to the current ...
1 week, 2 days ago (2017-04-19 21:42:57 UTC) #3
Yusuf
https://codereview.chromium.org/2828093002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/metrics/VariationsSession.java File chrome/android/java/src/org/chromium/chrome/browser/metrics/VariationsSession.java (right): https://codereview.chromium.org/2828093002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/metrics/VariationsSession.java#newcode73 chrome/android/java/src/org/chromium/chrome/browser/metrics/VariationsSession.java:73: * @return The latest country according to the current ...
1 week, 2 days ago (2017-04-19 22:16:46 UTC) #4
slow (dfalcantara)
lgtm https://codereview.chromium.org/2828093002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/metrics/VariationsSession.java File chrome/android/java/src/org/chromium/chrome/browser/metrics/VariationsSession.java (right): https://codereview.chromium.org/2828093002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/metrics/VariationsSession.java#newcode80 chrome/android/java/src/org/chromium/chrome/browser/metrics/VariationsSession.java:80: protected native String nativeGetLatestCountry(); does this thing need ...
1 week, 1 day ago (2017-04-20 16:31:45 UTC) #5
Alexei Svitkine (slow)
lgtm https://codereview.chromium.org/2828093002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivitySessionTracker.java File chrome/android/java/src/org/chromium/chrome/browser/ChromeActivitySessionTracker.java (right): https://codereview.chromium.org/2828093002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivitySessionTracker.java#newcode86 chrome/android/java/src/org/chromium/chrome/browser/ChromeActivitySessionTracker.java:86: * @return The latest country according to the ...
1 week, 1 day ago (2017-04-20 16:35:41 UTC) #6
slow (dfalcantara)
https://codereview.chromium.org/2828093002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivitySessionTracker.java File chrome/android/java/src/org/chromium/chrome/browser/ChromeActivitySessionTracker.java (right): https://codereview.chromium.org/2828093002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivitySessionTracker.java#newcode86 chrome/android/java/src/org/chromium/chrome/browser/ChromeActivitySessionTracker.java:86: * @return The latest country according to the current ...
1 week, 1 day ago (2017-04-20 16:38:37 UTC) #7
Yusuf
https://codereview.chromium.org/2828093002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivitySessionTracker.java File chrome/android/java/src/org/chromium/chrome/browser/ChromeActivitySessionTracker.java (right): https://codereview.chromium.org/2828093002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivitySessionTracker.java#newcode86 chrome/android/java/src/org/chromium/chrome/browser/ChromeActivitySessionTracker.java:86: * @return The latest country according to the current ...
1 week, 1 day ago (2017-04-20 18:16:13 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2828093002/40001
1 week, 1 day ago (2017-04-20 18:17:11 UTC) #11
commit-bot: I haz the power
1 week, 1 day ago (2017-04-20 20:24:20 UTC) #14
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/d855bfda144dd7d99d9b39f77983...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cc6ac46