ServiceWorker: Prevent worker script with incorrect MIME type to be registered.

ServiceWorker: Prevent worker script with incorrect MIME type to be registered.

Fix for spec update: https://github.com/slightlyoff/ServiceWorker/issues/380.
A valid worker script MIME type should be one of below:
application/x-javascript
text/javascript
application/javascript

BUG=400604
TEST=https://codereview.chromium.org/448063002/

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=288922

[xiang, 2014-08-07 05:16:15]

PTAL, thanks!
The layout test is added in: https://codereview.chromium.org/448063002/.

[falken, 2014-08-07 07:15:13]

LGTM

People sometimes like to see how the change can be tested in the CL description.
For patches like this I often add a line like
TEST=https://codereview.chromium.org/448063002/ under the BUG= line.

[michaeln, 2014-08-07 20:00:03]

https://codereview.chromium.org/446323002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_write_to_cache_job.cc
(right):

https://codereview.chromium.org/446323002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_write_to_cache_job.cc:307:
request->GetResponseHeaderByName("content-type", &content_type);
lgtm except please use GetMimeType() here so that any charset extensions are
stripped off.

[xiang, 2014-08-11 01:43:02]

On 2014/08/07 20:00:03, michaeln wrote:
>
https://codereview.chromium.org/446323002/diff/1/content/browser/service_work...
> File content/browser/service_worker/service_worker_write_to_cache_job.cc
> (right):
> 
>
https://codereview.chromium.org/446323002/diff/1/content/browser/service_work...
> content/browser/service_worker/service_worker_write_to_cache_job.cc:307:
> request->GetResponseHeaderByName("content-type", &content_type);
> lgtm except please use GetMimeType() here so that any charset extensions are
> stripped off.

Done. Thanks!

[xiang, 2014-08-11 01:43:14]

The CQ bit was checked by xiang.long@intel.com

[commit-bot: I haz the power, 2014-08-11 01:46:09]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/xiang.long@intel.com/446323002/20001

[xiang, 2014-08-11 04:52:11]

The CQ bit was unchecked by xiang.long@intel.com

[xiang, 2014-08-11 07:06:13]

Updated the CL to fix browsertests failures.
PTAL, thanks!

[Lei Zhang, 2014-08-11 18:29:32]

chrome/browser/chrome_service_worker_browsertest.cc lgtm stamp

[xiang, 2014-08-12 01:22:28]

The CQ bit was checked by xiang.long@intel.com

[commit-bot: I haz the power, 2014-08-12 01:33:33]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/xiang.long@intel.com/446323002/40001

[commit-bot: I haz the power, 2014-08-12 05:49:24]

Change committed as 288922

[falken, 2014-08-12 07:36:42]

On 2014/08/12 05:49:24, I haz the power (commit-bot) wrote:
> Change committed as 288922

Unfortunately, this CL seems to have collided with
https://codereview.chromium.org/434453005

Now the test that was added in the other CL is failing:
http://build.chromium.org/p/chromium.linux/builders/Linux%20Tests/builds/12883

I don't see an obvious fix, so will revert this change.

[tapted, 2014-08-12 07:38:27]

A revert of this CL has been created in
https://codereview.chromium.org/459383002/ by tapted@chromium.org.

The reason for reverting is: Suspected for breakage on Mac waterfall bots
(10.6/10.7)

test ServiceWorkerBrowserTest.ImportsBustMemcache failing since

http://build.chromium.org/p/chromium.mac/builders/Mac10.6%20Tests%20%281%29/b...
http://build.chromium.org/p/chromium.mac/builders/Mac10.7%20Tests%20%281%29/b...

Times out. errors like

[ RUN      ] ServiceWorkerBrowserTest.ImportsBustMemcache
[16435:263:0812/001516:2160051331590:INFO:CONSOLE(13)] "initial unregistration
done", source: http://127.0.0.1:58683/service_worker/imports_bust_memcache.html
(13)
BrowserTestBase signal handler received SIGTERM. Backtrace:
0   content_browsertests                0x6249bcaa
base::debug::StackTrace::StackTrace() + 26
..
11  AppKit                              0x96eeddd6 -[NSApplication
nextEventMatchingMask:untilDate:inMode:dequeue:] + 156
12  AppKit                              0x96eb01f3 -[NSApplication run] + 821
..
19  content_browsertests                0x6228bf46
content::TitleWatcher::WaitAndGetTitle() + 118
20  content_browsertests                0x61ed5252
content::ServiceWorkerBrowserTest_ImportsBustMemcache_Test::RunTestOnMainThread()
+ 530
[649/649] ServiceWorkerBrowserTest.ImportsBustMemcache (TIMED OUT).

[xiang, 2014-08-12 08:17:39]

On 2014/08/12 07:36:42, falken wrote:
> On 2014/08/12 05:49:24, I haz the power (commit-bot) wrote:
> > Change committed as 288922
> 
> Unfortunately, this CL seems to have collided with
> https://codereview.chromium.org/434453005
> 
> Now the test that was added in the other CL is failing:
> http://build.chromium.org/p/chromium.linux/builders/Linux%20Tests/builds/12883
> 
> I don't see an obvious fix, so will revert this change.

It's caused by the new browsertest from
https://codereview.chromium.org/434453005.
The interceptors need return header with MIME type specified.

Do I need to create a new CL as this's closed?

[falken, 2014-08-12 08:23:55]

On 2014/08/12 08:17:39, xiang wrote:
> On 2014/08/12 07:36:42, falken wrote:
> > On 2014/08/12 05:49:24, I haz the power (commit-bot) wrote:
> > > Change committed as 288922
> > 
> > Unfortunately, this CL seems to have collided with
> > https://codereview.chromium.org/434453005
> > 
> > Now the test that was added in the other CL is failing:
> >
http://build.chromium.org/p/chromium.linux/builders/Linux%20Tests/builds/12883
> > 
> > I don't see an obvious fix, so will revert this change.
> 
> It's caused by the new browsertest from
> https://codereview.chromium.org/434453005.
> The interceptors need return header with MIME type specified.
> 
> Do I need to create a new CL as this's closed?

I see. Yep, please make a new CL for relanding.

I'll lg.tm it. If the fix is trivial enough you can also just TBR.