|
|
Chromium Code Reviews|
Created:
3 years, 11 months ago by horo Modified:
3 years, 11 months ago CC:
chromium-reviews, michaeln, jsbell+serviceworker_chromium.org, kenjibaheux+watch_chromium.org, shimazu+serviceworker_chromium.org, serviceworker-reviews, jam, nhiroki, kinuko+serviceworker, horo+watch_chromium.org, blink-reviews, darin-cc_chromium.org, kinuko+watch, tzik, falken+watch_chromium.org, blink-worker-reviews_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionEnable ServiceWorkerNavigationPreload by default for Origin-Trial.
The CL https://codereview.chromium.org/2627023002/ introduced the mechanism for
Origin-Trial experiment of Service Worker Navigation Preload. But base::Feature
of the feature was disabled by default. So when we start the Origin-Trial
experiment, we had to enable the feature by the field trial mechanism. But it
is possible that the feature is not correctly enabled by the field trial
mechanism (Ex: in the first boot of Chrome). In order to avoid the confusion,
this CL enables the base::Feature of the feature by default.
If something goes wrong and we will want to stop the origin-trial, we can stop
the experiment by disabling this feature using the field trial mechanism.
* Origin Trial: Have an effective token.
Command line
Default Enable Disabled
Default *A* A B
Field trial Enabled A A B
Disabled B A B
* Origin Trial: No token.
Command line
Default Enable Disabled
Default C A C
Field trial Enabled C A C
Disabled C A C
- A: Navigation Preload related methods and attributes are available in JS
and work correctly.
- B: Navigation Preload related methods and attributes are available in
JS. But NavigationPreloadManager's enable, disable and setHeaderValue
methods always return a rejected promise. And FetchEvent's
preloadResponse attribute returns a promise which always resolve with
undefined.
- C: Navigation Preload related methods and attributes are not available
in JS.
BUG=649558
Review-Url: https://codereview.chromium.org/2637763002
Cr-Commit-Position: refs/heads/master@{#443851}
Committed: https://chromium.googlesource.com/chromium/src/+/9ad964459156c4bd27fa68a0bec4155409ec492d
Patch Set 1 #
Total comments: 2
Patch Set 2 : incorporated kinuko's comment #
Messages
Total messages: 20 (13 generated)
The CQ bit was checked by horo@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Enable ServiceWorkerNavigationPreload by default for Origin-Trial. BUG=649558 ========== to ========== Enable ServiceWorkerNavigationPreload by default for Origin-Trial. The CL https://codereview.chromium.org/2627023002/ introduced the mechanism for Origin-Trial experiment of Service Worker Navigation Preload. But base::Feature of the feature was disabled by default. So when we start the Origin-Trial experiment, we have to enable the feature by the field trial mechanism. But it is possible that the feature is not correctly enabled by the field trial mechanism (Ex: in the first boot of Chrome). In order to avoid the confusion, this CL enables the base::Feature of the feature by default. BUG=649558 ==========
Description was changed from ========== Enable ServiceWorkerNavigationPreload by default for Origin-Trial. The CL https://codereview.chromium.org/2627023002/ introduced the mechanism for Origin-Trial experiment of Service Worker Navigation Preload. But base::Feature of the feature was disabled by default. So when we start the Origin-Trial experiment, we have to enable the feature by the field trial mechanism. But it is possible that the feature is not correctly enabled by the field trial mechanism (Ex: in the first boot of Chrome). In order to avoid the confusion, this CL enables the base::Feature of the feature by default. BUG=649558 ========== to ========== The CL https://codereview.chromium.org/2627023002/ introduced the mechanism for Origin-Trial experiment of Service Worker Navigation Preload. But base::Feature of the feature was disabled by default. So when we start the Origin-Trial experiment, we have to enable the feature by the field trial mechanism. But it is possible that the feature is not correctly enabled by the field trial mechanism (Ex: in the first boot of Chrome). In order to avoid the confusion, this CL enables the base::Feature of the feature by default. If something goes wrong and we will want to stop the origin-trial, we can stop the experiment by disabling this feature using the field trial mechanism. * Origin Trial: Have an effective header. Command line Default Enable Disabled Default *A* A B Field trial Enabled A A B Disabled B A B * Origin Trial: No header. Command line Default Enable Disabled Default C A C Field trial Enabled C A C Disabled C A C - A: Navigation Preload related methods and attributes are available in JS and work correctly. - B: Navigation Preload related methods and attributes are available in JS. But NavigationPreloadManager's enable, disable and setHeaderValue methods always return a rejected promise. And FetchEvent's preloadResponse attribute returns a promise which always resolve with undefined. - C: Navigation Preload related methods and attributes are not available in JS. BUG=649558 ==========
Description was changed from ========== The CL https://codereview.chromium.org/2627023002/ introduced the mechanism for Origin-Trial experiment of Service Worker Navigation Preload. But base::Feature of the feature was disabled by default. So when we start the Origin-Trial experiment, we have to enable the feature by the field trial mechanism. But it is possible that the feature is not correctly enabled by the field trial mechanism (Ex: in the first boot of Chrome). In order to avoid the confusion, this CL enables the base::Feature of the feature by default. If something goes wrong and we will want to stop the origin-trial, we can stop the experiment by disabling this feature using the field trial mechanism. * Origin Trial: Have an effective header. Command line Default Enable Disabled Default *A* A B Field trial Enabled A A B Disabled B A B * Origin Trial: No header. Command line Default Enable Disabled Default C A C Field trial Enabled C A C Disabled C A C - A: Navigation Preload related methods and attributes are available in JS and work correctly. - B: Navigation Preload related methods and attributes are available in JS. But NavigationPreloadManager's enable, disable and setHeaderValue methods always return a rejected promise. And FetchEvent's preloadResponse attribute returns a promise which always resolve with undefined. - C: Navigation Preload related methods and attributes are not available in JS. BUG=649558 ========== to ========== The CL https://codereview.chromium.org/2627023002/ introduced the mechanism for Origin-Trial experiment of Service Worker Navigation Preload. But base::Feature of the feature was disabled by default. So when we start the Origin-Trial experiment, we had to enable the feature by the field trial mechanism. But it is possible that the feature is not correctly enabled by the field trial mechanism (Ex: in the first boot of Chrome). In order to avoid the confusion, this CL enables the base::Feature of the feature by default. If something goes wrong and we will want to stop the origin-trial, we can stop the experiment by disabling this feature using the field trial mechanism. * Origin Trial: Have an effective header. Command line Default Enable Disabled Default *A* A B Field trial Enabled A A B Disabled B A B * Origin Trial: No header. Command line Default Enable Disabled Default C A C Field trial Enabled C A C Disabled C A C - A: Navigation Preload related methods and attributes are available in JS and work correctly. - B: Navigation Preload related methods and attributes are available in JS. But NavigationPreloadManager's enable, disable and setHeaderValue methods always return a rejected promise. And FetchEvent's preloadResponse attribute returns a promise which always resolve with undefined. - C: Navigation Preload related methods and attributes are not available in JS. BUG=649558 ==========
Description was changed from ========== The CL https://codereview.chromium.org/2627023002/ introduced the mechanism for Origin-Trial experiment of Service Worker Navigation Preload. But base::Feature of the feature was disabled by default. So when we start the Origin-Trial experiment, we had to enable the feature by the field trial mechanism. But it is possible that the feature is not correctly enabled by the field trial mechanism (Ex: in the first boot of Chrome). In order to avoid the confusion, this CL enables the base::Feature of the feature by default. If something goes wrong and we will want to stop the origin-trial, we can stop the experiment by disabling this feature using the field trial mechanism. * Origin Trial: Have an effective header. Command line Default Enable Disabled Default *A* A B Field trial Enabled A A B Disabled B A B * Origin Trial: No header. Command line Default Enable Disabled Default C A C Field trial Enabled C A C Disabled C A C - A: Navigation Preload related methods and attributes are available in JS and work correctly. - B: Navigation Preload related methods and attributes are available in JS. But NavigationPreloadManager's enable, disable and setHeaderValue methods always return a rejected promise. And FetchEvent's preloadResponse attribute returns a promise which always resolve with undefined. - C: Navigation Preload related methods and attributes are not available in JS. BUG=649558 ========== to ========== Enable ServiceWorkerNavigationPreload by default for Origin-Trial. The CL https://codereview.chromium.org/2627023002/ introduced the mechanism for Origin-Trial experiment of Service Worker Navigation Preload. But base::Feature of the feature was disabled by default. So when we start the Origin-Trial experiment, we had to enable the feature by the field trial mechanism. But it is possible that the feature is not correctly enabled by the field trial mechanism (Ex: in the first boot of Chrome). In order to avoid the confusion, this CL enables the base::Feature of the feature by default. If something goes wrong and we will want to stop the origin-trial, we can stop the experiment by disabling this feature using the field trial mechanism. * Origin Trial: Have an effective header. Command line Default Enable Disabled Default *A* A B Field trial Enabled A A B Disabled B A B * Origin Trial: No header. Command line Default Enable Disabled Default C A C Field trial Enabled C A C Disabled C A C - A: Navigation Preload related methods and attributes are available in JS and work correctly. - B: Navigation Preload related methods and attributes are available in JS. But NavigationPreloadManager's enable, disable and setHeaderValue methods always return a rejected promise. And FetchEvent's preloadResponse attribute returns a promise which always resolve with undefined. - C: Navigation Preload related methods and attributes are not available in JS. BUG=649558 ==========
Description was changed from ========== Enable ServiceWorkerNavigationPreload by default for Origin-Trial. The CL https://codereview.chromium.org/2627023002/ introduced the mechanism for Origin-Trial experiment of Service Worker Navigation Preload. But base::Feature of the feature was disabled by default. So when we start the Origin-Trial experiment, we had to enable the feature by the field trial mechanism. But it is possible that the feature is not correctly enabled by the field trial mechanism (Ex: in the first boot of Chrome). In order to avoid the confusion, this CL enables the base::Feature of the feature by default. If something goes wrong and we will want to stop the origin-trial, we can stop the experiment by disabling this feature using the field trial mechanism. * Origin Trial: Have an effective header. Command line Default Enable Disabled Default *A* A B Field trial Enabled A A B Disabled B A B * Origin Trial: No header. Command line Default Enable Disabled Default C A C Field trial Enabled C A C Disabled C A C - A: Navigation Preload related methods and attributes are available in JS and work correctly. - B: Navigation Preload related methods and attributes are available in JS. But NavigationPreloadManager's enable, disable and setHeaderValue methods always return a rejected promise. And FetchEvent's preloadResponse attribute returns a promise which always resolve with undefined. - C: Navigation Preload related methods and attributes are not available in JS. BUG=649558 ========== to ========== Enable ServiceWorkerNavigationPreload by default for Origin-Trial. The CL https://codereview.chromium.org/2627023002/ introduced the mechanism for Origin-Trial experiment of Service Worker Navigation Preload. But base::Feature of the feature was disabled by default. So when we start the Origin-Trial experiment, we had to enable the feature by the field trial mechanism. But it is possible that the feature is not correctly enabled by the field trial mechanism (Ex: in the first boot of Chrome). In order to avoid the confusion, this CL enables the base::Feature of the feature by default. If something goes wrong and we will want to stop the origin-trial, we can stop the experiment by disabling this feature using the field trial mechanism. * Origin Trial: Have an effective token. Command line Default Enable Disabled Default *A* A B Field trial Enabled A A B Disabled B A B * Origin Trial: No token. Command line Default Enable Disabled Default C A C Field trial Enabled C A C Disabled C A C - A: Navigation Preload related methods and attributes are available in JS and work correctly. - B: Navigation Preload related methods and attributes are available in JS. But NavigationPreloadManager's enable, disable and setHeaderValue methods always return a rejected promise. And FetchEvent's preloadResponse attribute returns a promise which always resolve with undefined. - C: Navigation Preload related methods and attributes are not available in JS. BUG=649558 ==========
horo@chromium.org changed reviewers: + falken@chromium.org
falken@ Please review this.
lgtm
horo@chromium.org changed reviewers: + kinuko@chromium.org
kinuko@ Could you please review content/child/runtime_features.cc content/public/common/content_features.cc?
lgtm https://codereview.chromium.org/2637763002/diff/1/content/public/common/conte... File content/public/common/content_features.cc (right): https://codereview.chromium.org/2637763002/diff/1/content/public/common/conte... content/public/common/content_features.cc:195: // runtime_features.cc and service_worker_version.h for the details. nit: avoid using 'we' in general, e.g. 'Enables this base::Feature by default for Origin-Trial, but enables the corresponding blink::WebRuntimeFeatures only when '--enable-features' command line flag is given. See the...'
Thank you. https://codereview.chromium.org/2637763002/diff/1/content/public/common/conte... File content/public/common/content_features.cc (right): https://codereview.chromium.org/2637763002/diff/1/content/public/common/conte... content/public/common/content_features.cc:195: // runtime_features.cc and service_worker_version.h for the details. On 2017/01/16 05:07:10, kinuko wrote: > nit: avoid using 'we' in general, e.g. 'Enables this base::Feature by default > for Origin-Trial, but enables the corresponding blink::WebRuntimeFeatures only > when '--enable-features' command line flag is given. See the...' 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, kinuko@chromium.org Link to the patchset: https://codereview.chromium.org/2637763002/#ps20001 (title: "incorporated kinuko's comment")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484544698370990,
"parent_rev": "4bb402f2c33c749362a6b6d3baf6220d9ec6a759", "commit_rev":
"9ad964459156c4bd27fa68a0bec4155409ec492d"}
Message was sent while issue was closed.
Description was changed from ========== Enable ServiceWorkerNavigationPreload by default for Origin-Trial. The CL https://codereview.chromium.org/2627023002/ introduced the mechanism for Origin-Trial experiment of Service Worker Navigation Preload. But base::Feature of the feature was disabled by default. So when we start the Origin-Trial experiment, we had to enable the feature by the field trial mechanism. But it is possible that the feature is not correctly enabled by the field trial mechanism (Ex: in the first boot of Chrome). In order to avoid the confusion, this CL enables the base::Feature of the feature by default. If something goes wrong and we will want to stop the origin-trial, we can stop the experiment by disabling this feature using the field trial mechanism. * Origin Trial: Have an effective token. Command line Default Enable Disabled Default *A* A B Field trial Enabled A A B Disabled B A B * Origin Trial: No token. Command line Default Enable Disabled Default C A C Field trial Enabled C A C Disabled C A C - A: Navigation Preload related methods and attributes are available in JS and work correctly. - B: Navigation Preload related methods and attributes are available in JS. But NavigationPreloadManager's enable, disable and setHeaderValue methods always return a rejected promise. And FetchEvent's preloadResponse attribute returns a promise which always resolve with undefined. - C: Navigation Preload related methods and attributes are not available in JS. BUG=649558 ========== to ========== Enable ServiceWorkerNavigationPreload by default for Origin-Trial. The CL https://codereview.chromium.org/2627023002/ introduced the mechanism for Origin-Trial experiment of Service Worker Navigation Preload. But base::Feature of the feature was disabled by default. So when we start the Origin-Trial experiment, we had to enable the feature by the field trial mechanism. But it is possible that the feature is not correctly enabled by the field trial mechanism (Ex: in the first boot of Chrome). In order to avoid the confusion, this CL enables the base::Feature of the feature by default. If something goes wrong and we will want to stop the origin-trial, we can stop the experiment by disabling this feature using the field trial mechanism. * Origin Trial: Have an effective token. Command line Default Enable Disabled Default *A* A B Field trial Enabled A A B Disabled B A B * Origin Trial: No token. Command line Default Enable Disabled Default C A C Field trial Enabled C A C Disabled C A C - A: Navigation Preload related methods and attributes are available in JS and work correctly. - B: Navigation Preload related methods and attributes are available in JS. But NavigationPreloadManager's enable, disable and setHeaderValue methods always return a rejected promise. And FetchEvent's preloadResponse attribute returns a promise which always resolve with undefined. - C: Navigation Preload related methods and attributes are not available in JS. BUG=649558 Review-Url: https://codereview.chromium.org/2637763002 Cr-Commit-Position: refs/heads/master@{#443851} Committed: https://chromium.googlesource.com/chromium/src/+/9ad964459156c4bd27fa68a0bec4... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/9ad964459156c4bd27fa68a0bec4... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
