Chromium Code Reviews
|
|
Chromium Code Reviews|
Created:
5 years, 4 months ago by jungkees Modified:
5 years, 4 months ago Reviewers:
michaeln, falken, tkent, nhiroki, kinuko, lgombos, zino, nhiroki (google) CC:
chromium-reviews, horo, jam, kinuko+serviceworker, kinuko+watch, serviceworker-reviews, tzik Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionService Worker: Add missing ServiceWorkerDispatcherHost::SendUpdateError.
Add ServiceWorkerDispatcherHost::SendUpdateError method as a follow-up patch for
https://codereview.chromium.org/1270513002/.
While working on the layout test, itt has been found that
ServiceWorkerDispatcherHost::UpdateComplete was calling
ServiceWorkerDispatcherHost::SendRegistrationError instead of the missing
ServiceWorkerDispatcherHost::SendUpdateError so the callback in the
renderer-side could not be properly located.
Related CL: https://codereview.chromium.org/1270513002/
BUG=513655
Committed: https://crrev.com/ee412360fbb8b20c0fdd80239ca6a0e99038e1b1
Cr-Commit-Position: refs/heads/master@{#342091}
Patch Set 1 #
Created: 5 years, 4 months ago
Messages
Total messages: 21 (7 generated)
jungkee.song@samsung.com changed reviewers: + jinho.bang@samsung.com, l.gombos@samsung.com
zino@, while working on the layout test, I found it had an issue where the update() promise is not settled in error cases. And I found the reason that SendUpdateError method is missing and SendRegistrationError was being called instead which led the code path to trying to find a callback in a wrong map when it got back in the renderer. This CL addresses this problem. PTAL.
On 2015/08/06 07:12:46, jungkees wrote: > zino@, while working on the layout test, I found it had an issue where the > update() promise is not settled in error cases. And I found the reason that > SendUpdateError method is missing and SendRegistrationError was being called > instead which led the code path to trying to find a callback in a wrong map when > it got back in the renderer. This CL addresses this problem. PTAL. lgtm
SW OWNERs, PTAL.
On 2015/08/06 08:00:33, jungkees wrote: > SW OWNERs, PTAL. LGTM
On 2015/08/06 08:11:26, nhiroki (google) wrote: > On 2015/08/06 08:00:33, jungkees wrote: > > SW OWNERs, PTAL. > > LGTM Please use BUG=513655
The CQ bit was checked by jungkee.song@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1278713002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1278713002/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
nhiroki@, it seems a presubmit error (http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...) occurs with the following reason: ** Presubmit ERRORS ** Missing LGTM from an OWNER for these files: content/browser/service_worker/service_worker_dispatcher_host.cc content/browser/service_worker/service_worker_dispatcher_host.h It's weird as you gave one.
On 2015/08/06 08:25:08, jungkees wrote: > nhiroki@, it seems a presubmit error > (http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...) > occurs with the following reason: > > ** Presubmit ERRORS ** > Missing LGTM from an OWNER for these files: > content/browser/service_worker/service_worker_dispatcher_host.cc > content/browser/service_worker/service_worker_dispatcher_host.h > > It's weird as you gave one. Sorry! I gave LGTM from a wrong address...
The CQ bit was checked by nhiroki@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1278713002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1278713002/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by jungkee.song@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1278713002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1278713002/1
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/ee412360fbb8b20c0fdd80239ca6a0e99038e1b1 Cr-Commit-Position: refs/heads/master@{#342091} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
