New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 604632 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Don't sync content settings to mobile which aren't used there

Project Member Reported by raymes@chromium.org, Apr 19 2016

Issue description

We shouldn't sync content settings to mobile if they aren't used there. This will cause privacy sensitive data to be stored on the device which the user will have no way of viewing/erasing/etc.
 

Comment 1 by engedy@chromium.org, Apr 22 2016

Components: Privacy

Comment 2 by engedy@chromium.org, Apr 22 2016

Cc: engedy@chromium.org
Cc: bauerb@chromium.org
I'm tracing Android crashes due to un-registering images and plugins on Android.
One place that gets access to image settings is in https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/android/preferences/pref_service_bridge.cc&l=164, which will set images to be ALLOW on all sites because of bug( https://crbug.com/505844 ).

So how should we deal with this case?

+bauerb since he is the reviewer of that bug fix and might have some background of it.:-)
Well, if we're confident that a sufficient number of users has been migrated, we could remove the migration code from https://codereview.chromium.org/1240153002, and remove CONTENT_SETTINGS_TYPE_IMAGES from that DCHECK.
My guess is, the number of users who need to be migrated (i.e. who changed the default setting for images to be BLOCK in the short period of time) is not so big, and the migration code has been there for a very long time. So I tend to remove the migration code so that we can unregister CONTENT_SETTINGS_TYPE_IMAGES on Android completely.
Cc: raymes@chromium.org
Owner: lshang@chromium.org
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/afd97ff8f20acb90a6b495776e385411371651f0

commit afd97ff8f20acb90a6b495776e385411371651f0
Author: lshang <lshang@chromium.org>
Date: Tue Jun 07 00:28:41 2016

Only Register() platform specific content settings types on different platforms

Content settings are not used on all platforms. Some content settings are only
used on desktop platforms, or desktop and android. We shouldn't register these
types on other platforms that aren't using them, so that we don't create unused
prefs for them and more importantly so that we don't sync settings to these
platforms which would go unused but be a potential privacy concern.

In this CL, an enum WebsiteSettingsInfo::Platform is added and used when
registering content settings to indicate which platform set it is applied to.

BUG= 604632 

Review-Url: https://codereview.chromium.org/1991623005
Cr-Commit-Position: refs/heads/master@{#398174}

[modify] https://crrev.com/afd97ff8f20acb90a6b495776e385411371651f0/components/content_settings/core/browser/content_settings_default_provider.cc
[modify] https://crrev.com/afd97ff8f20acb90a6b495776e385411371651f0/components/content_settings/core/browser/content_settings_registry.cc
[modify] https://crrev.com/afd97ff8f20acb90a6b495776e385411371651f0/components/content_settings/core/browser/content_settings_registry.h
[modify] https://crrev.com/afd97ff8f20acb90a6b495776e385411371651f0/components/content_settings/core/browser/content_settings_registry_unittest.cc
[modify] https://crrev.com/afd97ff8f20acb90a6b495776e385411371651f0/components/content_settings/core/browser/host_content_settings_map.cc
[modify] https://crrev.com/afd97ff8f20acb90a6b495776e385411371651f0/components/content_settings/core/browser/website_settings_registry.cc
[modify] https://crrev.com/afd97ff8f20acb90a6b495776e385411371651f0/components/content_settings/core/browser/website_settings_registry.h
[modify] https://crrev.com/afd97ff8f20acb90a6b495776e385411371651f0/components/content_settings/core/browser/website_settings_registry_unittest.cc
[modify] https://crrev.com/afd97ff8f20acb90a6b495776e385411371651f0/components/content_settings/core/common/content_settings_types.h

Project Member

Comment 8 by bugdroid1@chromium.org, Jun 14 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e085f20530d930427a3efff3a2fd9aa2f94d61a9

commit e085f20530d930427a3efff3a2fd9aa2f94d61a9
Author: lshang <lshang@chromium.org>
Date: Tue Jun 14 01:25:08 2016

Unregister Images, Plugins and Mouselock content settings on android

Images, plugins and mouselock content settings aren't used on Android, so
unregister them in ContentSettingsRegistry and handle places in shared code
where get access to these types.

Several places in shared code are if-defed out when they try to get access to
unregistered types. Some tests are also changed to use COOKIES instead of IMAGES
to make the tests more platform generic, if they are not testing images
specifically.

This is a follow up CL of https://codereview.chromium.org/1991623005/.

BUG= 604632 

Review-Url: https://codereview.chromium.org/2039803002
Cr-Commit-Position: refs/heads/master@{#399634}

[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/chrome/android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/chrome/browser/android/preferences/pref_service_bridge.cc
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/chrome/browser/browsing_data/browsing_data_remover_unittest.cc
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/chrome/browser/content_settings/content_settings_default_provider_unittest.cc
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/chrome/browser/content_settings/content_settings_pref_provider_unittest.cc
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/chrome/browser/content_settings/host_content_settings_map_unittest.cc
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/chrome/browser/content_settings/mock_settings_observer.cc
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/chrome/browser/content_settings/tab_specific_content_settings_unittest.cc
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/chrome/browser/ui/website_settings/website_settings.cc
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/chrome/browser/ui/website_settings/website_settings_unittest.cc
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/components/content_settings/core/browser/content_settings_default_provider.cc
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/components/content_settings/core/browser/content_settings_registry.cc
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/components/content_settings/core/browser/content_settings_registry_unittest.cc
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/components/content_settings/core/browser/content_settings_utils.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e085f20530d930427a3efff3a2fd9aa2f94d61a9

commit e085f20530d930427a3efff3a2fd9aa2f94d61a9
Author: lshang <lshang@chromium.org>
Date: Tue Jun 14 01:25:08 2016

Unregister Images, Plugins and Mouselock content settings on android

Images, plugins and mouselock content settings aren't used on Android, so
unregister them in ContentSettingsRegistry and handle places in shared code
where get access to these types.

Several places in shared code are if-defed out when they try to get access to
unregistered types. Some tests are also changed to use COOKIES instead of IMAGES
to make the tests more platform generic, if they are not testing images
specifically.

This is a follow up CL of https://codereview.chromium.org/1991623005/.

BUG= 604632 

Review-Url: https://codereview.chromium.org/2039803002
Cr-Commit-Position: refs/heads/master@{#399634}

[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/chrome/android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/chrome/browser/android/preferences/pref_service_bridge.cc
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/chrome/browser/browsing_data/browsing_data_remover_unittest.cc
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/chrome/browser/content_settings/content_settings_default_provider_unittest.cc
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/chrome/browser/content_settings/content_settings_pref_provider_unittest.cc
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/chrome/browser/content_settings/host_content_settings_map_unittest.cc
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/chrome/browser/content_settings/mock_settings_observer.cc
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/chrome/browser/content_settings/tab_specific_content_settings_unittest.cc
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/chrome/browser/ui/website_settings/website_settings.cc
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/chrome/browser/ui/website_settings/website_settings_unittest.cc
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/components/content_settings/core/browser/content_settings_default_provider.cc
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/components/content_settings/core/browser/content_settings_registry.cc
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/components/content_settings/core/browser/content_settings_registry_unittest.cc
[modify] https://crrev.com/e085f20530d930427a3efff3a2fd9aa2f94d61a9/components/content_settings/core/browser/content_settings_utils.cc

After the changes, now each content setting type's applied platforms are:

cookies--------------desktop*, android, ios
popups---------------desktop, android, ios
javascript-----------desktop, android
geolocation----------desktop, android
notifications--------desktop, android
fullscreen-----------desktop, android
mediastream-mic------desktop, android
mediastream-camera---desktop, android
automatic-downloads--desktop, android
midi-sysex-----------desktop, android
push-messaging-------desktop, android
durable-storage------desktop, android
keygen---------------desktop, android
background-sync------desktop, android
autoplay-------------desktop, android
bluetooth-guard------desktop, android
images------------desktop
plugins-----------desktop
mouselock---------desktop
ppapi-broker------desktop
protocol-handler--desktop
mixed-script------desktop
protected-media-identifier---android, chromeos

*desktop: win, linux, chromeos, mac
Status: Fixed (was: Assigned)
For clarity, does this also change whether the PreferenceSpecifics corresponding to desktop-only content setting types get synced down to the local Sync Database on Android; or just what gets stored in JSON format in Preferences?  

(Sorry for reviving this.)
Unfortunately, as it was clarified with the Sync team after we made this change, it only affects whether the data are written to the preferences. The data are still physically sent to the device.
Thanks for the clarification.

Sign in to add a comment