Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2451663002:
Add check for PlzNavigate in ServiceWorkerFetchDispatcher. (Closed)
|
Created:
4 years, 1 month ago by horo Modified:
4 years, 1 month ago Reviewers:
falken CC:
chromium-reviews, michaeln, jsbell+serviceworker_chromium.org, shimazu+serviceworker_chromium.org, serviceworker-reviews, jam, nhiroki, kinuko+serviceworker, horo+watch_chromium.org, darin-cc_chromium.org, kinuko+watch, tzik, blink-worker-reviews_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd check for PlzNavigate in ServiceWorkerFetchDispatcher.
ServiceWorkerProviderHost::PreCreateNavigationHost() creates a ServiceWorkerProviderHost
without ServiceWorkerDispatcherHost. So MaybeStartNavigationPreload() is called with null
MojoURLLoaderFactoryGetter. This will cause crashes when PlzNavigate is enabled.
I want to disable NavigationPreload for PlzNavigate in this CL.
BUG=649558
Committed: https://crrev.com/193ab9f14c15355404ec9bbfe002dd48980a465a
Cr-Commit-Position: refs/heads/master@{#427310}
Patch Set 1 #
Total comments: 2
Patch Set 2 : add NOTIMPLEMENTED() #Messages
Total messages: 17 (9 generated)
Description was changed from ========== Add check for PlzNavigate in ServiceWorkerFetchDispatcher. ServiceWorkerProviderHost::PreCreateNavigationHost() creates a ServiceWorkerProviderHost without ServiceWorkerDispatcherHost. So ServiceWorkerFetchDispatcher::MaybeStartNavigationPreload() is called with null MojoURLLoaderFactoryGetter. This will cause crashes. I want to disable NavigationPreload for PlzNavigate in this CL. BUG=649558 ========== to ========== Add check for PlzNavigate in ServiceWorkerFetchDispatcher. ServiceWorkerProviderHost::PreCreateNavigationHost() creates a ServiceWorkerProviderHost without ServiceWorkerDispatcherHost. So MaybeStartNavigationPreload() is called with null MojoURLLoaderFactoryGetter. This will cause crashes. I want to disable NavigationPreload for PlzNavigate in this CL. BUG=649558 ==========
Description was changed from ========== Add check for PlzNavigate in ServiceWorkerFetchDispatcher. ServiceWorkerProviderHost::PreCreateNavigationHost() creates a ServiceWorkerProviderHost without ServiceWorkerDispatcherHost. So MaybeStartNavigationPreload() is called with null MojoURLLoaderFactoryGetter. This will cause crashes. I want to disable NavigationPreload for PlzNavigate in this CL. BUG=649558 ========== to ========== Add check for PlzNavigate in ServiceWorkerFetchDispatcher. ServiceWorkerProviderHost::PreCreateNavigationHost() creates a ServiceWorkerProviderHost without ServiceWorkerDispatcherHost. So MaybeStartNavigationPreload() is called with null MojoURLLoaderFactoryGetter. This will cause crashes when PlzNavigate is enabled. I want to disable NavigationPreload for PlzNavigate in this CL. BUG=649558 ==========
horo@chromium.org changed reviewers: + falken@chromium.org
falken@ Could you please review?
lgtm https://codereview.chromium.org/2451663002/diff/1/content/browser/service_wor... File content/browser/service_worker/service_worker_fetch_dispatcher.cc (right): https://codereview.chromium.org/2451663002/diff/1/content/browser/service_wor... content/browser/service_worker/service_worker_fetch_dispatcher.cc:300: // TODO(horo): Support NavigationPreload with PlzNavigate. how about calling NOTIMPLEMENTED()
https://codereview.chromium.org/2451663002/diff/1/content/browser/service_wor... File content/browser/service_worker/service_worker_fetch_dispatcher.cc (right): https://codereview.chromium.org/2451663002/diff/1/content/browser/service_wor... content/browser/service_worker/service_worker_fetch_dispatcher.cc:300: // TODO(horo): Support NavigationPreload with PlzNavigate. On 2016/10/25 07:32:38, falken wrote: > how about calling NOTIMPLEMENTED() Done.
The CQ bit was checked by horo@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from falken@chromium.org Link to the patchset: https://codereview.chromium.org/2451663002/#ps20001 (title: "add NOTIMPLEMENTED()")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
The CQ bit was checked by horo@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Add check for PlzNavigate in ServiceWorkerFetchDispatcher. ServiceWorkerProviderHost::PreCreateNavigationHost() creates a ServiceWorkerProviderHost without ServiceWorkerDispatcherHost. So MaybeStartNavigationPreload() is called with null MojoURLLoaderFactoryGetter. This will cause crashes when PlzNavigate is enabled. I want to disable NavigationPreload for PlzNavigate in this CL. BUG=649558 ========== to ========== Add check for PlzNavigate in ServiceWorkerFetchDispatcher. ServiceWorkerProviderHost::PreCreateNavigationHost() creates a ServiceWorkerProviderHost without ServiceWorkerDispatcherHost. So MaybeStartNavigationPreload() is called with null MojoURLLoaderFactoryGetter. This will cause crashes when PlzNavigate is enabled. I want to disable NavigationPreload for PlzNavigate in this CL. BUG=649558 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Add check for PlzNavigate in ServiceWorkerFetchDispatcher. ServiceWorkerProviderHost::PreCreateNavigationHost() creates a ServiceWorkerProviderHost without ServiceWorkerDispatcherHost. So MaybeStartNavigationPreload() is called with null MojoURLLoaderFactoryGetter. This will cause crashes when PlzNavigate is enabled. I want to disable NavigationPreload for PlzNavigate in this CL. BUG=649558 ========== to ========== Add check for PlzNavigate in ServiceWorkerFetchDispatcher. ServiceWorkerProviderHost::PreCreateNavigationHost() creates a ServiceWorkerProviderHost without ServiceWorkerDispatcherHost. So MaybeStartNavigationPreload() is called with null MojoURLLoaderFactoryGetter. This will cause crashes when PlzNavigate is enabled. I want to disable NavigationPreload for PlzNavigate in this CL. BUG=649558 Committed: https://crrev.com/193ab9f14c15355404ec9bbfe002dd48980a465a Cr-Commit-Position: refs/heads/master@{#427310} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/193ab9f14c15355404ec9bbfe002dd48980a465a Cr-Commit-Position: refs/heads/master@{#427310} |
