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

Issue 1110993003: [BackgroundSyncManager] Remove origin argument from public BackgroundSyncManager methods (Closed)

Created:
5 years, 7 months ago by jkarlin
Modified:
5 years, 7 months ago
Reviewers:
jsbell
CC:
chromium-reviews, tim+watch_chromium.org, zea+watch_chromium.org, maxbogue+watch_chromium.org, jam, pvalenzuela+watch_chromium.org, plaree+watch_chromium.org, jkarlin+watch_chromium.org, darin-cc_chromium.org, maniscalco+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[BackgroundSyncManager] Remove origin argument from public BackgroundSyncManager methods The origin can be determined from the live ServiceWorkerRegistration, use that instead of requiring the renderer to pass it. Also, verify that the live registration is active to prevent BackgroundSyncManager::Register from being called prematurely. BUG=482012 Committed: https://crrev.com/c1a361d95254ab697b077187d0c6204cb18722b9 Cr-Commit-Position: refs/heads/master@{#327371}

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+70 lines, -38 lines) Patch
M content/browser/background_sync/background_sync_manager.h View 7 chunks +6 lines, -11 lines 0 comments Download
M content/browser/background_sync/background_sync_manager.cc View 8 chunks +16 lines, -11 lines 1 comment Download
M content/browser/background_sync/background_sync_manager_unittest.cc View 12 chunks +48 lines, -16 lines 1 comment Download

Messages

Total messages: 7 (2 generated)
jkarlin
jsbell@ PTAL at everything, thanks!
5 years, 7 months ago (2015-04-28 16:34:10 UTC) #2
jsbell
lgtm https://codereview.chromium.org/1110993003/diff/1/content/browser/background_sync/background_sync_manager.cc File content/browser/background_sync/background_sync_manager.cc (right): https://codereview.chromium.org/1110993003/diff/1/content/browser/background_sync/background_sync_manager.cc#newcode282 content/browser/background_sync/background_sync_manager.cc:282: sw_registration->pattern().GetOrigin(), It seems to me we should probably ...
5 years, 7 months ago (2015-04-28 20:52:33 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1110993003/1
5 years, 7 months ago (2015-04-28 20:55:06 UTC) #5
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 7 months ago (2015-04-28 22:01:06 UTC) #6
commit-bot: I haz the power
5 years, 7 months ago (2015-04-28 22:01:56 UTC) #7
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/c1a361d95254ab697b077187d0c6204cb18722b9
Cr-Commit-Position: refs/heads/master@{#327371}

Powered by Google App Engine
This is Rietveld 408576698