ServiceWorker: Store the existence of fetch event handler

ServiceWorker: Store the existence of fetch event handler

Currently |has_fetch_handler| is always set to true though the ServiceWorker script has no fetch event handler.
This patch passes the existence of fetch handler in the response to the install event, so the correct value will be stored.
This information will be used for bypassing ServiceWorker when no fetch handler is registered.

BUG=605844
TEST=./out/Default/content_unittests  --gtest_filter="ServiceWorkerJobTest.HasFetchHandler"
TEST=./out/Default/content_browsertests --gtest_filter="ServiceWorkerVersionBrowserTest.InstallWith*"

Committed: https://crrev.com/ceec8e3a24f7ad3a2bd7a64ae7f6d82f25dfabdb
Cr-Commit-Position: refs/heads/master@{#396381}

[shimazu, 2016-05-23 07:37:50]

Description was changed from

==========
ServiceWorker: Store the existence of fetch event handler 2/2

true is stored as |data.has_fetch_handler| on current implementation.
This information is needed for skipping SW when no fetch handler is registered.

BUG=605844
TEST=./out/Releasee/content_browsertests --gtest_filter='*ServiceWorker*'
==========

to

==========
ServiceWorker: Store the existence of fetch event handler 2/2

true is stored as |data.has_fetch_handler| on current implementation.
This information is needed for skipping SW when no fetch handler is registered.

1 Blink: https://codereview.chromium.org/2006593002
2 Chromium: This CL

BUG=605844
TEST=./out/Releasee/content_browsertests --gtest_filter='*ServiceWorker*'
==========

[shimazu, 2016-05-23 07:41:51]

shimazu@chromium.org changed reviewers:
+ falken@chromium.org, palmer@chromium.org

[shimazu, 2016-05-23 07:41:51]

PTAL:)

==

palmer@chromium.org: Please review changes in IPC

falken@chromium.org: Please review changes in the others

[shimazu, 2016-05-23 08:37:07]

shimazu@chromium.org changed reviewers:
+ keishi@chromium.org

[shimazu, 2016-05-23 08:37:08]

keishi-san: Sorry for the inconvenience, please check again.

[shimazu, 2016-05-23 08:37:27]

Description was changed from

==========
ServiceWorker: Store the existence of fetch event handler 2/2

true is stored as |data.has_fetch_handler| on current implementation.
This information is needed for skipping SW when no fetch handler is registered.

1 Blink: https://codereview.chromium.org/2006593002
2 Chromium: This CL

BUG=605844
TEST=./out/Releasee/content_browsertests --gtest_filter='*ServiceWorker*'
==========

to

==========
ServiceWorker: Store the existence of fetch event handler

true is stored as |data.has_fetch_handler| on current implementation.
This information is needed for skipping SW when no fetch handler is registered.

BUG=605844
TEST=./out/Releasee/content_browsertests --gtest_filter='*ServiceWorker*'
==========

[falken, 2016-05-23 08:41:12]

falken@chromium.org changed reviewers:
- keishi@chromium.org

[falken, 2016-05-23 08:41:13]

The choice of passing the flag in the install event response is a bit subtle,
but it makes sense. From the spec perspective, we should check whether the SW
has a fetch event handler after initial script evaluation, not after the install
event (see "set of event types to handle"). But since we store the SW upon
install event finishing, I think it makes sense to pass and handle the bit then.

We should also disallow adding a fetch event handler inside the install event
handler. But that can be a different patch/bug (see
https://bugs.chromium.org/p/chromium/issues/detail?id=594160)

https://codereview.chromium.org/2002883002/diff/1/content/browser/service_wor...
File content/browser/service_worker/embedded_worker_test_helper.cc (left):

https://codereview.chromium.org/2002883002/diff/1/content/browser/service_wor...
content/browser/service_worker/embedded_worker_test_helper.cc:116: void
EmbeddedWorkerTestHelper::SimulateAddProcessToPattern(
mistake?

https://codereview.chromium.org/2002883002/diff/1/content/browser/service_wor...
File content/browser/service_worker/service_worker_version.h (right):

https://codereview.chromium.org/2002883002/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_version.h:257: // to the result of
script evaluation.
I think DispatchEvent() (not DispatchSimpleEvent) was designed to handle
specialized callbacks. Can you use that instead of special casing Install? (It
looks like currently only "fetch" uses it.)

That would move handling the has_fetch_handler flag to ServiceWorkerRegisterJob,
which feels a bit cleaner, too.

[shimazu, 2016-05-23 10:44:24]

shimazu@chromium.org changed reviewers:
+ keishi@chromium.org

[shimazu, 2016-05-23 10:44:24]

https://codereview.chromium.org/2002883002/diff/1/content/browser/service_wor...
File content/browser/service_worker/embedded_worker_test_helper.cc (left):

https://codereview.chromium.org/2002883002/diff/1/content/browser/service_wor...
content/browser/service_worker/embedded_worker_test_helper.cc:116: void
EmbeddedWorkerTestHelper::SimulateAddProcessToPattern(
On 2016/05/23 08:41:13, falken wrote:
> mistake?

Oh,, This must be a mistake.
I haven't found which target includes this code. Could you tell me?

https://codereview.chromium.org/2002883002/diff/1/content/browser/service_wor...
File content/browser/service_worker/service_worker_version.h (right):

https://codereview.chromium.org/2002883002/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_version.h:257: // to the result of
script evaluation.
On 2016/05/23 08:41:13, falken wrote:
> I think DispatchEvent() (not DispatchSimpleEvent) was designed to handle
> specialized callbacks. Can you use that instead of special casing Install? (It
> looks like currently only "fetch" uses it.)
> 
> That would move handling the has_fetch_handler flag to
ServiceWorkerRegisterJob,
> which feels a bit cleaner, too.

I considered has_fetch_handler is a property which should be bound to
ServiceWorkerVersion, so that is updated only by ServiceWorkerVersion itself or
ServiceWorkerStorage in this patch.

What do you think?

[palmer, 2016-05-23 21:31:32]

https://codereview.chromium.org/2002883002/diff/40001/content/browser/service...
File content/browser/service_worker/service_worker_version.h (right):

https://codereview.chromium.org/2002883002/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_version.h:255: // For install
event, the callback should be specialized to update the stat
I don't understand this comment. Can you rewrite it to say who should specialize
the callback, and who should set |has_fetch_handler|? Also, is "stat" a typo for
"status"?

[falken, 2016-05-24 01:15:34]

https://codereview.chromium.org/2002883002/diff/1/content/browser/service_wor...
File content/browser/service_worker/embedded_worker_test_helper.cc (left):

https://codereview.chromium.org/2002883002/diff/1/content/browser/service_wor...
content/browser/service_worker/embedded_worker_test_helper.cc:116: void
EmbeddedWorkerTestHelper::SimulateAddProcessToPattern(
On 2016/05/23 10:44:24, makoto wrote:
> On 2016/05/23 08:41:13, falken wrote:
> > mistake?
> 
> Oh,, This must be a mistake.
> I haven't found which target includes this code. Could you tell me?

content_unittests

I often run:
out/Default/content_unittests --gtest_filter="ServiceWorker*:EmbeddedWorker*"
out/Default/content_browsertests --gtest_filter="ServiceWorker*:EmbeddedWorker*"
third_party/WebKit/Tools/Scripts/run-webkit-tests --target=Default
--no-retry-failures http/tests/serviceworker

when making SW changes.

https://codereview.chromium.org/2002883002/diff/1/content/browser/service_wor...
File content/browser/service_worker/service_worker_version.h (right):

https://codereview.chromium.org/2002883002/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_version.h:257: // to the result of
script evaluation.
On 2016/05/23 10:44:24, makoto wrote:
> On 2016/05/23 08:41:13, falken wrote:
> > I think DispatchEvent() (not DispatchSimpleEvent) was designed to handle
> > specialized callbacks. Can you use that instead of special casing Install?
(It
> > looks like currently only "fetch" uses it.)
> > 
> > That would move handling the has_fetch_handler flag to
> ServiceWorkerRegisterJob,
> > which feels a bit cleaner, too.
> 
> I considered has_fetch_handler is a property which should be bound to
> ServiceWorkerVersion, so that is updated only by ServiceWorkerVersion itself
or
> ServiceWorkerStorage in this patch.
> 
> What do you think?

But SWVersion is kind of transient: it can be destroyed and recreated for the
same worker. This patch is depending on the version getting persisted
immediately after setting the bit. We know that is what happens, but I'd rather
make that dependency explicit. The current patch does:
1. SWRegisterJob dispatches install event.
2. SWVersion sets the bit on itself.
3. SWRegisterJob gets the bit and persists the worker.

Alternatively, this seems clearer:
1. SWRegisterJob dispatches the install event.
2. SWRegisterJob install event callback is passed the bit and persists the
worker.

But more than that, the special casing of DispatchInstallEvent seems unnecessary
since we have DispatchEvent.

[shimazu, 2016-05-24 04:17:47]

PTAL again

https://codereview.chromium.org/2002883002/diff/1/content/browser/service_wor...
File content/browser/service_worker/embedded_worker_test_helper.cc (left):

https://codereview.chromium.org/2002883002/diff/1/content/browser/service_wor...
content/browser/service_worker/embedded_worker_test_helper.cc:116: void
EmbeddedWorkerTestHelper::SimulateAddProcessToPattern(
On 2016/05/24 01:15:34, falken wrote:
> On 2016/05/23 10:44:24, makoto wrote:
> > On 2016/05/23 08:41:13, falken wrote:
> > > mistake?
> > 
> > Oh,, This must be a mistake.
> > I haven't found which target includes this code. Could you tell me?
> 
> content_unittests
> 
> I often run:
> out/Default/content_unittests --gtest_filter="ServiceWorker*:EmbeddedWorker*"
> out/Default/content_browsertests
--gtest_filter="ServiceWorker*:EmbeddedWorker*"
> third_party/WebKit/Tools/Scripts/run-webkit-tests --target=Default
> --no-retry-failures http/tests/serviceworker
> 
> when making SW changes.

Thank you, I'll do it.

https://codereview.chromium.org/2002883002/diff/1/content/browser/service_wor...
File content/browser/service_worker/service_worker_version.h (right):

https://codereview.chromium.org/2002883002/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_version.h:257: // to the result of
script evaluation.
On 2016/05/24 01:15:34, falken wrote:
> On 2016/05/23 10:44:24, makoto wrote:
> > On 2016/05/23 08:41:13, falken wrote:
> > > I think DispatchEvent() (not DispatchSimpleEvent) was designed to handle
> > > specialized callbacks. Can you use that instead of special casing Install?
> (It
> > > looks like currently only "fetch" uses it.)
> > > 
> > > That would move handling the has_fetch_handler flag to
> > ServiceWorkerRegisterJob,
> > > which feels a bit cleaner, too.
> > 
> > I considered has_fetch_handler is a property which should be bound to
> > ServiceWorkerVersion, so that is updated only by ServiceWorkerVersion itself
> or
> > ServiceWorkerStorage in this patch.
> > 
> > What do you think?
> 
> But SWVersion is kind of transient: it can be destroyed and recreated for the
> same worker. This patch is depending on the version getting persisted
> immediately after setting the bit. We know that is what happens, but I'd
rather
> make that dependency explicit. The current patch does:
> 1. SWRegisterJob dispatches install event.
> 2. SWVersion sets the bit on itself.
> 3. SWRegisterJob gets the bit and persists the worker.
> 
> Alternatively, this seems clearer:
> 1. SWRegisterJob dispatches the install event.
> 2. SWRegisterJob install event callback is passed the bit and persists the
> worker.
> 
> But more than that, the special casing of DispatchInstallEvent seems
unnecessary
> since we have DispatchEvent.

Thanks. Done.

https://codereview.chromium.org/2002883002/diff/40001/content/browser/service...
File content/browser/service_worker/service_worker_version.h (right):

https://codereview.chromium.org/2002883002/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_version.h:255: // For install
event, the callback should be specialized to update the stat
On 2016/05/23 21:31:32, palmer wrote:
> I don't understand this comment. Can you rewrite it to say who should
specialize
> the callback, and who should set |has_fetch_handler|? Also, is "stat" a typo
for
> "status"?

Thanks for your review. I removed this code.

[falken, 2016-05-24 06:25:48]

Looking good. We'll also need tests of some sort. For this feature a browser
test in service_worker_browsertest may be good. You can use the existing files
worker.js (no fetch event handler) and fetch_event.js (with fetch handler).
Register them and then check that the bit is set appropriately.

https://codereview.chromium.org/2002883002/diff/60001/content/browser/service...
File content/browser/service_worker/service_worker_register_job.cc (right):

https://codereview.chromium.org/2002883002/diff/60001/content/browser/service...
content/browser/service_worker/service_worker_register_job.cc:447: status =
SERVICE_WORKER_ERROR_EVENT_WAITUNTIL_REJECTED;
This construct is a bit scary. If we someday add a new error enum value it'll
default to OK unless this code is updated. I'd recommend a switch/case.
(I understand this is taken from SWVersion, but prefer to fix it rather than
repeating.)

BTW blink::WebServiceWorkerEventResultRejected looks misnamed/misdocumented. We
use this error code even the event failed due to a runtime exception. But we
don't have to fix in this patch.

https://codereview.chromium.org/2002883002/diff/60001/content/browser/service...
content/browser/service_worker/service_worker_register_job.cc:449:
ServiceWorkerMetrics::RecordInstallEventStatus(status);
I think you need this in OnInstallFailed() also.

https://codereview.chromium.org/2002883002/diff/60001/content/browser/service...
content/browser/service_worker/service_worker_register_job.cc:452:
ServiceWorkerStatusToString(status));
Prefer to call OnInstallFailed over copy/pasting the code.

[shimazu, 2016-05-24 08:30:27]

Thank you for the reviews:)

I also added the two tests.

https://codereview.chromium.org/2002883002/diff/60001/content/browser/service...
File content/browser/service_worker/service_worker_register_job.cc (right):

https://codereview.chromium.org/2002883002/diff/60001/content/browser/service...
content/browser/service_worker/service_worker_register_job.cc:447: status =
SERVICE_WORKER_ERROR_EVENT_WAITUNTIL_REJECTED;
On 2016/05/24 06:25:48, falken wrote:
> This construct is a bit scary. If we someday add a new error enum value it'll
> default to OK unless this code is updated. I'd recommend a switch/case.
> (I understand this is taken from SWVersion, but prefer to fix it rather than
> repeating.)
> 
> BTW blink::WebServiceWorkerEventResultRejected looks misnamed/misdocumented.
We
> use this error code even the event failed due to a runtime exception. But we
> don't have to fix in this patch.

Done.

https://codereview.chromium.org/2002883002/diff/60001/content/browser/service...
content/browser/service_worker/service_worker_register_job.cc:449:
ServiceWorkerMetrics::RecordInstallEventStatus(status);
On 2016/05/24 06:25:48, falken wrote:
> I think you need this in OnInstallFailed() also.

Done.

https://codereview.chromium.org/2002883002/diff/60001/content/browser/service...
content/browser/service_worker/service_worker_register_job.cc:452:
ServiceWorkerStatusToString(status));
On 2016/05/24 06:25:48, falken wrote:
> Prefer to call OnInstallFailed over copy/pasting the code.

Done.

[keishi, 2016-05-24 08:38:50]

web/ LGTM

[falken, 2016-05-25 01:19:35]

Thanks for the test. I forgot that the browsertest would not actually exercise
SWRegisterJob. So now we're testing the renderer-side part that the fetch
handler bit is passed  correctly according to the script, but not yet the
browser-side part that it's being persisted. Could you add a test to
service_worker_job_unittest that tests that SWRegisterJob will store the
has_fetch_handler bit according to the OnInstallFinished callback? For this you
could override EmbeddedWorkerTestHelper::OnInstallEvent to pass true/false for
has_fetch_handler.

https://codereview.chromium.org/2002883002/diff/80001/content/browser/service...
File content/browser/service_worker/service_worker_register_job.cc (right):

https://codereview.chromium.org/2002883002/diff/80001/content/browser/service...
content/browser/service_worker/service_worker_register_job.cc:452: break;
default: NOTREACHED()

[shimazu, 2016-05-25 03:35:13]

PTAL again:)


> Thanks for the test. I forgot that the browsertest would not actually exercise
> SWRegisterJob. So now we're testing the renderer-side part that the fetch
> handler bit is passed  correctly according to the script, but not yet the
> browser-side part that it's being persisted. Could you add a test to
> service_worker_job_unittest that tests that SWRegisterJob will store the
> has_fetch_handler bit according to the OnInstallFinished callback? For this
you
> could override EmbeddedWorkerTestHelper::OnInstallEvent to pass true/false for
> has_fetch_handler.

I could add a browser-side test by extending EventCallbackHelper.

https://codereview.chromium.org/2002883002/diff/80001/content/browser/service...
File content/browser/service_worker/service_worker_register_job.cc (right):

https://codereview.chromium.org/2002883002/diff/80001/content/browser/service...
content/browser/service_worker/service_worker_register_job.cc:452: break;
On 2016/05/25 01:19:35, falken wrote:
> default: NOTREACHED()

Done.

[falken, 2016-05-25 04:30:17]

looks good

https://codereview.chromium.org/2002883002/diff/100001/content/browser/servic...
File content/browser/service_worker/service_worker_browsertest.cc (right):

https://codereview.chromium.org/2002883002/diff/100001/content/browser/servic...
content/browser/service_worker/service_worker_browsertest.cc:891:
ASSERT_TRUE(version_->has_fetch_handler());
EXPECT_TRUE (see
https://github.com/google/googletest/blob/master/googletest/docs/Primer.md)

https://codereview.chromium.org/2002883002/diff/100001/content/browser/servic...
content/browser/service_worker/service_worker_browsertest.cc:897:
ASSERT_FALSE(version_->has_fetch_handler());
ditto

https://codereview.chromium.org/2002883002/diff/100001/content/browser/servic...
File content/browser/service_worker/service_worker_job_unittest.cc (right):

https://codereview.chromium.org/2002883002/diff/100001/content/browser/servic...
content/browser/service_worker/service_worker_job_unittest.cc:1573: registration
= FindRegistrationForPattern(pattern);
I think FindRegistraitonForPattern will ultimately just return the live
registration, so you'll just get registration back again. I'd just simplify
these paragraphs to:

helper->set_has_fetch_handler(true);
RunRegisterJob(pattern, script);
registration = FindRegistrationForPattern(pattern);
EXPECT_TRUE(registration->active_version()->has_fetch_handler());
RunUnregisterJob(pattern)

helper->set_has_fetch_handler(false);
...

[shimazu, 2016-05-25 07:22:39]

https://codereview.chromium.org/2002883002/diff/100001/content/browser/servic...
File content/browser/service_worker/service_worker_browsertest.cc (right):

https://codereview.chromium.org/2002883002/diff/100001/content/browser/servic...
content/browser/service_worker/service_worker_browsertest.cc:891:
ASSERT_TRUE(version_->has_fetch_handler());
On 2016/05/25 04:30:17, falken wrote:
> EXPECT_TRUE (see
> https://github.com/google/googletest/blob/master/googletest/docs/Primer.md)

Done.

https://codereview.chromium.org/2002883002/diff/100001/content/browser/servic...
content/browser/service_worker/service_worker_browsertest.cc:897:
ASSERT_FALSE(version_->has_fetch_handler());
On 2016/05/25 04:30:17, falken wrote:
> ditto

Done.

https://codereview.chromium.org/2002883002/diff/100001/content/browser/servic...
File content/browser/service_worker/service_worker_job_unittest.cc (right):

https://codereview.chromium.org/2002883002/diff/100001/content/browser/servic...
content/browser/service_worker/service_worker_job_unittest.cc:1573: registration
= FindRegistrationForPattern(pattern);
On 2016/05/25 04:30:17, falken wrote:
> I think FindRegistraitonForPattern will ultimately just return the live
> registration, so you'll just get registration back again. I'd just simplify
> these paragraphs to:
> 
> helper->set_has_fetch_handler(true);
> RunRegisterJob(pattern, script);
> registration = FindRegistrationForPattern(pattern);
> EXPECT_TRUE(registration->active_version()->has_fetch_handler());
> RunUnregisterJob(pattern)
> 
> helper->set_has_fetch_handler(false);
> ...

Done.

[falken, 2016-05-25 08:00:01]

Code looks good.

The commit description is a bit hard to understand. It's unclear what is the
behavior before the patch, behavior after the patch, and the future plans. To be
thorough, I would hit these points:
1) Before this patch, |has_fetch_handler| was always stored set to true
2) This patch passes the existence of fetch handler in the response to the
install event, so |has_fetch_handler| is stored with the correct value
3) Future patches will use this information for skipping blah blah

[shimazu, 2016-05-26 02:33:18]

Description was changed from

==========
ServiceWorker: Store the existence of fetch event handler

true is stored as |data.has_fetch_handler| on current implementation.
This information is needed for skipping SW when no fetch handler is registered.

BUG=605844
TEST=./out/Releasee/content_browsertests --gtest_filter='*ServiceWorker*'
==========

to

==========
ServiceWorker: Store the existence of fetch event handler

Currently |has_fetch_handler| is always set to true though the ServiceWorker
script has no fetch event handler.
This patch passes the existence of fetch handler in the response to the install
event, so the correct value will be stored.
This information will be used for bypassing ServiceWorker when no fetch handler
is registered.

BUG=605844
TEST=./out/Default/content_unittests 
--gtest_filter="ServiceWorkerJobTest.HasFetchHandler"
TEST=./out/Releasee/content_browsertests
--gtest_filter="ServiceWorkerVersionBrowserTest.InstallWith*"
==========

[shimazu, 2016-05-26 02:34:15]

On 2016/05/25 08:00:01, falken wrote:
> Code looks good.
> 
> The commit description is a bit hard to understand. It's unclear what is the
> behavior before the patch, behavior after the patch, and the future plans. To
be
> thorough, I would hit these points:
> 1) Before this patch, |has_fetch_handler| was always stored set to true
> 2) This patch passes the existence of fetch handler in the response to the
> install event, so |has_fetch_handler| is stored with the correct value
> 3) Future patches will use this information for skipping blah blah

Thanks, updated.

[falken, 2016-05-26 03:05:25]

lgtm

The TEST= line is optional, but you might want to make it consistent whether
it's out/Default or out/Release (or just remove the directory part of the path)

TEST=./out/Default/content_unittests 
--gtest_filter="ServiceWorkerJobTest.HasFetchHandler"
TEST=./out/Releasee/content_browsertests
--gtest_filter="ServiceWorkerVersionBrowserTest.InstallWith*"

[shimazu, 2016-05-26 03:06:01]

Description was changed from

==========
ServiceWorker: Store the existence of fetch event handler

Currently |has_fetch_handler| is always set to true though the ServiceWorker
script has no fetch event handler.
This patch passes the existence of fetch handler in the response to the install
event, so the correct value will be stored.
This information will be used for bypassing ServiceWorker when no fetch handler
is registered.

BUG=605844
TEST=./out/Default/content_unittests 
--gtest_filter="ServiceWorkerJobTest.HasFetchHandler"
TEST=./out/Releasee/content_browsertests
--gtest_filter="ServiceWorkerVersionBrowserTest.InstallWith*"
==========

to

==========
ServiceWorker: Store the existence of fetch event handler

Currently |has_fetch_handler| is always set to true though the ServiceWorker
script has no fetch event handler.
This patch passes the existence of fetch handler in the response to the install
event, so the correct value will be stored.
This information will be used for bypassing ServiceWorker when no fetch handler
is registered.

BUG=605844
TEST=./out/Default/content_unittests 
--gtest_filter="ServiceWorkerJobTest.HasFetchHandler"
TEST=./out/Default/content_browsertests
--gtest_filter="ServiceWorkerVersionBrowserTest.InstallWith*"
==========

[palmer, 2016-05-26 20:39:23]

lgtm

[shimazu, 2016-05-27 01:52:37]

The CQ bit was checked by shimazu@chromium.org

[shimazu, 2016-05-27 01:52:38]

The patchset sent to the CQ was uploaded after l-g-t-m from keishi@chromium.org
Link to the patchset: https://codereview.chromium.org/2002883002/#ps120001
(title: "Use EXPECT_* and remove unnecessary checks")

[commit-bot: I haz the power, 2016-05-27 01:52:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2002883002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2002883002/120001

[commit-bot: I haz the power, 2016-05-27 01:55:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-27 01:55:10]

Try jobs failed on following builders:
  ios-device on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)
  ios-simulator on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)

[shimazu, 2016-05-27 02:32:46]

The CQ bit was checked by shimazu@chromium.org

[shimazu, 2016-05-27 02:32:47]

The patchset sent to the CQ was uploaded after l-g-t-m from palmer@chromium.org,
falken@chromium.org, keishi@chromium.org
Link to the patchset: https://codereview.chromium.org/2002883002/#ps140001
(title: "Rebase")

[commit-bot: I haz the power, 2016-05-27 02:33:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2002883002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2002883002/140001

[commit-bot: I haz the power, 2016-05-27 04:34:16]

Description was changed from

==========
ServiceWorker: Store the existence of fetch event handler

Currently |has_fetch_handler| is always set to true though the ServiceWorker
script has no fetch event handler.
This patch passes the existence of fetch handler in the response to the install
event, so the correct value will be stored.
This information will be used for bypassing ServiceWorker when no fetch handler
is registered.

BUG=605844
TEST=./out/Default/content_unittests 
--gtest_filter="ServiceWorkerJobTest.HasFetchHandler"
TEST=./out/Default/content_browsertests
--gtest_filter="ServiceWorkerVersionBrowserTest.InstallWith*"
==========

to

==========
ServiceWorker: Store the existence of fetch event handler

Currently |has_fetch_handler| is always set to true though the ServiceWorker
script has no fetch event handler.
This patch passes the existence of fetch handler in the response to the install
event, so the correct value will be stored.
This information will be used for bypassing ServiceWorker when no fetch handler
is registered.

BUG=605844
TEST=./out/Default/content_unittests 
--gtest_filter="ServiceWorkerJobTest.HasFetchHandler"
TEST=./out/Default/content_browsertests
--gtest_filter="ServiceWorkerVersionBrowserTest.InstallWith*"
==========

[commit-bot: I haz the power, 2016-05-27 04:34:17]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-05-27 04:36:02]

Description was changed from

==========
ServiceWorker: Store the existence of fetch event handler

Currently |has_fetch_handler| is always set to true though the ServiceWorker
script has no fetch event handler.
This patch passes the existence of fetch handler in the response to the install
event, so the correct value will be stored.
This information will be used for bypassing ServiceWorker when no fetch handler
is registered.

BUG=605844
TEST=./out/Default/content_unittests 
--gtest_filter="ServiceWorkerJobTest.HasFetchHandler"
TEST=./out/Default/content_browsertests
--gtest_filter="ServiceWorkerVersionBrowserTest.InstallWith*"
==========

to

==========
ServiceWorker: Store the existence of fetch event handler

Currently |has_fetch_handler| is always set to true though the ServiceWorker
script has no fetch event handler.
This patch passes the existence of fetch handler in the response to the install
event, so the correct value will be stored.
This information will be used for bypassing ServiceWorker when no fetch handler
is registered.

BUG=605844
TEST=./out/Default/content_unittests 
--gtest_filter="ServiceWorkerJobTest.HasFetchHandler"
TEST=./out/Default/content_browsertests
--gtest_filter="ServiceWorkerVersionBrowserTest.InstallWith*"

Committed: https://crrev.com/ceec8e3a24f7ad3a2bd7a64ae7f6d82f25dfabdb
Cr-Commit-Position: refs/heads/master@{#396381}
==========

[commit-bot: I haz the power, 2016-05-27 04:36:04]

Patchset 8 (id:??) landed as
https://crrev.com/ceec8e3a24f7ad3a2bd7a64ae7f6d82f25dfabdb
Cr-Commit-Position: refs/heads/master@{#396381}