Support DataSaver for SharedWorker

chrome/browser/chrome_content_browser_client.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chrome_content_browser_client.h"
 
 #include <map>
 #include <set>
 #include <utility>
 #include <vector>
@@ skipping 190 matching lines @@
 #include "ppapi/features/features.h"
 #include "ppapi/host/ppapi_host.h"
 #include "printing/features/features.h"
 #include "services/preferences/public/interfaces/preferences.mojom.h"
 #include "services/service_manager/public/cpp/binder_registry.h"
 #include "services/service_manager/public/cpp/interface_provider.h"
 #include "services/service_manager/public/cpp/service.h"
 #include "storage/browser/fileapi/external_mount_points.h"
 #include "third_party/WebKit/public/platform/modules/installedapp/installed_app_provider.mojom.h"
 #include "third_party/WebKit/public/platform/modules/webshare/webshare.mojom.h"
+#include "third_party/WebKit/public/web/WebDataSaverFlag.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/resources/grit/ui_resources.h"
 #include "url/gurl.h"
 #include "url/origin.h"
 
 #if defined(OS_WIN)
 #include "base/strings/string_tokenizer.h"
 #include "chrome/browser/chrome_browser_main_win.h"
 #include "chrome/browser/conflicts/module_database_win.h"
@@ skipping 2479 matching lines @@
   }
   DCHECK(!web_prefs->default_encoding.empty());
 
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnablePotentiallyAnnoyingSecurityFeatures)) {
     web_prefs->disable_reading_from_canvas = true;
     web_prefs->strict_mixed_content_checking = true;
     web_prefs->strict_powerful_feature_restrictions = true;
   }
 
-  web_prefs->data_saver_enabled = GetDataSaverEnabledPref(prefs);
+  web_prefs->data_saver_flag = GetDataSaverEnabledPref(prefs)
+                                   ? blink::WebDataSaverFlag::kEnabled
+                                   : blink::WebDataSaverFlag::kDisabled;

[kinuko, 2017/05/18 03:48:21]

Ah sorry- I hadn't realized this change would propagate further.

We also have IsDataSaverEnabled code on this class which returns boolean, which
feels a little consistent now?

And totally orthogonal-- looking into further it looks what
GetDataSaverEnabledPref returns and IsDataSaverEnabled returns could be
different, is that ok?

...and then I found/recalled that we have EmbeddedWorkerSetting for service
worker case.

Ok, might it be cleaner if we just turn WebEmbeddedWorkerSettings in
WorkerSettings, put it under content/common and use it for both workers (with a
comment to note that v8 cache option is applicable only for SW)?

Either works, but what do you think? (Please comment back first if you're unsure
/ disagree)

Sorry for the back and forth!

[horo, 2017/05/18 05:08:27]

Are you saying "inconsistent"?
I can rename ChromeContentBrowserClient::IsDataSaverEnabled() to
GetDataSaverFlag() and make it to return the flag.
Shall I do so?
I think it is an existing bug that
ChromeContentBrowserClient::IsDataSaverEnabled() doesn't check FieldTrialList.
https://codereview.chromium.org/1698973002 introduced GetDataSaverEnabledPref()
which checks FieldTrialList.
https://codereview.chromium.org/1708583003 introduced IsDataSaverEnabled() which
doesn't check FieldTrialList.

+CC:rajendrant@, bengr@
I have already updated EmbeddedWorkerSetting.
https://codereview.chromium.org/2881073003/diff/180001/content/common/service...
Currently we don't support V8CacheOptions for SharedWorker.
Yes, it is desirable to support it. But it is not in the scope of this CL.
And I think it is confusing to introduce a new structure with unused flag
(without tests).

If we need to check the DataSaverFlag out of FetchContext, we should have
WorkerSettings::GetDataSaverFlag(). But currently we don't need it.

 
 #if defined(OS_ANDROID)
   content::WebContents* contents =
       content::WebContents::FromRenderViewHost(rvh);
   if (contents) {
     TabAndroid* tab_android = TabAndroid::FromWebContents(contents);
     if (tab_android) {
       web_prefs->embedded_media_experience_enabled =
           tab_android->ShouldEnableEmbeddedMediaExperience();
 
@@ skipping 888 matching lines @@
     RedirectNonUINonIOBrowserThreadsToTaskScheduler() {
   return variations::GetVariationParamValue(
              "BrowserScheduler", "RedirectNonUINonIOBrowserThreads") == "true";
 }
 
 // static
 void ChromeContentBrowserClient::SetDefaultQuotaSettingsForTesting(
     const storage::QuotaSettings* settings) {
   g_default_quota_settings = settings;
 }