New issue
Advanced search Search tips

Issue 844971 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: ----
Type: ----

Blocking:
issue 773295



Sign in to add a comment

Migrate components/translate/core/browser/translate_url_fetcher.cc

Project Member Reported by dxie@google.com, May 20 2018

Issue description


 

Comment 1 by dxie@google.com, May 20 2018

Labels: Proj-Servicification-Canary Proj-Servicification OS-Windows OS-Linux OS-Mac OS-Chrome Proj-Servicification-network-url OS-Android
Status: Available (was: Untriaged)
Owner: toniki...@chromium.org
Status: Assigned (was: Available)
Status: Started (was: Assigned)
Blocking: 773295
WIP: https://chromium-review.googlesource.com/c/chromium/src/+/1085947 

As of today, CL misses some iOS bits.
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 5 2018

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

commit 060e337b92dbac48c82a268910c10c9d5f0bfdbc
Author: Antonio Gomes <tonikitoo@igalia.com>
Date: Tue Jun 05 16:48:51 2018

Factor the URL composition logic out of TranslateScript::Request

... into its own helper.

This is a preparation CL that helps with the migration of
TranslateURLFetcher et al to SimpleURLLoader.
Basically, when using TestURLLoaderFactory (see [1]), it is handy
to know to the URL being 'loaded', so that we can install interceptor
accordingly and provide the proper response.

No functional change. No news tests.

BUG= 844971 

[1] https://crrev.com/c/1085947

Change-Id: I2c709a86a55dc636ec099e66475ad8a91947f674
Reviewed-on: https://chromium-review.googlesource.com/1087067
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564542}
[modify] https://crrev.com/060e337b92dbac48c82a268910c10c9d5f0bfdbc/components/translate/core/browser/translate_script.cc
[modify] https://crrev.com/060e337b92dbac48c82a268910c10c9d5f0bfdbc/components/translate/core/browser/translate_script.h

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 13 2018

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

commit 61a972c30fcab43b3ea65e510b8fe8dde26e6274
Author: Antonio Gomes <tonikitoo@igalia.com>
Date: Wed Jun 13 21:45:49 2018

Migrate TranslateURLFetcher to SimpleURLLoader

URLFetcher et al will stop working in the browser process with advent
of Network Service, and SimpleURLLoader is the replacement API
for most clients. CL migrates TranslateURLFetcher to the new API.

Additionally, various unittests were also updated to use the new
TestURLLoaderFactory and EmbeddedTestServer machinery.

As a bonus, the CL allows us to get of URLRequestContextGetter
references throughout the TranslateURLFetcher path, including
TranslateLanguageList, TranslateScript, etc.

Since TestURLFetcherFactory allowed us to retrieve the actual
URLFetcher object (given an known 'id'), and SimpleURLLoader does
not, some helper methods were introduced, in order to verify the
state of the objects at a certain moment of the execution flow,
eg LanguageFetchURLForTesting and HasOngoingLanguageListLoadingForTesting.

Last, in a follow up CL, I will change the nomenclature used
throughout these files, from "fetcher" to "loader".

Bug: 773295, 844971 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.linux:linux_mojo;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ica404dded4fab79a0035df825f12d2b1bb06329e
Reviewed-on: https://chromium-review.googlesource.com/1085947
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/master@{#567002}
[modify] https://crrev.com/61a972c30fcab43b3ea65e510b8fe8dde26e6274/chrome/browser/autofill/autofill_interactive_uitest.cc
[modify] https://crrev.com/61a972c30fcab43b3ea65e510b8fe8dde26e6274/chrome/browser/translate/translate_manager_browsertest.cc
[modify] https://crrev.com/61a972c30fcab43b3ea65e510b8fe8dde26e6274/chrome/browser/translate/translate_manager_render_view_host_unittest.cc
[modify] https://crrev.com/61a972c30fcab43b3ea65e510b8fe8dde26e6274/chrome/browser/translate/translate_service.cc
[modify] https://crrev.com/61a972c30fcab43b3ea65e510b8fe8dde26e6274/chrome/browser/ui/views/translate/translate_language_browsertest.cc
[modify] https://crrev.com/61a972c30fcab43b3ea65e510b8fe8dde26e6274/components/translate/core/browser/BUILD.gn
[modify] https://crrev.com/61a972c30fcab43b3ea65e510b8fe8dde26e6274/components/translate/core/browser/DEPS
[modify] https://crrev.com/61a972c30fcab43b3ea65e510b8fe8dde26e6274/components/translate/core/browser/translate_download_manager.cc
[modify] https://crrev.com/61a972c30fcab43b3ea65e510b8fe8dde26e6274/components/translate/core/browser/translate_download_manager.h
[modify] https://crrev.com/61a972c30fcab43b3ea65e510b8fe8dde26e6274/components/translate/core/browser/translate_language_list.cc
[modify] https://crrev.com/61a972c30fcab43b3ea65e510b8fe8dde26e6274/components/translate/core/browser/translate_language_list.h
[modify] https://crrev.com/61a972c30fcab43b3ea65e510b8fe8dde26e6274/components/translate/core/browser/translate_language_list_unittest.cc
[modify] https://crrev.com/61a972c30fcab43b3ea65e510b8fe8dde26e6274/components/translate/core/browser/translate_script.h
[modify] https://crrev.com/61a972c30fcab43b3ea65e510b8fe8dde26e6274/components/translate/core/browser/translate_script_unittest.cc
[modify] https://crrev.com/61a972c30fcab43b3ea65e510b8fe8dde26e6274/components/translate/core/browser/translate_url_fetcher.cc
[modify] https://crrev.com/61a972c30fcab43b3ea65e510b8fe8dde26e6274/components/translate/core/browser/translate_url_fetcher.h
[modify] https://crrev.com/61a972c30fcab43b3ea65e510b8fe8dde26e6274/ios/chrome/browser/translate/translate_service_ios.cc
[modify] https://crrev.com/61a972c30fcab43b3ea65e510b8fe8dde26e6274/ios/web_view/internal/DEPS
[modify] https://crrev.com/61a972c30fcab43b3ea65e510b8fe8dde26e6274/ios/web_view/internal/app/application_context.cc
[modify] https://crrev.com/61a972c30fcab43b3ea65e510b8fe8dde26e6274/ios/web_view/internal/app/application_context.h
[modify] https://crrev.com/61a972c30fcab43b3ea65e510b8fe8dde26e6274/ios/web_view/internal/translate/web_view_translate_service.cc

Status: Fixed (was: Started)
Project Member

Comment 9 by bugdroid1@chromium.org, Jul 2

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

commit b7f73d439d43a993633b425e3298cfd86d573fc7
Author: Antonio Gomes <tonikitoo@igalia.com>
Date: Mon Jul 02 12:35:39 2018

Get rid of TranslateURLFetcher::id_

This is a follow up of [1]. URLFetcher instances can be associated
with an id (integer). Since this class has moved away from using
URLFetcher (in favor of SimpleURLLoader), we can now remove
the id's altogether.

[1] https://crrev.com/c/1085947/9/components/translate/core/browser/translate_url_fetcher.h@67

BUG= 844971 

Change-Id: I6b4a9ec8046fe815104cd42f8d79d57eca90559c
Reviewed-on: https://chromium-review.googlesource.com/1120506
Reviewed-by: David Roger <droger@chromium.org>
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/master@{#571873}
[modify] https://crrev.com/b7f73d439d43a993633b425e3298cfd86d573fc7/components/translate/core/browser/translate_language_list.cc
[modify] https://crrev.com/b7f73d439d43a993633b425e3298cfd86d573fc7/components/translate/core/browser/translate_language_list.h
[modify] https://crrev.com/b7f73d439d43a993633b425e3298cfd86d573fc7/components/translate/core/browser/translate_script.cc
[modify] https://crrev.com/b7f73d439d43a993633b425e3298cfd86d573fc7/components/translate/core/browser/translate_script.h
[modify] https://crrev.com/b7f73d439d43a993633b425e3298cfd86d573fc7/components/translate/core/browser/translate_url_fetcher.cc
[modify] https://crrev.com/b7f73d439d43a993633b425e3298cfd86d573fc7/components/translate/core/browser/translate_url_fetcher.h

Sign in to add a comment