Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(66)

Issue 2559243003: Extend with a language code in http accept languages

Created:
4 years ago by Yirui Huang
Modified:
3 years, 2 months ago
CC:
chromium-reviews, cbentzel+watch_chromium.org, lcwu+watch_chromium.org, pkl (ping after 24h if needed), halliwell+watch_chromium.org, agrieve+watch_chromium.org, alokp+watch_chromium.org, android-webview-reviews_chromium.org, jshin+watch_chromium.org, sdefresne+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Extend with a language code in http accept languages This CL is a clean up for accept languages in HTTP header. Both Android Chrome and Android Webview will return a unique language list. Then in the http header generater, the unique language list will be expanded with a langauge code. In this way, it keeps the same format in final accept languages that appear in http header for both Chrome and Webview. Since http_util could not use icu, this CL separated method GenerateAcceptLanguageHeader into a new file http_util_icu under net. This CL maintains the addition of en-US for accept languages in Android Webview as before. BUG=667705

Patch Set 1 : Rebased separate Generate AcceptLanguage Header from http_util #

Total comments: 24

Patch Set 2 : Removed unnecessary imports and checked develop path #

Total comments: 2

Patch Set 3 : Rebased and modified descriptions #

Total comments: 4

Patch Set 4 : Rebased and removed unnecessary import #

Total comments: 2

Patch Set 5 : Rebased and used namespcace instead of class #

Total comments: 28

Patch Set 6 : Rebased, fixed nits and handed over this CL :( #

Unified diffs Side-by-side diffs Delta from patch set Stats (+237 lines, -182 lines) Patch
M android_webview/browser/aw_content_browser_client.cc View 1 2 3 4 5 1 chunk +0 lines, -2 lines 0 comments Download
M android_webview/browser/net/aw_http_user_agent_settings.cc View 1 2 3 4 2 chunks +3 lines, -1 line 0 comments Download
M android_webview/javatests/src/org/chromium/android_webview/test/AcceptLanguageTest.java View 1 2 3 4 5 7 chunks +29 lines, -34 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PwsClientImpl.java View 2 chunks +6 lines, -28 lines 0 comments Download
M chrome/android/javatests/src/org/chromium/chrome/browser/physicalweb/PwsClientImplTest.java View 4 chunks +4 lines, -14 lines 0 comments Download
M chrome/browser/android/preferences/pref_service_bridge.cc View 1 2 3 2 chunks +6 lines, -22 lines 0 comments Download
M chrome/browser/android/preferences/pref_service_bridge_unittest.cc View 3 chunks +7 lines, -12 lines 0 comments Download
M chrome/browser/net/chrome_http_user_agent_settings.cc View 1 2 3 4 3 chunks +5 lines, -4 lines 0 comments Download
M chromecast/browser/cast_http_user_agent_settings.cc View 1 2 3 4 2 chunks +2 lines, -2 lines 0 comments Download
M ios/chrome/browser/net/ios_chrome_http_user_agent_settings.mm View 1 2 3 4 3 chunks +5 lines, -4 lines 0 comments Download
M net/BUILD.gn View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M net/DEPS View 1 chunk +5 lines, -0 lines 0 comments Download
M net/http/http_util.h View 1 chunk +0 lines, -17 lines 0 comments Download
M net/http/http_util.cc View 1 chunk +0 lines, -34 lines 0 comments Download
A net/http/http_util_icu.h View 1 2 3 4 5 1 chunk +35 lines, -0 lines 0 comments Download
A net/http/http_util_icu.cc View 1 2 3 4 5 1 chunk +89 lines, -0 lines 0 comments Download
A net/http/http_util_icu_unittest.cc View 1 2 3 4 1 chunk +38 lines, -0 lines 0 comments Download
M net/http/http_util_unittest.cc View 1 chunk +0 lines, -8 lines 0 comments Download
M net/net.gypi View 1 2 3 4 2 chunks +2 lines, -0 lines 0 comments Download

Messages

Total messages: 68 (48 generated)
Seigo Nonaka
https://codereview.chromium.org/2559243003/diff/40001/android_webview/javatests/src/org/chromium/android_webview/test/AcceptLanguageTest.java File android_webview/javatests/src/org/chromium/android_webview/test/AcceptLanguageTest.java (right): https://codereview.chromium.org/2559243003/diff/40001/android_webview/javatests/src/org/chromium/android_webview/test/AcceptLanguageTest.java#newcode159 android_webview/javatests/src/org/chromium/android_webview/test/AcceptLanguageTest.java:159: // Test language code is inserted only after the ...
4 years ago (2016-12-12 01:48:54 UTC) #13
Yirui Huang
https://codereview.chromium.org/2559243003/diff/40001/android_webview/javatests/src/org/chromium/android_webview/test/AcceptLanguageTest.java File android_webview/javatests/src/org/chromium/android_webview/test/AcceptLanguageTest.java (right): https://codereview.chromium.org/2559243003/diff/40001/android_webview/javatests/src/org/chromium/android_webview/test/AcceptLanguageTest.java#newcode159 android_webview/javatests/src/org/chromium/android_webview/test/AcceptLanguageTest.java:159: // Test language code is inserted only after the ...
4 years ago (2016-12-12 06:50:41 UTC) #21
Seigo Nonaka
https://codereview.chromium.org/2559243003/diff/40001/net/http/http_util_icu.cc File net/http/http_util_icu.cc (right): https://codereview.chromium.org/2559243003/diff/40001/net/http/http_util_icu.cc#newcode9 net/http/http_util_icu.cc:9: #include <string> string is necessary for std::string https://codereview.chromium.org/2559243003/diff/80001/net/http/http_util_icu.cc File ...
4 years ago (2016-12-12 07:29:26 UTC) #22
Yirui Huang
https://codereview.chromium.org/2559243003/diff/40001/net/http/http_util_icu.cc File net/http/http_util_icu.cc (right): https://codereview.chromium.org/2559243003/diff/40001/net/http/http_util_icu.cc#newcode9 net/http/http_util_icu.cc:9: #include <string> On 2016/12/12 07:29:26, Seigo Nonaka wrote: > ...
4 years ago (2016-12-12 07:41:35 UTC) #25
Seigo Nonaka
lgtm with nits. Please get review from net owner and Jungshik. https://codereview.chromium.org/2559243003/diff/140001/chromecast/browser/cast_http_user_agent_settings.cc File chromecast/browser/cast_http_user_agent_settings.cc (right): ...
4 years ago (2016-12-14 01:27:21 UTC) #36
Yirui Huang
+ droger@ as an owner for ios/ + slan@ as an owner for chromecast/ + ...
4 years ago (2016-12-14 01:53:10 UTC) #38
slan
On 2016/12/14 01:53:10, Yirui Huang wrote: > + droger@ as an owner for ios/ > ...
4 years ago (2016-12-14 04:08:14 UTC) #43
Torne
android_webview LGTM, thanks!
4 years ago (2016-12-14 11:23:51 UTC) #44
droger
ios lgtm
4 years ago (2016-12-14 12:17:37 UTC) #45
Maria
android lgtm
4 years ago (2016-12-14 17:43:58 UTC) #46
Ryan Sleevi
Sorry I'm slow on this - this is quite subtle and it's going to take ...
4 years ago (2016-12-14 20:02:30 UTC) #47
Yirui Huang
https://codereview.chromium.org/2559243003/diff/160001/net/http/http_util_icu.h File net/http/http_util_icu.h (right): https://codereview.chromium.org/2559243003/diff/160001/net/http/http_util_icu.h#newcode14 net/http/http_util_icu.h:14: class NET_EXPORT HttpUtilIcu { On 2016/12/14 20:02:30, Ryan Sleevi ...
4 years ago (2016-12-15 02:37:10 UTC) #50
Ryan Sleevi
So I'm still working through timezones with folks on the //net ICU issues, but there's ...
4 years ago (2016-12-15 23:44:07 UTC) #53
Yirui Huang
https://codereview.chromium.org/2559243003/diff/180001/android_webview/browser/net/aw_http_user_agent_settings.cc File android_webview/browser/net/aw_http_user_agent_settings.cc (right): https://codereview.chromium.org/2559243003/diff/180001/android_webview/browser/net/aw_http_user_agent_settings.cc#newcode29 android_webview/browser/net/aw_http_user_agent_settings.cc:29: net::http_util_icu::GenerateAcceptLanguageHeader( On 2016/12/15 23:44:07, Ryan Sleevi wrote: > So ...
4 years ago (2016-12-16 08:00:08 UTC) #57
jungshik at Google
> This CL maintains the addition of en-US for accept languages in > Android Webview ...
4 years ago (2016-12-16 08:01:45 UTC) #58
Yirui Huang
https://codereview.chromium.org/2559243003/diff/180001/android_webview/javatests/src/org/chromium/android_webview/test/AcceptLanguageTest.java File android_webview/javatests/src/org/chromium/android_webview/test/AcceptLanguageTest.java (right): https://codereview.chromium.org/2559243003/diff/180001/android_webview/javatests/src/org/chromium/android_webview/test/AcceptLanguageTest.java#newcode58 android_webview/javatests/src/org/chromium/android_webview/test/AcceptLanguageTest.java:58: * "de-DE,de;q=0.8,en-US;q=0.6,en-UK;q=0.4,en;q=0.2" On 2016/12/16 08:01:45, jungshik at google wrote: ...
4 years ago (2016-12-16 08:14:25 UTC) #62
Ryan Sleevi
nona@ - Do you plan to maintain this CL? Otherwise, I'd like to close it ...
3 years, 10 months ago (2017-01-25 20:02:43 UTC) #65
Torne
Is someone taking this work over? This does still look like it's useful, and rietveld ...
3 years, 2 months ago (2017-09-25 18:37:29 UTC) #66
Seigo Nonaka
On 2017/09/25 18:37:29, Torne wrote: > Is someone taking this work over? This does still ...
3 years, 2 months ago (2017-09-27 22:22:48 UTC) #67
Seigo Nonaka
3 years, 2 months ago (2017-09-27 22:22:50 UTC) #68
On 2017/09/25 18:37:29, Torne wrote:
> Is someone taking this work over? This does still look like it's useful, and
> rietveld is going read-only soon.

I'm sorry I lose the context and don't follow the latest code. This code maybe
out of date. I'm going to revisit when I have time.

Powered by Google App Engine
This is Rietveld 408576698