[ServiceWorker] Plumbing the request credentials mode to the ServiceWorker. [2/2 chromium]

[ServiceWorker] Plumbing the request credentials mode to the ServiceWorker. [2/2 chromium]

Currently the credentials mode of the FetchEvent's request is always 'omit'.
But this should be 'same-origin' or 'include'.
ex:
- <img src="./test.png">
 mode: no CORS
 credentials mode: same-origin
- <img src="./test.png" crossOrigin="anonymous">
 mode: CORS
 credentials mode: same-origin
- <img src="./test.png" crossOrigin="use-credentials">
 mode: CORS
 credentials mode: include

[1/2] blink: https://codereview.chromium.org/610403002/
[2/2] chromium: https://codereview.chromium.org/615493003/

This value is passed from the renederer to the ServiceWorker in the following steps.

In the renederer process:
  blink::ResourceRequest::setFetchCredentialsMode()  // not called yet
 blink::ResourceRequest::m_fetchCredentialsMode
  content::WebURLLoaderImpl::Context::Start()
   blink::WebURLRequest::fetchCredentialsMode()
    GetFetchCredentialsMode()
 conetnt::RequestInfo::fetch_credentials_mode

In the browser process:
 conetnt::RequestInfo::fetch_credentials_mode
  content::ResourceDispatcherHostImpl::BeginRequest()
   content::ServiceWorkerRequestHandler::InitializeHandler()
    content::ServiceWorkerProviderHost::CreateRequestHandler()
     content::ServiceWorkerControlleeRequestHandler::ServiceWorkerControlleeRequestHandler()
 content::ServiceWorkerControlleeRequestHandler::request_credentials_mode_
  content::ServiceWorkerControlleeRequestHandler::MaybeCreateJob()
   content::ServiceWorkerURLRequestJob::ServiceWorkerURLRequestJob()
 content::ServiceWorkerURLRequestJob::request_credentials_mode_
  content::ServiceWorkerURLRequestJob::CreateFetchRequest()
 content::ServiceWorkerFetchRequest::credentials_mode

In the ServiceWorker process:
 content::ServiceWorkerFetchRequest::credentials_mode
   content::ServiceWorkerScriptContext::OnFetchEvent()
   GetBlinkFetchRequestCredentialsMode()
   blink::WebServiceWorkerRequest::setCredentialsMode()
 blink::WebServiceWorkerRequest::WebServiceWorkerRequestPrivate::m_credentialsMode
  blink::Request::create()
  blink::FetchRequestData::create()
 blink::FetchRequestData::m_credentials

BUG=418509
Committed: https://crrev.com/89252e8f8b163ac1c8715882e3651aa79e6cbfb6
Cr-Commit-Position: refs/heads/master@{#297812}

[horo, 2014-09-29 11:20:36]

horo@chromium.org changed reviewers:
+ yhirano@chromium.org

[horo, 2014-09-29 11:20:37]

yhirano@
Could you please review?

[horo, 2014-09-30 09:20:47]

Patchset #2 (id:20001) has been deleted

[yhirano, 2014-10-01 06:41:12]

https://codereview.chromium.org/615493003/diff/60001/content/browser/service_...
File content/browser/service_worker/service_worker_url_request_job.cc (right):

https://codereview.chromium.org/615493003/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_url_request_job.cc:21: #include
"net/base/load_flags.h"
Is this include directive needed?

https://codereview.chromium.org/615493003/diff/60001/content/child/web_url_lo...
File content/child/web_url_loader_impl.cc (right):

https://codereview.chromium.org/615493003/diff/60001/content/child/web_url_lo...
content/child/web_url_loader_impl.cc:410: 
How about placing assertions here?
credentials mode is omit => credentials flag is unset
credentials mode is include => credentials flag is set

https://codereview.chromium.org/615493003/diff/60001/content/common/resource_...
File content/common/resource_messages.h (right):

https://codereview.chromium.org/615493003/diff/60001/content/common/resource_...
content/common/resource_messages.h:205:
IPC_STRUCT_MEMBER(content::FetchCredentialsMode, fetch_credentials_mode)
The mode value is not set in resource_dispatcher.cc.

[horo, 2014-10-01 06:57:34]

https://codereview.chromium.org/615493003/diff/60001/content/browser/service_...
File content/browser/service_worker/service_worker_url_request_job.cc (right):

https://codereview.chromium.org/615493003/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_url_request_job.cc:21: #include
"net/base/load_flags.h"
On 2014/10/01 06:41:12, yhirano wrote:
> Is this include directive needed?

Done.

https://codereview.chromium.org/615493003/diff/60001/content/child/web_url_lo...
File content/child/web_url_loader_impl.cc (right):

https://codereview.chromium.org/615493003/diff/60001/content/child/web_url_lo...
content/child/web_url_loader_impl.cc:410: 
On 2014/10/01 06:41:12, yhirano wrote:
> How about placing assertions here?
> credentials mode is omit => credentials flag is unset
> credentials mode is include => credentials flag is set

Currently we can't add these assertions because credentials_mode is not set
correctly yet.
I will add in the future CL.

https://codereview.chromium.org/615493003/diff/60001/content/common/resource_...
File content/common/resource_messages.h (right):

https://codereview.chromium.org/615493003/diff/60001/content/common/resource_...
content/common/resource_messages.h:205:
IPC_STRUCT_MEMBER(content::FetchCredentialsMode, fetch_credentials_mode)
On 2014/10/01 06:41:12, yhirano wrote:
> The mode value is not set in resource_dispatcher.cc.

Done.

[yhirano, 2014-10-01 07:59:47]

lgtm

[yhirano, 2014-10-01 08:01:08]

Please update the description like the blink-side CL.

[horo, 2014-10-01 08:02:49]

On 2014/10/01 08:01:08, yhirano wrote:
> Please update the description like the blink-side CL.

done

[horo, 2014-10-01 08:03:51]

horo@chromium.org changed reviewers:
+ jochen@chromium.org

[horo, 2014-10-01 08:03:52]

jochen@
Could you please review this?
Thank you.

[Mike West, 2014-10-01 08:27:19]

mkwst@chromium.org changed reviewers:
+ mkwst@chromium.org, tsepez@chromium.org

[Mike West, 2014-10-01 08:27:20]

Drive-by LGTM. You'll need approval for the IPC change; Tom, would you mind
signing off on that pretty trivial enum addition?

https://codereview.chromium.org/615493003/diff/60001/content/child/web_url_lo...
File content/child/web_url_loader_impl.cc (right):

https://codereview.chromium.org/615493003/diff/60001/content/child/web_url_lo...
content/child/web_url_loader_impl.cc:410: 
On 2014/10/01 06:57:34, horo wrote:
> On 2014/10/01 06:41:12, yhirano wrote:
> > How about placing assertions here?
> > credentials mode is omit => credentials flag is unset
> > credentials mode is include => credentials flag is set
> 
> Currently we can't add these assertions because credentials_mode is not set
> correctly yet.
> I will add in the future CL.

Can you add a TODO comment here to that effect?

[horo, 2014-10-01 08:36:02]

https://codereview.chromium.org/615493003/diff/60001/content/child/web_url_lo...
File content/child/web_url_loader_impl.cc (right):

https://codereview.chromium.org/615493003/diff/60001/content/child/web_url_lo...
content/child/web_url_loader_impl.cc:410: 
On 2014/10/01 08:27:20, Mike West wrote:
> On 2014/10/01 06:57:34, horo wrote:
> > On 2014/10/01 06:41:12, yhirano wrote:
> > > How about placing assertions here?
> > > credentials mode is omit => credentials flag is unset
> > > credentials mode is include => credentials flag is set
> > 
> > Currently we can't add these assertions because credentials_mode is not set
> > correctly yet.
> > I will add in the future CL.
> 
> Can you add a TODO comment here to that effect?

Done.

[Tom Sepez, 2014-10-01 17:17:08]

Messages LGTM.

[jochen (gone - plz use gerrit), 2014-10-02 07:35:56]

lgtm

[horo, 2014-10-02 10:39:04]

The CQ bit was checked by horo@chromium.org

[commit-bot: I haz the power, 2014-10-02 10:39:57]

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

[commit-bot: I haz the power, 2014-10-02 10:51:57]

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

[commit-bot: I haz the power, 2014-10-02 10:51:59]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[horo, 2014-10-02 10:54:32]

The CQ bit was checked by horo@chromium.org

[commit-bot: I haz the power, 2014-10-02 10:55:21]

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

[commit-bot: I haz the power, 2014-10-02 11:08:52]

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

[commit-bot: I haz the power, 2014-10-02 11:08:53]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[horo, 2014-10-02 11:22:01]

The CQ bit was checked by horo@chromium.org

[commit-bot: I haz the power, 2014-10-02 11:22:25]

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

[commit-bot: I haz the power, 2014-10-02 12:03:53]

Committed patchset #7 (id:140001) as 9a4165b157b2f35e3f1aed1fcf5574f0167ed943

[commit-bot: I haz the power, 2014-10-02 12:04:28]

Patchset 7 (id:??) landed as
https://crrev.com/89252e8f8b163ac1c8715882e3651aa79e6cbfb6
Cr-Commit-Position: refs/heads/master@{#297812}