Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(9)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 months ago by Yusuf
Modified:
3 months 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 ...
3 months 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 ...
3 months 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 ...
3 months ago (2017-04-19 22:16:46 UTC) #4
gone
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 ...
3 months 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 ...
3 months ago (2017-04-20 16:35:41 UTC) #6
gone
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 ...
3 months 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 ...
3 months 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
3 months ago (2017-04-20 18:17:11 UTC) #11
commit-bot: I haz the power
3 months 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 25c286973