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

Issue 844938 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 9
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: ----
Type: ----

Blocking:
issue 773295



Sign in to add a comment

Migrate components/metrics/net to network::SimpleURLLoader

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: pilgrim@chromium.org
Status: Started (was: Available)
Owner: ----
Status: Available (was: Started)
Blocking: 773295
Components: Internals>Metrics
Summary: Migrate components/metrics/net to network::SimpleURLLoader (was: Migrate components/metrics/net/net_metrics_log_uploader.cc)
Owner: toniki...@chromium.org
Status: Started (was: Available)
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 9

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

commit c50aca7d32a8f613ff84ef0c90a7b560e1f9cbe2
Author: Antonio Gomes <tonikitoo@igalia.com>
Date: Thu Aug 09 00:06:43 2018

Migrate components/metrics/net to network::SimpleURLLoader

URLFetcher will stop working with advent of Network Service, and
SimpleURLLoader is the replacement API for most clients.
This CL migrates NetMetricsLogUploader away from URLFetcher.

Note that previously, code was written like:

  URLFetcher* fetcher = ...;
  if (condition) {
    fetcher->SetUploadData(foo)
    fetcher->AddExtraRequestHeader(blah)
    fetcher->AddExtraRequestHeader(bleh)
  } else {
    fetcher->SetUploadData(foo2)
    fetcher->AddExtraRequestHeader(blah2)
  }

... whereas now, given how SimpleURLLoader and ResourceRequest APIs
are designed, code needed to change to something like:

  ResourceResource request = ...;
  if (condition) {
    request->headers.SetHeader(blah)
    fetcher->headers.SetHeader(bleh)
  } else {
    fetcher->headers.SetHeader(blah2)
  }
  SimpleURLLoader loader = ctor(std::move(request),... );
  if (condition) {
    loader->AttachStringForUpload(foo)
  } else {
    loader->AttachStringForUpload(foo2)
  }

Bug:  844938 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ia97f065eb22e65e32066d45010b909b2cab9d5fb
Reviewed-on: https://chromium-review.googlesource.com/1136518
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Luke Halliwell <halliwell@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/master@{#581727}
[modify] https://crrev.com/c50aca7d32a8f613ff84ef0c90a7b560e1f9cbe2/chrome/browser/metrics/chrome_metrics_service_client.cc
[modify] https://crrev.com/c50aca7d32a8f613ff84ef0c90a7b560e1f9cbe2/chromecast/browser/cast_browser_main_parts.cc
[modify] https://crrev.com/c50aca7d32a8f613ff84ef0c90a7b560e1f9cbe2/chromecast/browser/metrics/cast_metrics_service_client.cc
[modify] https://crrev.com/c50aca7d32a8f613ff84ef0c90a7b560e1f9cbe2/chromecast/browser/metrics/cast_metrics_service_client.h
[modify] https://crrev.com/c50aca7d32a8f613ff84ef0c90a7b560e1f9cbe2/components/metrics/BUILD.gn
[modify] https://crrev.com/c50aca7d32a8f613ff84ef0c90a7b560e1f9cbe2/components/metrics/net/DEPS
[modify] https://crrev.com/c50aca7d32a8f613ff84ef0c90a7b560e1f9cbe2/components/metrics/net/net_metrics_log_uploader.cc
[modify] https://crrev.com/c50aca7d32a8f613ff84ef0c90a7b560e1f9cbe2/components/metrics/net/net_metrics_log_uploader.h
[modify] https://crrev.com/c50aca7d32a8f613ff84ef0c90a7b560e1f9cbe2/components/metrics/net/net_metrics_log_uploader_unittest.cc
[modify] https://crrev.com/c50aca7d32a8f613ff84ef0c90a7b560e1f9cbe2/ios/chrome/browser/metrics/ios_chrome_metrics_service_client.mm
[modify] https://crrev.com/c50aca7d32a8f613ff84ef0c90a7b560e1f9cbe2/tools/metrics/histograms/histograms.xml

Status: Fixed (was: Started)
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 19

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

commit 8d5f7bbac136173a500237c0c200d9ed81f83315
Author: Antonio Gomes <tonikitoo@igalia.com>
Date: Wed Sep 19 18:40:25 2018

fixup! Migrate components/metrics/net to network::SimpleURLLoader

Follow up of [1], that introduces
TestURLLoaderFactory::GetPendingRequest.

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

TBR=isherman@chromium.org

BUG= 844938 

Change-Id: I59b19774462957fa71e09759f0154471f2a6c8f3
Reviewed-on: https://chromium-review.googlesource.com/1234634
Reviewed-by: Antonio Gomes <tonikitoo@igalia.com>
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/master@{#592491}
[modify] https://crrev.com/8d5f7bbac136173a500237c0c200d9ed81f83315/components/metrics/net/net_metrics_log_uploader_unittest.cc

Sign in to add a comment