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

Issue 633791 link

Starred by 2 users
Status: Fixed
Owner:
dominickn@chromium.org
Closed: Oct 2016
Cc:
hanxi@chromium.org
pkotw...@chromium.org
msramek@chromium.org
█ bauerb@chromium.org
█ wnwen@chromium.org
█ dfalcant...@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocked on:
issue 653627


Sign in to add a comment

WebappDataStorage and WebappRegistry should write to SharedPreferences from the UI thread

Project Member Reported by dominickn@chromium.org, Aug 3 2016 
Currently, WebappDataStorage and WebappRegistry use SharedPreferences#apply() in an AsyncTask to write to SharedPreferences. However, that method is async, so it can be called from the UI thread. Writing to SharedPreferences from a background thread introduces a race condition where it may also be modified from the UI thread; SharedPreferences are written all at once for all prefs across the whole app.

Both classes need to be refactored to write from the UI thread only.

Comment 1 by wnwen@chromium.org, Sep 21 2016

Cc: wnwen@chromium.org

Comment 2 by dominickn@chromium.org, Sep 22 2016

Labels: M-55
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 28 2016 

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

commit 54870dc572cb26a1c92b4ac5484d64d15f51be86
Author: dominickn <dominickn@chromium.org>
Date: Wed Sep 28 02:22:08 2016

Remove the context argument from all WebappRegistry/WebappDataStorage methods.

This CL replaces the context argument with calls to
ContextUtils.getApplicationContext() within WebappRegistry and
WebappDataStorage methods. There is only one application context within
a process, so this should not change any behaviour.

This simplifies the interfaces, and will prepare for:
 * pre-warming the SharedPreferences on startup
 * calling SharedPreferences.editor.apply() exclusively on the main
   thread in all webapp code.
 * moving most WebappDataStorage and WebappRegistry methods to
   being synchronous and directly callable on the main thread instead
   of needing a callback

BUG= 633791 

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

[modify] https://crrev.com/54870dc572cb26a1c92b4ac5484d64d15f51be86/chrome/android/java/src/org/chromium/chrome/browser/ServiceTabLauncher.java
[modify] https://crrev.com/54870dc572cb26a1c92b4ac5484d64d15f51be86/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java
[modify] https://crrev.com/54870dc572cb26a1c92b4ac5484d64d15f51be86/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java
[modify] https://crrev.com/54870dc572cb26a1c92b4ac5484d64d15f51be86/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkUpdateManager.java
[modify] https://crrev.com/54870dc572cb26a1c92b4ac5484d64d15f51be86/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
[modify] https://crrev.com/54870dc572cb26a1c92b4ac5484d64d15f51be86/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
[modify] https://crrev.com/54870dc572cb26a1c92b4ac5484d64d15f51be86/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java
[modify] https://crrev.com/54870dc572cb26a1c92b4ac5484d64d15f51be86/chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java
[modify] https://crrev.com/54870dc572cb26a1c92b4ac5484d64d15f51be86/chrome/android/javatests/src/org/chromium/chrome/browser/browsing_data/BrowsingDataRemoverIntegrationTest.java
[modify] https://crrev.com/54870dc572cb26a1c92b4ac5484d64d15f51be86/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesTest.java
[modify] https://crrev.com/54870dc572cb26a1c92b4ac5484d64d15f51be86/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/AddToHomescreenManagerTest.java
[modify] https://crrev.com/54870dc572cb26a1c92b4ac5484d64d15f51be86/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappActivityTestBase.java
[modify] https://crrev.com/54870dc572cb26a1c92b4ac5484d64d15f51be86/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappModeTest.java
[modify] https://crrev.com/54870dc572cb26a1c92b4ac5484d64d15f51be86/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappSplashScreenIconTest.java
[modify] https://crrev.com/54870dc572cb26a1c92b4ac5484d64d15f51be86/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappSplashScreenTest.java
[modify] https://crrev.com/54870dc572cb26a1c92b4ac5484d64d15f51be86/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappDataStorageTest.java
[modify] https://crrev.com/54870dc572cb26a1c92b4ac5484d64d15f51be86/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappRegistryTest.java
[modify] https://crrev.com/54870dc572cb26a1c92b4ac5484d64d15f51be86/chrome/browser/android/webapps/webapp_registry.cc
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 4 2016 

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

commit 78cf6596040662c0a73c0e4038bc84fb2b3c470a
Author: dominickn <dominickn@chromium.org>
Date: Tue Oct 04 04:19:54 2016

Refactor WebappRegistry into a singleton instance.

This CL refactors WebappRegistry and WebappDataStorage to make most of
the methods synchronous. WebappRegistry is now a singleton instance that
is instantiated at browser startup. This allows all SharedPreferences files to
be pre-warmed before the class is used; new web apps open new
SharedPreferences on a background thread when registered, after which the
preferences are cached automatically.

Most static methods on WebappRegistry and WebappDataStorage have been
converted to instance methods or removed. This makes the code much
cleaner and more efficient; each static method had to independently open
their SharedPreferences, which minimally performs a stat() on the
underlying XML file to see if it has changed. Now the singleton
WebappRegistry caches all WebappDataStorage objects on startup and
whenever new ones are added. This reduces disk IO overhead.

This CL allows all calls to SharedPreferences.Editor.apply() in
WebappRegistry and WebappDataStorage to occur on the main thread,
mostly removing the need for unwieldy callback interfaces and bare
pointer passing across the JNI.

BUG= 633791 

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

[modify] https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a/chrome/android/java/src/org/chromium/chrome/browser/DeferredStartupHandler.java
[modify] https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a/chrome/android/java/src/org/chromium/chrome/browser/ServiceTabLauncher.java
[modify] https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java
[modify] https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationService.java
[modify] https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java
[modify] https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkUpdateManager.java
[modify] https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
[modify] https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
[modify] https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java
[modify] https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a/chrome/android/java_sources.gni
[modify] https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a/chrome/android/javatests/src/org/chromium/chrome/browser/browsing_data/BrowsingDataRemoverIntegrationTest.java
[modify] https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesTest.java
[add] https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TestFetchStorageCallback.java
[modify] https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappActivityTestBase.java
[modify] https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappModeTest.java
[modify] https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappSplashScreenTest.java
[modify] https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappDataStorageTest.java
[modify] https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappRegistryTest.java
[modify] https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a/chrome/browser/android/chrome_jni_registrar.cc
[modify] https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a/chrome/browser/android/webapps/webapp_registry.cc
[modify] https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a/chrome/browser/android/webapps/webapp_registry.h
[modify] https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a/chrome/browser/browsing_data/browsing_data_remover.cc
[modify] https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a/chrome/browser/browsing_data/browsing_data_remover.h
[modify] https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a/chrome/browser/browsing_data/browsing_data_remover_unittest.cc
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 4 2016 

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

commit 83ff6feda1e62ebbdaac19a8ebdd473d9dfd398a
Author: penghuang <penghuang@chromium.org>
Date: Tue Oct 04 12:17:10 2016

Revert of Refactor WebappRegistry into a singleton instance. (patchset #11 id:200001 of https://codereview.chromium.org/2351113005/ )

Reason for revert:
chrome_public_test_apk failed.

https://uberchromegw.corp.google.com/i/chromium.linux/builders/Android%20Tests%20%28dbg%29/builds/36476

Original issue's description:
> Refactor WebappRegistry into a singleton instance.
>
> This CL refactors WebappRegistry and WebappDataStorage to make most of
> the methods synchronous. WebappRegistry is now a singleton instance that
> is instantiated at browser startup. This allows all SharedPreferences files to
> be pre-warmed before the class is used; new web apps open new
> SharedPreferences on a background thread when registered, after which the
> preferences are cached automatically.
>
> Most static methods on WebappRegistry and WebappDataStorage have been
> converted to instance methods or removed. This makes the code much
> cleaner and more efficient; each static method had to independently open
> their SharedPreferences, which minimally performs a stat() on the
> underlying XML file to see if it has changed. Now the singleton
> WebappRegistry caches all WebappDataStorage objects on startup and
> whenever new ones are added. This reduces disk IO overhead.
>
> This CL allows all calls to SharedPreferences.Editor.apply() in
> WebappRegistry and WebappDataStorage to occur on the main thread,
> mostly removing the need for unwieldy callback interfaces and bare
> pointer passing across the JNI.
>
> BUG= 633791 
>
> Committed: https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a
> Cr-Commit-Position: refs/heads/master@{#422703}

TBR=dfalcantara@chromium.org,wnwen@chromium.org,msramek@chromium.org,dominickn@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 633791 

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

[modify] https://crrev.com/83ff6feda1e62ebbdaac19a8ebdd473d9dfd398a/chrome/android/java/src/org/chromium/chrome/browser/DeferredStartupHandler.java
[modify] https://crrev.com/83ff6feda1e62ebbdaac19a8ebdd473d9dfd398a/chrome/android/java/src/org/chromium/chrome/browser/ServiceTabLauncher.java
[modify] https://crrev.com/83ff6feda1e62ebbdaac19a8ebdd473d9dfd398a/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java
[modify] https://crrev.com/83ff6feda1e62ebbdaac19a8ebdd473d9dfd398a/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationService.java
[modify] https://crrev.com/83ff6feda1e62ebbdaac19a8ebdd473d9dfd398a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java
[modify] https://crrev.com/83ff6feda1e62ebbdaac19a8ebdd473d9dfd398a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkUpdateManager.java
[modify] https://crrev.com/83ff6feda1e62ebbdaac19a8ebdd473d9dfd398a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
[modify] https://crrev.com/83ff6feda1e62ebbdaac19a8ebdd473d9dfd398a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
[modify] https://crrev.com/83ff6feda1e62ebbdaac19a8ebdd473d9dfd398a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java
[modify] https://crrev.com/83ff6feda1e62ebbdaac19a8ebdd473d9dfd398a/chrome/android/java_sources.gni
[modify] https://crrev.com/83ff6feda1e62ebbdaac19a8ebdd473d9dfd398a/chrome/android/javatests/src/org/chromium/chrome/browser/browsing_data/BrowsingDataRemoverIntegrationTest.java
[modify] https://crrev.com/83ff6feda1e62ebbdaac19a8ebdd473d9dfd398a/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesTest.java
[delete] https://crrev.com/2a97db8de8551ccd894bf9e96a3af5481dc44e24/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TestFetchStorageCallback.java
[modify] https://crrev.com/83ff6feda1e62ebbdaac19a8ebdd473d9dfd398a/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappActivityTestBase.java
[modify] https://crrev.com/83ff6feda1e62ebbdaac19a8ebdd473d9dfd398a/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappModeTest.java
[modify] https://crrev.com/83ff6feda1e62ebbdaac19a8ebdd473d9dfd398a/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappSplashScreenTest.java
[modify] https://crrev.com/83ff6feda1e62ebbdaac19a8ebdd473d9dfd398a/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappDataStorageTest.java
[modify] https://crrev.com/83ff6feda1e62ebbdaac19a8ebdd473d9dfd398a/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappRegistryTest.java
[modify] https://crrev.com/83ff6feda1e62ebbdaac19a8ebdd473d9dfd398a/chrome/browser/android/chrome_jni_registrar.cc
[modify] https://crrev.com/83ff6feda1e62ebbdaac19a8ebdd473d9dfd398a/chrome/browser/android/webapps/webapp_registry.cc
[modify] https://crrev.com/83ff6feda1e62ebbdaac19a8ebdd473d9dfd398a/chrome/browser/android/webapps/webapp_registry.h
[modify] https://crrev.com/83ff6feda1e62ebbdaac19a8ebdd473d9dfd398a/chrome/browser/browsing_data/browsing_data_remover.cc
[modify] https://crrev.com/83ff6feda1e62ebbdaac19a8ebdd473d9dfd398a/chrome/browser/browsing_data/browsing_data_remover.h
[modify] https://crrev.com/83ff6feda1e62ebbdaac19a8ebdd473d9dfd398a/chrome/browser/browsing_data/browsing_data_remover_unittest.cc
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 5 2016 

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

commit 6e08a0b7993fb106e3dea796950521cfd04a47cf
Author: dominickn <dominickn@chromium.org>
Date: Wed Oct 05 04:52:31 2016

[Reland] Refactor WebappRegistry into a singleton instance.

This CL refactors WebappRegistry and WebappDataStorage to make most of
the methods synchronous. WebappRegistry is now a singleton instance that
is instantiated at browser startup. This allows all SharedPreferences files to
be pre-warmed before the class is used; new web apps open new
SharedPreferences on a background thread when registered, after which the
preferences are cached automatically.

Most static methods on WebappRegistry and WebappDataStorage have been
converted to instance methods or removed. This makes the code much
cleaner and more efficient; each static method had to independently open
their SharedPreferences, which minimally performs a stat() on the
underlying XML file to see if it has changed. Now the singleton
WebappRegistry caches all WebappDataStorage objects on startup and
whenever new ones are added. This reduces disk IO overhead.

This CL allows all calls to SharedPreferences.Editor.apply() in
WebappRegistry and WebappDataStorage to occur on the main thread,
mostly removing the need for unwieldy callback interfaces and bare
pointer passing across the JNI.

BUG= 633791 

Committed: https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a
Review-Url: https://codereview.chromium.org/2351113005
Cr-Original-Commit-Position: refs/heads/master@{#422703}
Cr-Commit-Position: refs/heads/master@{#423077}

[modify] https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf/chrome/android/java/src/org/chromium/chrome/browser/DeferredStartupHandler.java
[modify] https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf/chrome/android/java/src/org/chromium/chrome/browser/ServiceTabLauncher.java
[modify] https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java
[modify] https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationService.java
[modify] https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java
[modify] https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkUpdateManager.java
[modify] https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
[modify] https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
[modify] https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java
[modify] https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf/chrome/android/java_sources.gni
[modify] https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf/chrome/android/javatests/src/org/chromium/chrome/browser/browsing_data/BrowsingDataRemoverIntegrationTest.java
[modify] https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesTest.java
[add] https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TestFetchStorageCallback.java
[modify] https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappActivityTestBase.java
[modify] https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappModeTest.java
[modify] https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappSplashScreenTest.java
[modify] https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappDataStorageTest.java
[modify] https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappRegistryTest.java
[modify] https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf/chrome/browser/android/chrome_jni_registrar.cc
[modify] https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf/chrome/browser/android/webapps/webapp_registry.cc
[modify] https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf/chrome/browser/android/webapps/webapp_registry.h
[modify] https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf/chrome/browser/browsing_data/browsing_data_remover.cc
[modify] https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf/chrome/browser/browsing_data/browsing_data_remover.h
[modify] https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf/chrome/browser/browsing_data/browsing_data_remover_unittest.cc
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 5 2016 

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

commit 5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2
Author: penghuang <penghuang@chromium.org>
Date: Wed Oct 05 13:20:36 2016

Revert of [Reland] Refactor WebappRegistry into a singleton instance. (patchset #13 id:240001 of https://codereview.chromium.org/2351113005/ )

Reason for revert:
chrome_public_test_apk failure
https://uberchromegw.corp.google.com/i/chromium.linux/builders/Android%20Tests%20%28dbg%29/builds/36501

Original issue's description:
> [Reland] Refactor WebappRegistry into a singleton instance.
>
> This CL refactors WebappRegistry and WebappDataStorage to make most of
> the methods synchronous. WebappRegistry is now a singleton instance that
> is instantiated at browser startup. This allows all SharedPreferences files to
> be pre-warmed before the class is used; new web apps open new
> SharedPreferences on a background thread when registered, after which the
> preferences are cached automatically.
>
> Most static methods on WebappRegistry and WebappDataStorage have been
> converted to instance methods or removed. This makes the code much
> cleaner and more efficient; each static method had to independently open
> their SharedPreferences, which minimally performs a stat() on the
> underlying XML file to see if it has changed. Now the singleton
> WebappRegistry caches all WebappDataStorage objects on startup and
> whenever new ones are added. This reduces disk IO overhead.
>
> This CL allows all calls to SharedPreferences.Editor.apply() in
> WebappRegistry and WebappDataStorage to occur on the main thread,
> mostly removing the need for unwieldy callback interfaces and bare
> pointer passing across the JNI.
>
> BUG= 633791 
>
> Committed: https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a
> Committed: https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf
> Cr-Original-Commit-Position: refs/heads/master@{#422703}
> Cr-Commit-Position: refs/heads/master@{#423077}

TBR=dfalcantara@chromium.org,wnwen@chromium.org,msramek@chromium.org,dominickn@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 633791 

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

[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/java/src/org/chromium/chrome/browser/DeferredStartupHandler.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/java/src/org/chromium/chrome/browser/ServiceTabLauncher.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationService.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkUpdateManager.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/java_sources.gni
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/javatests/src/org/chromium/chrome/browser/browsing_data/BrowsingDataRemoverIntegrationTest.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesTest.java
[delete] https://crrev.com/ffb1ebb3a7a1486984f2b2a0133c0ac20d0dadb1/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TestFetchStorageCallback.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappActivityTestBase.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappModeTest.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappSplashScreenTest.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappDataStorageTest.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappRegistryTest.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/browser/android/chrome_jni_registrar.cc
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/browser/android/webapps/webapp_registry.cc
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/browser/android/webapps/webapp_registry.h
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/browser/browsing_data/browsing_data_remover.cc
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/browser/browsing_data/browsing_data_remover.h
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/browser/browsing_data/browsing_data_remover_unittest.cc
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 6 2016 

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

commit 8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537
Author: dominickn <dominickn@chromium.org>
Date: Thu Oct 06 01:52:26 2016

[Reland] Refactor WebappRegistry into a singleton instance.

This CL refactors WebappRegistry and WebappDataStorage to make most of
the methods synchronous. WebappRegistry is now a singleton instance that
is instantiated at browser startup. This allows all SharedPreferences files to
be pre-warmed before the class is used; new web apps open new
SharedPreferences on a background thread when registered, after which the
preferences are cached automatically.

Most static methods on WebappRegistry and WebappDataStorage have been
converted to instance methods or removed. This makes the code much
cleaner and more efficient; each static method had to independently open
their SharedPreferences, which minimally performs a stat() on the
underlying XML file to see if it has changed. Now the singleton
WebappRegistry caches all WebappDataStorage objects on startup and
whenever new ones are added. This reduces disk IO overhead.

This CL allows all calls to SharedPreferences.Editor.apply() in
WebappRegistry and WebappDataStorage to occur on the main thread,
mostly removing the need for unwieldy callback interfaces and bare
pointer passing across the JNI.

BUG= 633791 

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

[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/java/src/org/chromium/chrome/browser/DeferredStartupHandler.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/java/src/org/chromium/chrome/browser/ServiceTabLauncher.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationService.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkUpdateManager.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/java_sources.gni
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/javatests/src/org/chromium/chrome/browser/browsing_data/BrowsingDataRemoverIntegrationTest.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesTest.java
[add] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TestFetchStorageCallback.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappActivityTestBase.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappModeTest.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappSplashScreenTest.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappDataStorageTest.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappRegistryTest.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/browser/android/chrome_jni_registrar.cc
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/browser/android/webapps/webapp_registry.cc
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/browser/android/webapps/webapp_registry.h
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/browser/browsing_data/browsing_data_remover.cc
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/browser/browsing_data/browsing_data_remover.h
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/browser/browsing_data/browsing_data_remover_unittest.cc
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 6 2016 

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

commit 0d4dfd29bee0715867058fa36823f6161a883f85
Author: nasko <nasko@chromium.org>
Date: Thu Oct 06 18:33:27 2016

Revert of [Reland] Refactor WebappRegistry into a singleton instance. (patchset #14 id:260001 of https://codereview.chromium.org/2351113005/ )

Reason for revert:
chrome_public_test_apk test failures in WebappSplashScreenTest.

https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/155149

failures:
org.chromium.chrome.browser.webapps.WebappSplashScreenTest#testRegularSplashScreenAppears
org.chromium.chrome.browser.webapps.WebappSplashScreenTest#testSmallSplashScreenAppears

Original issue's description:
> [Reland] Refactor WebappRegistry into a singleton instance.
>
> This CL refactors WebappRegistry and WebappDataStorage to make most of
> the methods synchronous. WebappRegistry is now a singleton instance that
> is instantiated at browser startup. This allows all SharedPreferences files to
> be pre-warmed before the class is used; new web apps open new
> SharedPreferences on a background thread when registered, after which the
> preferences are cached automatically.
>
> Most static methods on WebappRegistry and WebappDataStorage have been
> converted to instance methods or removed. This makes the code much
> cleaner and more efficient; each static method had to independently open
> their SharedPreferences, which minimally performs a stat() on the
> underlying XML file to see if it has changed. Now the singleton
> WebappRegistry caches all WebappDataStorage objects on startup and
> whenever new ones are added. This reduces disk IO overhead.
>
> This CL allows all calls to SharedPreferences.Editor.apply() in
> WebappRegistry and WebappDataStorage to occur on the main thread,
> mostly removing the need for unwieldy callback interfaces and bare
> pointer passing across the JNI.
>
> BUG= 633791 
>
> Committed: https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537
> Cr-Commit-Position: refs/heads/master@{#423389}

TBR=dfalcantara@chromium.org,wnwen@chromium.org,msramek@chromium.org,dominickn@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 633791 

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

[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/java/src/org/chromium/chrome/browser/DeferredStartupHandler.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/java/src/org/chromium/chrome/browser/ServiceTabLauncher.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationService.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkUpdateManager.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/java_sources.gni
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/javatests/src/org/chromium/chrome/browser/browsing_data/BrowsingDataRemoverIntegrationTest.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesTest.java
[delete] https://crrev.com/80979723f0248e791feae4829095b05aac1eb3a1/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TestFetchStorageCallback.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappActivityTestBase.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappModeTest.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappSplashScreenTest.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappDataStorageTest.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappRegistryTest.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/browser/android/chrome_jni_registrar.cc
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/browser/android/webapps/webapp_registry.cc
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/browser/android/webapps/webapp_registry.h
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/browser/browsing_data/browsing_data_remover.cc
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/browser/browsing_data/browsing_data_remover.h
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/browser/browsing_data/browsing_data_remover_unittest.cc

Comment 10 by dominickn@chromium.org, Oct 12 2016

Blockedon: 653627
My CL keeps making the WebappSplashScreenTests flaky. But those tests are pretty flaky anyways - the Low-End Lollipop bot shows that they misbehave quite a bit.

I'm going to try and deflake those tests first so I can (finally) land this refactor....
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 14 2016 

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

commit beceaa5ebceead7104900401ab96fe2facc26da1
Author: dominickn <dominickn@chromium.org>
Date: Fri Oct 14 00:28:31 2016

[Reland] Refactor WebappRegistry into a singleton instance.

This CL refactors WebappRegistry and WebappDataStorage to make most of
the methods synchronous. WebappRegistry is now a singleton instance that
is instantiated at browser startup. This allows all SharedPreferences files to
be pre-warmed before the class is used; new web apps open new
SharedPreferences on a background thread when registered, after which the
preferences are cached automatically.

Most static methods on WebappRegistry and WebappDataStorage have been
converted to instance methods or removed. This makes the code much
cleaner and more efficient; each static method had to independently open
their SharedPreferences, which minimally performs a stat() on the
underlying XML file to see if it has changed. Now the singleton
WebappRegistry caches all WebappDataStorage objects on startup and
whenever new ones are added. This reduces disk IO overhead.

This CL allows all calls to SharedPreferences.Editor.apply() in
WebappRegistry and WebappDataStorage to occur on the main thread,
mostly removing the need for unwieldy callback interfaces and bare
pointer passing across the JNI.

BUG= 633791 , 653627 

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

[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/android/java/src/org/chromium/chrome/browser/DeferredStartupHandler.java
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/android/java/src/org/chromium/chrome/browser/ServiceTabLauncher.java
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationService.java
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkUpdateManager.java
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/android/java_sources.gni
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/android/javatests/src/org/chromium/chrome/browser/browsing_data/BrowsingDataRemoverIntegrationTest.java
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesTest.java
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/AddToHomescreenManagerTest.java
[add] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TestFetchStorageCallback.java
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappActivityTestBase.java
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappModeTest.java
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappSplashScreenIconTest.java
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappSplashScreenTest.java
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappDataStorageTest.java
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappRegistryTest.java
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/browser/android/chrome_jni_registrar.cc
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/browser/android/webapps/webapp_registry.cc
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/browser/android/webapps/webapp_registry.h
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/browser/browsing_data/browsing_data_remover.cc
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/browser/browsing_data/browsing_data_remover.h
[modify] https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1/chrome/browser/browsing_data/browsing_data_remover_unittest.cc
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 27 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2

commit 5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2
Author: penghuang <penghuang@chromium.org>
Date: Wed Oct 05 13:20:36 2016

Revert of [Reland] Refactor WebappRegistry into a singleton instance. (patchset #13 id:240001 of https://codereview.chromium.org/2351113005/ )

Reason for revert:
chrome_public_test_apk failure
https://uberchromegw.corp.google.com/i/chromium.linux/builders/Android%20Tests%20%28dbg%29/builds/36501

Original issue's description:
> [Reland] Refactor WebappRegistry into a singleton instance.
>
> This CL refactors WebappRegistry and WebappDataStorage to make most of
> the methods synchronous. WebappRegistry is now a singleton instance that
> is instantiated at browser startup. This allows all SharedPreferences files to
> be pre-warmed before the class is used; new web apps open new
> SharedPreferences on a background thread when registered, after which the
> preferences are cached automatically.
>
> Most static methods on WebappRegistry and WebappDataStorage have been
> converted to instance methods or removed. This makes the code much
> cleaner and more efficient; each static method had to independently open
> their SharedPreferences, which minimally performs a stat() on the
> underlying XML file to see if it has changed. Now the singleton
> WebappRegistry caches all WebappDataStorage objects on startup and
> whenever new ones are added. This reduces disk IO overhead.
>
> This CL allows all calls to SharedPreferences.Editor.apply() in
> WebappRegistry and WebappDataStorage to occur on the main thread,
> mostly removing the need for unwieldy callback interfaces and bare
> pointer passing across the JNI.
>
> BUG= 633791 
>
> Committed: https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a
> Committed: https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf
> Cr-Original-Commit-Position: refs/heads/master@{#422703}
> Cr-Commit-Position: refs/heads/master@{#423077}

TBR=dfalcantara@chromium.org,wnwen@chromium.org,msramek@chromium.org,dominickn@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 633791 

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

[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/java/src/org/chromium/chrome/browser/DeferredStartupHandler.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/java/src/org/chromium/chrome/browser/ServiceTabLauncher.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationService.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkUpdateManager.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/java_sources.gni
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/javatests/src/org/chromium/chrome/browser/browsing_data/BrowsingDataRemoverIntegrationTest.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesTest.java
[delete] https://crrev.com/ffb1ebb3a7a1486984f2b2a0133c0ac20d0dadb1/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TestFetchStorageCallback.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappActivityTestBase.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappModeTest.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappSplashScreenTest.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappDataStorageTest.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappRegistryTest.java
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/browser/android/chrome_jni_registrar.cc
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/browser/android/webapps/webapp_registry.cc
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/browser/android/webapps/webapp_registry.h
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/browser/browsing_data/browsing_data_remover.cc
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/browser/browsing_data/browsing_data_remover.h
[modify] https://crrev.com/5a8dd1f93ea9f1257cc72ae8d0765dcf916a24e2/chrome/browser/browsing_data/browsing_data_remover_unittest.cc
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 27 2016 

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

commit 8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537
Author: dominickn <dominickn@chromium.org>
Date: Thu Oct 06 01:52:26 2016

[Reland] Refactor WebappRegistry into a singleton instance.

This CL refactors WebappRegistry and WebappDataStorage to make most of
the methods synchronous. WebappRegistry is now a singleton instance that
is instantiated at browser startup. This allows all SharedPreferences files to
be pre-warmed before the class is used; new web apps open new
SharedPreferences on a background thread when registered, after which the
preferences are cached automatically.

Most static methods on WebappRegistry and WebappDataStorage have been
converted to instance methods or removed. This makes the code much
cleaner and more efficient; each static method had to independently open
their SharedPreferences, which minimally performs a stat() on the
underlying XML file to see if it has changed. Now the singleton
WebappRegistry caches all WebappDataStorage objects on startup and
whenever new ones are added. This reduces disk IO overhead.

This CL allows all calls to SharedPreferences.Editor.apply() in
WebappRegistry and WebappDataStorage to occur on the main thread,
mostly removing the need for unwieldy callback interfaces and bare
pointer passing across the JNI.

BUG= 633791 

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

[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/java/src/org/chromium/chrome/browser/DeferredStartupHandler.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/java/src/org/chromium/chrome/browser/ServiceTabLauncher.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationService.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkUpdateManager.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/java_sources.gni
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/javatests/src/org/chromium/chrome/browser/browsing_data/BrowsingDataRemoverIntegrationTest.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesTest.java
[add] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TestFetchStorageCallback.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappActivityTestBase.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappModeTest.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappSplashScreenTest.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappDataStorageTest.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappRegistryTest.java
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/browser/android/chrome_jni_registrar.cc
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/browser/android/webapps/webapp_registry.cc
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/browser/android/webapps/webapp_registry.h
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/browser/browsing_data/browsing_data_remover.cc
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/browser/browsing_data/browsing_data_remover.h
[modify] https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537/chrome/browser/browsing_data/browsing_data_remover_unittest.cc
Project Member

Comment 14 by bugdroid1@chromium.org, Oct 27 2016 

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

commit 0d4dfd29bee0715867058fa36823f6161a883f85
Author: nasko <nasko@chromium.org>
Date: Thu Oct 06 18:33:27 2016

Revert of [Reland] Refactor WebappRegistry into a singleton instance. (patchset #14 id:260001 of https://codereview.chromium.org/2351113005/ )

Reason for revert:
chrome_public_test_apk test failures in WebappSplashScreenTest.

https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/155149

failures:
org.chromium.chrome.browser.webapps.WebappSplashScreenTest#testRegularSplashScreenAppears
org.chromium.chrome.browser.webapps.WebappSplashScreenTest#testSmallSplashScreenAppears

Original issue's description:
> [Reland] Refactor WebappRegistry into a singleton instance.
>
> This CL refactors WebappRegistry and WebappDataStorage to make most of
> the methods synchronous. WebappRegistry is now a singleton instance that
> is instantiated at browser startup. This allows all SharedPreferences files to
> be pre-warmed before the class is used; new web apps open new
> SharedPreferences on a background thread when registered, after which the
> preferences are cached automatically.
>
> Most static methods on WebappRegistry and WebappDataStorage have been
> converted to instance methods or removed. This makes the code much
> cleaner and more efficient; each static method had to independently open
> their SharedPreferences, which minimally performs a stat() on the
> underlying XML file to see if it has changed. Now the singleton
> WebappRegistry caches all WebappDataStorage objects on startup and
> whenever new ones are added. This reduces disk IO overhead.
>
> This CL allows all calls to SharedPreferences.Editor.apply() in
> WebappRegistry and WebappDataStorage to occur on the main thread,
> mostly removing the need for unwieldy callback interfaces and bare
> pointer passing across the JNI.
>
> BUG= 633791 
>
> Committed: https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537
> Cr-Commit-Position: refs/heads/master@{#423389}

TBR=dfalcantara@chromium.org,wnwen@chromium.org,msramek@chromium.org,dominickn@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 633791 

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

[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/java/src/org/chromium/chrome/browser/DeferredStartupHandler.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/java/src/org/chromium/chrome/browser/ServiceTabLauncher.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationService.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkUpdateManager.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/java_sources.gni
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/javatests/src/org/chromium/chrome/browser/browsing_data/BrowsingDataRemoverIntegrationTest.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesTest.java
[delete] https://crrev.com/80979723f0248e791feae4829095b05aac1eb3a1/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TestFetchStorageCallback.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappActivityTestBase.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappModeTest.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappSplashScreenTest.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappDataStorageTest.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappRegistryTest.java
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/browser/android/chrome_jni_registrar.cc
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/browser/android/webapps/webapp_registry.cc
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/browser/android/webapps/webapp_registry.h
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/browser/browsing_data/browsing_data_remover.cc
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/browser/browsing_data/browsing_data_remover.h
[modify] https://crrev.com/0d4dfd29bee0715867058fa36823f6161a883f85/chrome/browser/browsing_data/browsing_data_remover_unittest.cc

Comment 15 by dominickn@chromium.org, Oct 31 2016

Status: Fixed (was: Assigned)

Comment 16 by dimu@google.com, Nov 4 2016

Labels: -merge-merged-2840
[Automated comment] removing mislabelled merge-merged-2840
►

Sign in to add a comment