Chromium Code Reviews
Help | Chromium Project | Sign in
(18)

Issue 2678733002: [ServiceWorker] Mojofy ActivateEvent of Service Worker (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 weeks ago by xiaofengzhang
Modified:
1 week, 1 day ago
CC:
chromium-reviews, michaeln, jsbell+serviceworker_chromium.org, viettrungluu+watch_chromium.org, qsr+mojo_chromium.org, mlamouri+watch-content_chromium.org, shimazu+serviceworker_chromium.org, serviceworker-reviews, jam, nhiroki, abarth-chromium, Aaron Boodman, kinuko+serviceworker, yzshen+watch_chromium.org, horo+watch_chromium.org, darin-cc_chromium.org, kinuko+watch, tzik, darin (slow to review), blink-worker-reviews_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[ServiceWorker] Mojofy ActivateEvent of Service Worker This patch mojofies the following two messages: ServiceWorkerMsg_ActivateEvent ServiceWorkerHostMsg_ActivateEventFinished BUG=629701 TEST=content_unittests Review-Url: https://codereview.chromium.org/2678733002 Cr-Commit-Position: refs/heads/master@{#451513} Committed: https://chromium.googlesource.com/chromium/src/+/64975bc58e6b041ade1cfe2edb431896548845f3

Patch Set 1 #

Total comments: 12

Patch Set 2 : Rebase and address shimazu, falken and peter's comments #

Total comments: 26

Patch Set 3 : Address comments #25, #26, #33 #

Total comments: 3

Patch Set 4 : Address falken's comments #38 #

Patch Set 5 : Just rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+87 lines, -47 lines) Patch
M content/browser/service_worker/embedded_worker_test_helper.h View 1 2 3 4 5 chunks +11 lines, -2 lines 0 comments Download
M content/browser/service_worker/embedded_worker_test_helper.cc View 1 2 3 4 4 chunks +17 lines, -10 lines 0 comments Download
M content/browser/service_worker/service_worker_browsertest.cc View 1 1 chunk +2 lines, -2 lines 0 comments Download
M content/browser/service_worker/service_worker_context_unittest.cc View 1 2 3 4 chunks +15 lines, -12 lines 0 comments Download
M content/browser/service_worker/service_worker_job_unittest.cc View 1 2 3 2 chunks +18 lines, -4 lines 0 comments Download
M content/browser/service_worker/service_worker_registration.cc View 1 1 chunk +2 lines, -3 lines 0 comments Download
M content/common/service_worker/service_worker_event_dispatcher.mojom View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments Download
M content/common/service_worker/service_worker_messages.h View 1 2 3 4 2 chunks +0 lines, -7 lines 0 comments Download
M content/renderer/service_worker/service_worker_context_client.h View 1 2 3 4 2 chunks +2 lines, -1 line 0 comments Download
M content/renderer/service_worker/service_worker_context_client.cc View 1 2 3 4 6 chunks +17 lines, -6 lines 0 comments Download
Trybot results:  linux_android_rel_ng   cast_shell_linux   win_chromium_x64_rel_ng   win_chromium_compile_dbg_ng   linux_chromium_asan_rel_ng   android_compile_dbg   android_arm64_dbg_recipe   win_clang   android_n5x_swarming_rel   mac_chromium_compile_dbg_ng   ios-device   chromeos_amd64-generic_chromium_compile_only_ng   cast_shell_android   linux_chromium_compile_dbg_ng   linux_chromium_chromeos_ozone_rel_ng   ios-simulator-xcode-clang   ios-device-xcode-clang   android_clang_dbg_recipe   linux_chromium_rel_ng   mac_chromium_rel_ng   ios-simulator   win_chromium_rel_ng   linux_chromium_tsan_rel_ng   chromeos_daisy_chromium_compile_only_ng   linux_chromium_chromeos_rel_ng   chromium_presubmit   android_cronet   chromium_presubmit   linux_chromium_rel_ng   win_chromium_x64_rel_ng   chromium_presubmit   win_chromium_x64_rel_ng   chromium_presubmit   linux_chromium_chromeos_ozone_rel_ng   mac_chromium_rel_ng   linux_chromium_asan_rel_ng   win_clang   linux_chromium_compile_dbg_ng   android_cronet   linux_chromium_tsan_rel_ng   ios-simulator   cast_shell_android   android_n5x_swarming_rel   ios-simulator-xcode-clang   mac_chromium_compile_dbg_ng   ios-device   linux_chromium_rel_ng   chromeos_daisy_chromium_compile_only_ng   linux_android_rel_ng   ios-device-xcode-clang   android_compile_dbg   win_chromium_x64_rel_ng   win_chromium_compile_dbg_ng   linux_chromium_chromeos_rel_ng   android_clang_dbg_recipe   cast_shell_linux   win_chromium_rel_ng   chromeos_amd64-generic_chromium_compile_only_ng   android_arm64_dbg_recipe 
Commit queue not available (can’t edit this change).

Messages

Total messages: 66 (28 generated)
xiaofengzhang
shimazu@ Could you please review this? Thanks.
2 weeks, 6 days ago (2017-02-07 01:23:25 UTC) #6
shimazu
Thanks for continuing to work on this:) Overall looks good. https://codereview.chromium.org/2678733002/diff/1/content/browser/service_worker/embedded_worker_test_helper.h File content/browser/service_worker/embedded_worker_test_helper.h (right): https://codereview.chromium.org/2678733002/diff/1/content/browser/service_worker/embedded_worker_test_helper.h#newcode67 ...
2 weeks, 6 days ago (2017-02-07 04:43:48 UTC) #7
shimazu
s/ActiviteEvent/ActivateEvent/ in the subject of this CL.
2 weeks, 6 days ago (2017-02-07 04:44:31 UTC) #8
falken
https://codereview.chromium.org/2678733002/diff/1/content/browser/service_worker/embedded_worker_test_helper.h File content/browser/service_worker/embedded_worker_test_helper.h (right): https://codereview.chromium.org/2678733002/diff/1/content/browser/service_worker/embedded_worker_test_helper.h#newcode67 content/browser/service_worker/embedded_worker_test_helper.h:67: enum class Event { OnActivateEvent, LastEvent }; On 2017/02/07 ...
2 weeks, 6 days ago (2017-02-07 05:19:35 UTC) #10
xiaofengzhang
On 2017/02/07 04:44:31, shimazu wrote: > s/ActiviteEvent/ActivateEvent/ in the subject of this CL. Thanks, done ...
2 weeks, 6 days ago (2017-02-07 05:57:44 UTC) #13
xiaofengzhang
https://codereview.chromium.org/2678733002/diff/1/content/browser/service_worker/embedded_worker_test_helper.h File content/browser/service_worker/embedded_worker_test_helper.h (right): https://codereview.chromium.org/2678733002/diff/1/content/browser/service_worker/embedded_worker_test_helper.h#newcode67 content/browser/service_worker/embedded_worker_test_helper.h:67: enum class Event { OnActivateEvent, LastEvent }; On 2017/02/07 ...
2 weeks, 6 days ago (2017-02-07 06:15:36 UTC) #14
shimazu
+peter for asking a question about his comment on the previous CL. https://codereview.chromium.org/2678733002/diff/1/content/browser/service_worker/service_worker_registration.cc File content/browser/service_worker/service_worker_registration.cc ...
2 weeks, 6 days ago (2017-02-07 07:19:31 UTC) #16
Peter Beverloo
https://codereview.chromium.org/2678733002/diff/1/content/browser/service_worker/service_worker_registration.cc File content/browser/service_worker/service_worker_registration.cc (right): https://codereview.chromium.org/2678733002/diff/1/content/browser/service_worker/service_worker_registration.cc#newcode432 content/browser/service_worker/service_worker_registration.cc:432: activating_version, request_id)); On 2017/02/07 07:19:31, shimazu wrote: > On ...
2 weeks, 5 days ago (2017-02-08 15:13:16 UTC) #17
shimazu
https://codereview.chromium.org/2678733002/diff/1/content/browser/service_worker/service_worker_registration.cc File content/browser/service_worker/service_worker_registration.cc (right): https://codereview.chromium.org/2678733002/diff/1/content/browser/service_worker/service_worker_registration.cc#newcode432 content/browser/service_worker/service_worker_registration.cc:432: activating_version, request_id)); On 2017/02/08 15:13:16, Peter Beverloo wrote: > ...
2 weeks, 3 days ago (2017-02-10 05:42:27 UTC) #18
Peter Beverloo
On 2017/02/10 05:42:27, shimazu wrote: > https://codereview.chromium.org/2678733002/diff/1/content/browser/service_worker/service_worker_registration.cc > File content/browser/service_worker/service_worker_registration.cc (right): > > https://codereview.chromium.org/2678733002/diff/1/content/browser/service_worker/service_worker_registration.cc#newcode432 > ...
2 weeks, 3 days ago (2017-02-10 17:40:53 UTC) #19
xiaofengzhang
On 2017/02/10 17:40:53, Peter Beverloo wrote: > On 2017/02/10 05:42:27, shimazu wrote: > > > ...
1 week, 6 days ago (2017-02-14 06:34:57 UTC) #20
Peter Beverloo
Drive-by comments https://codereview.chromium.org/2678733002/diff/20001/content/browser/service_worker/embedded_worker_test_helper.h File content/browser/service_worker/embedded_worker_test_helper.h (right): https://codereview.chromium.org/2678733002/diff/20001/content/browser/service_worker/embedded_worker_test_helper.h#newcode72 content/browser/service_worker/embedded_worker_test_helper.h:72: const std::vector<Event>& dispatched_events() const { return events_; ...
1 week, 6 days ago (2017-02-14 15:48:13 UTC) #25
shimazu
Thanks for updating! Added some comments. https://codereview.chromium.org/2678733002/diff/20001/content/browser/service_worker/embedded_worker_test_helper.h File content/browser/service_worker/embedded_worker_test_helper.h (right): https://codereview.chromium.org/2678733002/diff/20001/content/browser/service_worker/embedded_worker_test_helper.h#newcode234 content/browser/service_worker/embedded_worker_test_helper.h:234: std::vector<Event> events_; On ...
1 week, 5 days ago (2017-02-15 01:29:02 UTC) #26
xiaofengzhang
https://codereview.chromium.org/2678733002/diff/20001/content/browser/service_worker/embedded_worker_test_helper.h File content/browser/service_worker/embedded_worker_test_helper.h (right): https://codereview.chromium.org/2678733002/diff/20001/content/browser/service_worker/embedded_worker_test_helper.h#newcode72 content/browser/service_worker/embedded_worker_test_helper.h:72: const std::vector<Event>& dispatched_events() const { return events_; } On ...
1 week, 5 days ago (2017-02-15 01:59:46 UTC) #27
xiaofengzhang
On 2017/02/15 01:59:46, xiaofengzhang wrote: > https://codereview.chromium.org/2678733002/diff/20001/content/browser/service_worker/embedded_worker_test_helper.h > File content/browser/service_worker/embedded_worker_test_helper.h (right): > > https://codereview.chromium.org/2678733002/diff/20001/content/browser/service_worker/embedded_worker_test_helper.h#newcode72 > ...
1 week, 5 days ago (2017-02-15 02:03:21 UTC) #28
xiaofengzhang
https://codereview.chromium.org/2678733002/diff/20001/content/browser/service_worker/embedded_worker_test_helper.h File content/browser/service_worker/embedded_worker_test_helper.h (right): https://codereview.chromium.org/2678733002/diff/20001/content/browser/service_worker/embedded_worker_test_helper.h#newcode234 content/browser/service_worker/embedded_worker_test_helper.h:234: std::vector<Event> events_; On 2017/02/15 01:59:45, xiaofengzhang wrote: > On ...
1 week, 5 days ago (2017-02-15 02:15:30 UTC) #30
shimazu
https://codereview.chromium.org/2678733002/diff/20001/content/browser/service_worker/embedded_worker_test_helper.h File content/browser/service_worker/embedded_worker_test_helper.h (right): https://codereview.chromium.org/2678733002/diff/20001/content/browser/service_worker/embedded_worker_test_helper.h#newcode234 content/browser/service_worker/embedded_worker_test_helper.h:234: std::vector<Event> events_; On 2017/02/15 02:15:30, xiaofengzhang wrote: > On ...
1 week, 5 days ago (2017-02-15 02:30:12 UTC) #31
xiaofengzhang
https://codereview.chromium.org/2678733002/diff/20001/content/browser/service_worker/embedded_worker_test_helper.h File content/browser/service_worker/embedded_worker_test_helper.h (right): https://codereview.chromium.org/2678733002/diff/20001/content/browser/service_worker/embedded_worker_test_helper.h#newcode234 content/browser/service_worker/embedded_worker_test_helper.h:234: std::vector<Event> events_; On 2017/02/15 02:30:11, shimazu wrote: > On ...
1 week, 5 days ago (2017-02-15 02:39:30 UTC) #32
falken
https://codereview.chromium.org/2678733002/diff/20001/content/browser/service_worker/embedded_worker_test_helper.h File content/browser/service_worker/embedded_worker_test_helper.h (right): https://codereview.chromium.org/2678733002/diff/20001/content/browser/service_worker/embedded_worker_test_helper.h#newcode234 content/browser/service_worker/embedded_worker_test_helper.h:234: std::vector<Event> events_; On 2017/02/15 02:30:11, shimazu wrote: > On ...
1 week, 5 days ago (2017-02-15 03:13:24 UTC) #33
shimazu
lgtm with a nit, thanks! https://codereview.chromium.org/2678733002/diff/20001/content/browser/service_worker/embedded_worker_test_helper.h File content/browser/service_worker/embedded_worker_test_helper.h (right): https://codereview.chromium.org/2678733002/diff/20001/content/browser/service_worker/embedded_worker_test_helper.h#newcode234 content/browser/service_worker/embedded_worker_test_helper.h:234: std::vector<Event> events_; On 2017/02/15 ...
1 week, 5 days ago (2017-02-15 03:14:54 UTC) #34
falken
Hi xiaofengzhang, typically the patch author will upload a new patch incorporating comment feedback and ...
1 week, 5 days ago (2017-02-15 05:11:47 UTC) #35
xiaofengzhang
On 2017/02/15 05:11:47, falken wrote: > Hi xiaofengzhang, typically the patch author will upload a ...
1 week, 5 days ago (2017-02-15 05:18:08 UTC) #36
xiaofengzhang
@falken, @peter, please help review again, thanks https://codereview.chromium.org/2678733002/diff/1/content/browser/service_worker/embedded_worker_test_helper.h File content/browser/service_worker/embedded_worker_test_helper.h (right): https://codereview.chromium.org/2678733002/diff/1/content/browser/service_worker/embedded_worker_test_helper.h#newcode71 content/browser/service_worker/embedded_worker_test_helper.h:71: const std::vector<Event>& ...
1 week, 5 days ago (2017-02-15 06:14:45 UTC) #37
falken
https://codereview.chromium.org/2678733002/diff/20001/content/browser/service_worker/service_worker_job_unittest.cc File content/browser/service_worker/service_worker_job_unittest.cc (right): https://codereview.chromium.org/2678733002/diff/20001/content/browser/service_worker/service_worker_job_unittest.cc#newcode1470 content/browser/service_worker/service_worker_job_unittest.cc:1470: : SERVICE_WORKER_ERROR_EVENT_WAITUNTIL_REJECTED, On 2017/02/15 01:29:02, shimazu wrote: > On ...
1 week, 5 days ago (2017-02-15 07:32:03 UTC) #38
shimazu
https://codereview.chromium.org/2678733002/diff/20001/content/browser/service_worker/service_worker_job_unittest.cc File content/browser/service_worker/service_worker_job_unittest.cc (right): https://codereview.chromium.org/2678733002/diff/20001/content/browser/service_worker/service_worker_job_unittest.cc#newcode1470 content/browser/service_worker/service_worker_job_unittest.cc:1470: : SERVICE_WORKER_ERROR_EVENT_WAITUNTIL_REJECTED, On 2017/02/15 07:32:03, falken wrote: > On ...
1 week, 5 days ago (2017-02-15 08:25:35 UTC) #39
xiaofengzhang
Hi falken please help to review again, thanks :) https://codereview.chromium.org/2678733002/diff/20001/content/browser/service_worker/service_worker_job_unittest.cc File content/browser/service_worker/service_worker_job_unittest.cc (right): https://codereview.chromium.org/2678733002/diff/20001/content/browser/service_worker/service_worker_job_unittest.cc#newcode1470 content/browser/service_worker/service_worker_job_unittest.cc:1470: ...
1 week, 4 days ago (2017-02-16 06:17:50 UTC) #40
falken
lgtm. thank you
1 week, 4 days ago (2017-02-16 08:55:00 UTC) #41
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2678733002/80001
1 week, 3 days ago (2017-02-17 01:20:18 UTC) #44
xiaofengzhang
Hi shimazu, I received a email that patchset 4 needs a l-g-t-m from you, thanks.
1 week, 3 days ago (2017-02-17 01:23:36 UTC) #45
commit-bot: I haz the power
Try jobs failed on following builders: android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_clang_dbg_recipe/builds/213695) android_cronet on master.tryserver.chromium.android (JOB_FAILED, ...
1 week, 3 days ago (2017-02-17 01:27:13 UTC) #47
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2678733002/100001
1 week, 3 days ago (2017-02-17 02:33:05 UTC) #50
falken
On 2017/02/17 01:23:36, xiaofengzhang wrote: > Hi shimazu, > > I received a email that ...
1 week, 3 days ago (2017-02-17 02:47:29 UTC) #51
xiaofengzhang
On 2017/02/17 02:47:29, falken wrote: > On 2017/02/17 01:23:36, xiaofengzhang wrote: > > Hi shimazu, ...
1 week, 3 days ago (2017-02-17 02:50:02 UTC) #52
commit-bot: I haz the power
Try jobs failed on following builders: chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_asan_rel_ng on ...
1 week, 3 days ago (2017-02-17 04:36:26 UTC) #54
xiaofengzhang
Hi, Tom, would you PTAL for OWNER review of mojom/typemap files? Thanks.
1 week, 3 days ago (2017-02-17 06:59:12 UTC) #58
Tom Sepez
mojom LGTM
1 week, 2 days ago (2017-02-17 19:35:43 UTC) #61
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2678733002/100001
1 week, 1 day ago (2017-02-19 03:05:25 UTC) #63
commit-bot: I haz the power
1 week, 1 day ago (2017-02-19 04:59:29 UTC) #66
Message was sent while issue was closed.
Committed patchset #5 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/64975bc58e6b041ade1cfe2edb43...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f8e48bd