DevTools: introduce the bypass service worker checkbox.

DevTools: introduce the bypass service worker checkbox.

BUG=601263
Committed: https://crrev.com/c93539131ee3c12548ba833e688dc872773b5f49
Cr-Commit-Position: refs/heads/master@{#388296}

[pfeldman, 2016-04-08 00:45:57]

pfeldman@chromium.org changed reviewers:
+ dgozman@chromium.org, horo@chromium.org

[horo, 2016-04-08 05:41:15]

This change will introduce DCHECK failures of "DCHECK_EQ(NOT_DETERMINED,
response_type_);".

The association between the page and the SW is calculated by
PrepareForMainResource().
So we should call FallbackToNetwork() after the all logic for
PrepareForMainResource() is finished.

So please:
(1) Remove line 110-112.
(2) Change line 374.
  if (context_->fallback_to_network_for_devtools())
    job_->FallbackToNetwork();
  else
    job_->ForwardToServiceWorker();
(3) Change line 274-281.
  if (context_->fallback_to_network_for_devtools()) {
    job_->FallbackToNetwork();
    TRACE_EVENT_ASYNC_END2(
        "ServiceWorker",
        "ServiceWorkerControlleeRequestHandler::PrepareForMainResource",
        job_.get(), "Status", status, "Info",
        "Bypass worker for network is set in DevTools, so falling back to "
        "network");
  } else {
    job_->ForwardToServiceWorker();
    TRACE_EVENT_ASYNC_END2(
        "ServiceWorker",
        "ServiceWorkerControlleeRequestHandler::PrepareForMainResource",
        job_.get(), "Status", status, "Info", "Forwarded to the ServiceWorker");
  }
(4) Change line 304.
  if (context_->fallback_to_network_for_devtools())
    job_->FallbackToNetwork();
  else
    job_->ForwardToServiceWorker();

https://codereview.chromium.org/1872673002/diff/1/content/browser/service_wor...
File content/browser/service_worker/service_worker_controllee_request_handler.cc
(right):

https://codereview.chromium.org/1872673002/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_controllee_request_handler.cc:110:
if (context_->fallback_to_network_for_devtools())
(1) Remove line 110-112.

https://codereview.chromium.org/1872673002/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_controllee_request_handler.cc:274:
job_->ForwardToServiceWorker();
(3) Change here.

https://codereview.chromium.org/1872673002/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_controllee_request_handler.cc:304:
job_->ForwardToServiceWorker();
(4) Change here.

https://codereview.chromium.org/1872673002/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_controllee_request_handler.cc:374:
job_->ForwardToServiceWorker();
(2) Change here.

[pfeldman, 2016-04-08 19:30:49]

Hm. I was building with assertions and it did not fail on me. Also, things
worked correct according to the server log that I have.

With the changes you suggested, I trigger assertions on CORS requests in
DocumentThreadableLoader::handleResponse:
- wasFetchedViaServiceWorker return false
- requests is not same origin, so we

[horo, 2016-04-11 04:27:01]

On 2016/04/08 19:30:49, pfeldman wrote:
> Hm. I was building with assertions and it did not fail on me. Also, things
> worked correct according to the server log that I have.
> 
> With the changes you suggested, I trigger assertions on CORS requests in
> DocumentThreadableLoader::handleResponse:
> - wasFetchedViaServiceWorker return false
> - requests is not same origin, so we

When there is no SW registered in the origin, FallbackToNetwork is called twice.

ServiceWorkerControlleeRequestHandler::MaybeCreateJob()
 | if (context_->fallback_to_network_for_devtools())
 |   job_->FallbackToNetwork();
 | if (is_main_resource_load_)
 -> ServiceWorkerControlleeRequestHandler::PrepareForMainResource()
  -> ServiceWorkerStorage::FindRegistrationForDocument()
   | When ContainsKey() is false. (= There is no SW in the origin)
   -> CompleteFindNow()
    ->
ServiceWorkerControlleeRequestHandler::DidLookupRegistrationForMainResource()
     -> job_->FallbackToNetwork();


Ah, yes....
CORS fallback logic is complecated.
It may be a better solution to call request.setSkipServiceWorker(true) in
InspectorResourceAgent::willSendRequest().

[pfeldman, 2016-04-19 00:15:12]

Migrated to Blink.

[horo, 2016-04-19 09:42:37]

lgtm

[pfeldman, 2016-04-19 13:22:52]

The CQ bit was checked by pfeldman@chromium.org

[commit-bot: I haz the power, 2016-04-19 13:22:56]

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

[commit-bot: I haz the power, 2016-04-19 14:34:23]

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

[commit-bot: I haz the power, 2016-04-19 14:34:24]

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_...)

[pfeldman, 2016-04-19 19:04:09]

The CQ bit was checked by pfeldman@chromium.org

[commit-bot: I haz the power, 2016-04-19 19:04:53]

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

[commit-bot: I haz the power, 2016-04-19 20:22:50]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-04-22 19:15:48]

Description was changed from

==========
DevTools: introduce the bypass service worker checkbox.

BUG=601263
==========

to

==========
DevTools: introduce the bypass service worker checkbox.

BUG=601263

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

[commit-bot: I haz the power, 2016-04-22 19:15:49]

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