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

Issue 10909115: Revert 155317 - Adjust ChromeToMobile error logging and mitigation. (Closed)

Created:
8 years, 3 months ago by erikwright (departed)
Modified:
8 years, 3 months ago
Reviewers:
msw
CC:
chromium-reviews
Visibility:
Public.

Description

Revert 155317 - Adjust ChromeToMobile error logging and mitigation. See http://build.chromium.org/p/chromium/builders/Win7%20Sync/builds/25157/steps/sync_integration_tests/logs/stdio . Specifically: Sendin[1920:3900:0907/000156:407089812:FATAL:url_request_context.cc(99)] Check failed: false. Leaked 2 URLRequest(s). First URL: https://www.google.com/cloudprint/search?requestor=chrome-to-mobile. [1920:3900:0907/000156:407089812:FATAL:url_request_context.cc(99)] Check failed: false. Leaked 2 URLRequest(s). First URL: https://www.google.com/cloudprint/search?requestor=chrome-to-mobile. My suspicion is that by increasing retries, it ended up that a request was still going when the browser is shutting down. Verify that these requests are being properly stopped/destroyed before the context they are using goes away. ------ Increase kMaxRetries for URLFetcher to 5 (common elsewhere). Decrease kDelayHours for URLFetcher to 1hr (w/exp backoff). Clear |access_token_| with new kGaiaOAuth2LoginAccessToken. Clear |access_token_| *and devices* on OnGetTokenFailure(). Handle invalid login refresh tokens as OnGetTokenFailure(). Clear token, devices, retry on 403 (auth) search responses. Add and log some new BAD_* UMA Metric enum values. (407 related to Issue 146685 and perhaps Issue 137267) LOG (Chrome, not UMA) send response data on failure. Corresponding src/tools/histograms/histograms.xml change: https://chromereviews.googleplex.com/4782033 BUG=102709, 120941, 146685 TEST=Less Chrome To Mobile failures, additional insightful logging. Review URL: https://chromiumcodereview.appspot.com/10913089 TBR=msw@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=155364

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+25 lines, -66 lines) Patch
M chrome/browser/chrome_to_mobile_service.h View 2 chunks +1 line, -10 lines 0 comments Download
M chrome/browser/chrome_to_mobile_service.cc View 9 chunks +24 lines, -56 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
erikwright (departed)
8 years, 3 months ago (2012-09-07 12:33:56 UTC) #1
msw
8 years, 3 months ago (2012-09-07 15:14:36 UTC) #2
Not the root cause, but fine to revert. LGTM, thanks for your help.
I'm fixing the root issue in my attempt to re-land the original patch:
http://codereview.chromium.org/10918119/

Powered by Google App Engine
This is Rietveld 408576698