[Reland] Refactor WebappRegistry into a singleton instance.

[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
Committed: https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1
Cr-Commit-Position: refs/heads/master@{#425214}

[dominickn, 2016-09-21 05:58:40]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-21 05:58:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dominickn, 2016-09-21 05:59:23]

Description was changed from

==========
Convert WebappRegistry to 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 by ChromeBrowserInitializer. This allows all
SharedPreferences files to be warmed at startup; new web apps which are
added will be 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
the 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 CL allows all calls to SharedPreferences.editor.apply() to occur on
the main thread, removing the need for unwieldy callback interfaces and
pointer passing across the JNI.

BUG=633791
==========

to

==========
Convert WebappRegistry to 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 by ChromeBrowserInitializer. This allows all
SharedPreferences files to be warmed at startup; new web apps which are
added will be 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
the 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 CL allows all calls to SharedPreferences.editor.apply() to occur on
the main thread, removing the need for unwieldy callback interfaces and
pointer passing across the JNI.

BUG=633791
==========

[dominickn, 2016-09-21 05:59:23]

dominickn@chromium.org changed reviewers:
+ dfalcantara@chromium.org, wnwen@chromium.org

[commit-bot: I haz the power, 2016-09-21 06:47:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-21 06:47:24]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[dominickn, 2016-09-21 06:58:42]

This is a follow-up from crrev.com/2352943003.

+wnwen: PTAL at WebappRegistry#warmUpSharedPrefs, particularly the StrictMode
policy. Is it okay to to warm these prefs?

+dfalcantara: Overall thoughts? I think this dramatically improves the
WebappRegistry/WebappDataStorage API, deletes a bunch of annoying boilerplate,
and generally reduces callback hell.

[dominickn, 2016-09-21 07:18:36]

Description was changed from

==========
Convert WebappRegistry to 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 by ChromeBrowserInitializer. This allows all
SharedPreferences files to be warmed at startup; new web apps which are
added will be 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
the 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 CL allows all calls to SharedPreferences.editor.apply() to occur on
the main thread, removing the need for unwieldy callback interfaces and
pointer passing across the JNI.

BUG=633791
==========

to

==========
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 by ChromeBrowserInitializer. This allows all
SharedPreferences files to be warmed at startup; new web apps which are
added will be 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
the 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 CL allows all calls to SharedPreferences.editor.apply() to occur on
the main thread, removing the need for unwieldy callback interfaces and
pointer passing across the JNI.

BUG=633791
==========

[dominickn, 2016-09-21 09:38:05]

Description was changed from

==========
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 by ChromeBrowserInitializer. This allows all
SharedPreferences files to be warmed at startup; new web apps which are
added will be 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
the 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 CL allows all calls to SharedPreferences.editor.apply() to occur on
the main thread, removing the need for unwieldy callback interfaces and
pointer passing across the JNI.

BUG=633791
==========

to

==========
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 by ChromeBrowserInitializer. This allows all
SharedPreferences files to be warmed at startup; new web apps which are
added will be 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
the 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 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 pointer passing across the JNI.

BUG=633791
==========

[dominickn, 2016-09-21 09:38:12]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-21 09:38:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dominickn, 2016-09-21 09:38:31]

Description was changed from

==========
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 by ChromeBrowserInitializer. This allows all
SharedPreferences files to be warmed at startup; new web apps which are
added will be 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
the 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 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 pointer passing across the JNI.

BUG=633791
==========

to

==========
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 by ChromeBrowserInitializer. This allows all
SharedPreferences files to be warmed at startup; new web apps which are
added will be 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
the 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 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
==========

[dominickn, 2016-09-21 09:40:14]

Description was changed from

==========
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 by ChromeBrowserInitializer. This allows all
SharedPreferences files to be warmed at startup; new web apps which are
added will be 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
the 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 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
==========

to

==========
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 by ChromeBrowserInitializer. This allows all
SharedPreferences files to be warmed at startup; 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
==========

[commit-bot: I haz the power, 2016-09-21 10:25:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-21 10:25:39]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[dominickn, 2016-09-22 03:55:07]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-22 03:55:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-22 04:43:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-22 04:43:11]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[dominickn, 2016-09-26 23:53:08]

On 2016/09/21 06:58:42, dominickn wrote:
> This is a follow-up from crrev.com/2352943003.
> 
> +wnwen: PTAL at WebappRegistry#warmUpSharedPrefs, particularly the StrictMode
> policy. Is it okay to to warm these prefs?
> 
> +dfalcantara: Overall thoughts? I think this dramatically improves the
> WebappRegistry/WebappDataStorage API, deletes a bunch of annoying boilerplate,
> and generally reduces callback hell.

wnwen: Ping. :)

[Peter Wen, 2016-09-27 20:57:25]

https://codereview.chromium.org/2351113005/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/init/ChromeBrowserInitializer.java
(right):

https://codereview.chromium.org/2351113005/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/init/ChromeBrowserInitializer.java:181:
WebappRegistry.warmUpSharedPrefs();
Do you know what the access pattern for WebappRegistry is? Can this entire call
be moved to DeferredStartupHandler?

https://codereview.chromium.org/2351113005/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java
(right):

https://codereview.chromium.org/2351113005/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java:81:
public static void warmUpSharedPrefs() {
Is this guaranteed to be called after getInstance()? Otherwise null check is
needed before sInstance is re-warmed up.

https://codereview.chromium.org/2351113005/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java:279:
mPreferences.getStringSet(KEY_WEBAPP_SET, Collections.<String>emptySet())) {
Since not every webapp will be needed on startup, can this be warmed up in
DeferredStartupHandler?

[dominickn, 2016-09-28 02:06:50]

Thanks!

https://codereview.chromium.org/2351113005/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/init/ChromeBrowserInitializer.java
(right):

https://codereview.chromium.org/2351113005/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/init/ChromeBrowserInitializer.java:181:
WebappRegistry.warmUpSharedPrefs();
On 2016/09/27 20:57:25, Peter Wen wrote:
> Do you know what the access pattern for WebappRegistry is? Can this entire
call
> be moved to DeferredStartupHandler?

When a PWA is launched, we'll need WebappRegistry and the WebappDataStorage
corresponding to that PWA in WebappActivity#postInflationStartup. We need
everything in WebappActivity#onResume, which cleans up any old data (though this
cleanup could potentially be moved into DeferredStartupHandler).

I'm not that familiar with the startup flow: is there a place where I can warm
up WebappRegistry and a WebappDataStorage before
WebappActivity#postInflationStartup that's not ChromeBrowserInitializer?

https://codereview.chromium.org/2351113005/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java
(right):

https://codereview.chromium.org/2351113005/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java:81:
public static void warmUpSharedPrefs() {
On 2016/09/27 20:57:25, Peter Wen wrote:
> Is this guaranteed to be called after getInstance()? Otherwise null check is
> needed before sInstance is re-warmed up.

It is intended that this would be called once at startup to warm everything
(i.e. before getInstance()).

https://codereview.chromium.org/2351113005/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java:279:
mPreferences.getStringSet(KEY_WEBAPP_SET, Collections.<String>emptySet())) {
On 2016/09/27 20:57:25, Peter Wen wrote:
> Since not every webapp will be needed on startup, can this be warmed up in
> DeferredStartupHandler?

See my response to your other comment. :)

[gone, 2016-09-28 18:39:17]

Sweet jeebus just avoiding callback hell is worth the relatively minor
(potential) StrictMode violation.

https://chromiumcodereview.appspot.com/2351113005/diff/40001/chrome/android/j...
File
chrome/android/java/src/org/chromium/chrome/browser/init/ChromeBrowserInitializer.java
(right):

https://chromiumcodereview.appspot.com/2351113005/diff/40001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/init/ChromeBrowserInitializer.java:181:
WebappRegistry.warmUpSharedPrefs();
On 2016/09/28 02:06:50, dominickn wrote:
> On 2016/09/27 20:57:25, Peter Wen wrote:
> > Do you know what the access pattern for WebappRegistry is? Can this entire
> call
> > be moved to DeferredStartupHandler?
> 
> When a PWA is launched, we'll need WebappRegistry and the WebappDataStorage
> corresponding to that PWA in WebappActivity#postInflationStartup. We need
> everything in WebappActivity#onResume, which cleans up any old data (though
this
> cleanup could potentially be moved into DeferredStartupHandler).
> 
> I'm not that familiar with the startup flow: is there a place where I can warm
> up WebappRegistry and a WebappDataStorage before
> WebappActivity#postInflationStartup that's not ChromeBrowserInitializer?

There's also WebappActivity#preInflationStartup.  You'd be able to avoid warming
up prefs on all types of Chrome startup and just focus on webapps, in that case.

https://chromiumcodereview.appspot.com/2351113005/diff/40001/chrome/android/j...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
(right):

https://chromiumcodereview.appspot.com/2351113005/diff/40001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java:92:
* Generates a WebappDataStorage class for a specified web app.
/** Generates a WebappDataStorage instance for a specific web app. */

https://chromiumcodereview.appspot.com/2351113005/diff/40001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java:104:
* Returns the current time in milliseconds.
Maybe /** @return Current time in milliseconds. */

https://chromiumcodereview.appspot.com/2351113005/diff/40001/chrome/android/j...
File
chrome/android/javatests/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesTest.java
(right):

https://chromiumcodereview.appspot.com/2351113005/diff/40001/chrome/android/j...
chrome/android/javatests/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesTest.java:110:
mCallbackCalled = true;
See comment elsewhere about CriteriaHelper -> CallbackHelper.  If it's too
painful, could be skipped for now.

https://chromiumcodereview.appspot.com/2351113005/diff/40001/chrome/android/j...
File
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappSplashScreenTest.java
(right):

https://chromiumcodereview.appspot.com/2351113005/diff/40001/chrome/android/j...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappSplashScreenTest.java:265:
mCallbackCalled = true;
Can you use a real CallbackHelper here instead of using a CriteriaHelper?  It'd
require these calls in relevant places instead:

CallbackHelper mStorageRetrievedCallback = new CallbackHelper();

mStorageRetrievedCallback.notifyCalled();

mStorageRetrievedCallback.waitForCallback(0);

You probably also want to do that after the updateSplashScreenImage call is
done.

https://chromiumcodereview.appspot.com/2351113005/diff/40001/chrome/android/j...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappSplashScreenTest.java:306:
CriteriaHelper.pollUiThread(new CallbackCriteria());
Same as above.

https://chromiumcodereview.appspot.com/2351113005/diff/40001/chrome/android/j...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappSplashScreenTest.java:342:
CriteriaHelper.pollUiThread(new CallbackCriteria());
Ditto.

[dominickn, 2016-09-29 06:51:22]

Description was changed from

==========
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 by ChromeBrowserInitializer. This allows all
SharedPreferences files to be warmed at startup; 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
==========

to

==========
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
==========

[dominickn, 2016-09-29 06:55:22]

Thanks! Prefs warming is now split up as follows:

1. Opening a PWA. WebappRegistry + the specific WebappDataStorage for the
opening web app are warmed in WebappActivity#preInflationStartup.

2. Opening Chrome (and PWAs). WebappRegistry + all webapps are warmed on a
background thread in DeferredStartupHandler, with the old webapp unregistration
taking place on the UI thread once that is finished. We need these to be warmed
in ordinary Chrome because Clearing Browsing Data will query WebappRegistry and
potentially delete some WebappStorages. When this happens after PWA launch, the
WebappRegistry instance is already warmed and all that's needed to do is warm
the other web app shared prefs.

3. Notification deep-linking. WebappRegistry + all webapps are warmed in
NotificationService, with a disabled strict mode warning. This is needed because
if Chrome isn't already running, the registry must be warmed to allow it to be
queried in ServiceTabLauncher.

https://codereview.chromium.org/2351113005/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
(right):

https://codereview.chromium.org/2351113005/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java:92:
* Generates a WebappDataStorage class for a specified web app.
On 2016/09/28 18:39:16, dfalcantara (slow) wrote:
> /** Generates a WebappDataStorage instance for a specific web app. */

Done.

https://codereview.chromium.org/2351113005/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java:104:
* Returns the current time in milliseconds.
On 2016/09/28 18:39:16, dfalcantara (slow) wrote:
> Maybe /** @return Current time in milliseconds. */

Done.

https://codereview.chromium.org/2351113005/diff/40001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesTest.java
(right):

https://codereview.chromium.org/2351113005/diff/40001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesTest.java:110:
mCallbackCalled = true;
On 2016/09/28 18:39:16, dfalcantara (slow) wrote:
> See comment elsewhere about CriteriaHelper -> CallbackHelper.  If it's too
> painful, could be skipped for now.

Done.

https://codereview.chromium.org/2351113005/diff/40001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappSplashScreenTest.java
(right):

https://codereview.chromium.org/2351113005/diff/40001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappSplashScreenTest.java:265:
mCallbackCalled = true;
On 2016/09/28 18:39:17, dfalcantara (slow) wrote:
> Can you use a real CallbackHelper here instead of using a CriteriaHelper? 
It'd
> require these calls in relevant places instead:
> 
> CallbackHelper mStorageRetrievedCallback = new CallbackHelper();
> 
> mStorageRetrievedCallback.notifyCalled();
> 
> mStorageRetrievedCallback.waitForCallback(0);
> 
> You probably also want to do that after the updateSplashScreenImage call is
> done.

Using CallbackHelper breaks this test because it looks like the
updateSplashScreenImage call doesn't get run. I'm loathe to add another callback
interface on updateSplashScreenImage just to allow us to use CallbackHelper.

[dominickn, 2016-09-29 06:55:23]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-29 06:55:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dominickn, 2016-09-29 06:57:56]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-29 06:58:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-29 07:37:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-29 07:37:28]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[dominickn, 2016-09-29 09:45:52]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-29 09:46:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-29 10:54:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-29 10:54:39]

Dry run: Exceeded global retry quota

[Peter Wen, 2016-09-29 18:24:47]

Warming up prefs and DeferredStartupHandler lgtm.

https://codereview.chromium.org/2351113005/diff/100001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
(right):

https://codereview.chromium.org/2351113005/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java:134:
WebappRegistry.warmUpSharedPrefs(id);
To get the full benefit of warming up shared prefs on N you'll have to do
something similar to ChromeBrowserInitializer#warmUpSharedPrefs and start an
AsyncTask on N. Since it may end up being a lot of preferences to warm up (every
PWA), I think it's worth it, probably not necessary to add to
NotificationService, but your choice.

[gone, 2016-09-29 21:06:44]

lgtm

https://codereview.chromium.org/2351113005/diff/100001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationService.java
(right):

https://codereview.chromium.org/2351113005/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationService.java:81:
// Warm up the WebappRegistry and all web app SharedPreferences, as we will need
check
need to check

[dominickn, 2016-09-30 00:17:00]

dominickn@chromium.org changed reviewers:
+ msramek@chromium.org

[dominickn, 2016-09-30 00:17:01]

Thanks!

+msramek: PTAL at browsing_data.

https://codereview.chromium.org/2351113005/diff/100001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationService.java
(right):

https://codereview.chromium.org/2351113005/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationService.java:81:
// Warm up the WebappRegistry and all web app SharedPreferences, as we will need
check
On 2016/09/29 21:06:44, dfalcantara (slow) wrote:
> need to check

Done.

https://codereview.chromium.org/2351113005/diff/100001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
(right):

https://codereview.chromium.org/2351113005/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java:134:
WebappRegistry.warmUpSharedPrefs(id);
On 2016/09/29 18:24:47, Peter Wen wrote:
> To get the full benefit of warming up shared prefs on N you'll have to do
> something similar to ChromeBrowserInitializer#warmUpSharedPrefs and start an
> AsyncTask on N. Since it may end up being a lot of preferences to warm up
(every
> PWA), I think it's worth it, probably not necessary to add to
> NotificationService, but your choice.

This will only warm two preferences: the WebappRegistry, and the specific
preference file for the PWA being opened right now (= id). The rest of the PWAs
will have their preferences warmed in DeferredStartupHandler. Will an async task
complete before postInflationStartup?

[dominickn, 2016-09-30 00:17:10]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-30 00:17:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-30 01:25:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-30 01:25:52]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[msramek, 2016-09-30 08:58:05]

LGTM, thanks for following up on those past discussions :)

(but note that you have a compilation error there)

[dominickn, 2016-09-30 09:38:28]

On 2016/09/30 08:58:05, msramek wrote:
> LGTM, thanks for following up on those past discussions :)
> 
> (but note that you have a compilation error there)

Thanks for the catch! I was getting some false positives out of that bot earlier
so I had been ignoring it.

dfalcantara/wnwen: any more comments in response to my question in #ps6?

[dominickn, 2016-09-30 09:38:30]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-30 09:38:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-30 11:11:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-30 11:11:39]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[dominickn, 2016-09-30 13:00:58]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-30 13:01:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-30 13:49:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-30 13:49:29]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[dominickn, 2016-09-30 13:50:18]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-30 13:50:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Peter Wen, 2016-09-30 13:59:19]

Still lgtm % StrictMode whitelist

On rare occasions DeferredStartupHandler does not get run in a timely fashion,
usually due to the initial page not loading at all. Do you need a back-up check
on use to load up WebappRegistry.mStorage or is it okay for that to very rarely
to not run or run very late?

https://codereview.chromium.org/2351113005/diff/100001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
(right):

https://codereview.chromium.org/2351113005/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java:134:
WebappRegistry.warmUpSharedPrefs(id);
On 2016/09/30 00:17:01, dominickn wrote:
> On 2016/09/29 18:24:47, Peter Wen wrote:
> > To get the full benefit of warming up shared prefs on N you'll have to do
> > something similar to ChromeBrowserInitializer#warmUpSharedPrefs and start an
> > AsyncTask on N. Since it may end up being a lot of preferences to warm up
> (every
> > PWA), I think it's worth it, probably not necessary to add to
> > NotificationService, but your choice.
> 
> This will only warm two preferences: the WebappRegistry, and the specific
> preference file for the PWA being opened right now (= id). The rest of the
PWAs
> will have their preferences warmed in DeferredStartupHandler. Will an async
task
> complete before postInflationStartup?

Ah, right. Good point, didn't realize about the id, so that's not as bad.
Probably not worth the overhead of another AsyncTask as you need it so soon
after.

You'll need to then add a StrictMode policy whitelist for this call as on N
these warm up calls read disk and cause violations.

[commit-bot: I haz the power, 2016-09-30 14:43:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-30 14:43:26]

Dry run: This issue passed the CQ dry run.

[dominickn, 2016-10-04 01:09:17]

Thanks!

Regarding DeferredStartupHandler failing, if the occasions are rare, then it
should be okay. WebappRegistry is force-warmed when it's crucial (Notification
Service and WebappActivity).

The DeferredStartupHandler warming accounts for users who use the Clear Browsing
Data Dialog. If it's rare that the deferred task fails, then it should be even
more rare for it to fail and the Clear Browsing Data dialog to be run when it's
failed.

https://codereview.chromium.org/2351113005/diff/100001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
(right):

https://codereview.chromium.org/2351113005/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java:134:
WebappRegistry.warmUpSharedPrefs(id);
On 2016/09/30 13:59:19, Peter Wen wrote:
> On 2016/09/30 00:17:01, dominickn wrote:
> > On 2016/09/29 18:24:47, Peter Wen wrote:
> > > To get the full benefit of warming up shared prefs on N you'll have to do
> > > something similar to ChromeBrowserInitializer#warmUpSharedPrefs and start
an
> > > AsyncTask on N. Since it may end up being a lot of preferences to warm up
> > (every
> > > PWA), I think it's worth it, probably not necessary to add to
> > > NotificationService, but your choice.
> > 
> > This will only warm two preferences: the WebappRegistry, and the specific
> > preference file for the PWA being opened right now (= id). The rest of the
> PWAs
> > will have their preferences warmed in DeferredStartupHandler. Will an async
> task
> > complete before postInflationStartup?
> 
> Ah, right. Good point, didn't realize about the id, so that's not as bad.
> Probably not worth the overhead of another AsyncTask as you need it so soon
> after.
> 
> You'll need to then add a StrictMode policy whitelist for this call as on N
> these warm up calls read disk and cause violations.

Done. Thanks for the explanations on this. :)

[dominickn, 2016-10-04 03:17:23]

The CQ bit was checked by dominickn@chromium.org

[dominickn, 2016-10-04 03:17:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
dfalcantara@chromium.org, msramek@chromium.org, wnwen@chromium.org
Link to the patchset: https://codereview.chromium.org/2351113005/#ps200001
(title: "Add strict mode override in WebappActivity")

[commit-bot: I haz the power, 2016-10-04 03:17:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-04 04:20:12]

Description was changed from

==========
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
==========

to

==========
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
==========

[commit-bot: I haz the power, 2016-10-04 04:20:14]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2016-10-04 04:22:46]

Description was changed from

==========
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
==========

to

==========
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}
==========

[commit-bot: I haz the power, 2016-10-04 04:22:48]

Patchset 11 (id:??) landed as
https://crrev.com/78cf6596040662c0a73c0e4038bc84fb2b3c470a
Cr-Commit-Position: refs/heads/master@{#422703}

[Peng, 2016-10-04 12:16:15]

A revert of this CL (patchset #11 id:200001) has been created in
https://codereview.chromium.org/2384413003/ by penghuang@chromium.org.

The reason for reverting is: chrome_public_test_apk failed.

https://uberchromegw.corp.google.com/i/chromium.linux/builders/Android%20Test....

[dominickn, 2016-10-04 12:30:16]

On 2016/10/04 12:16:15, Peng wrote:
> A revert of this CL (patchset #11 id:200001) has been created in
> https://codereview.chromium.org/2384413003/ by mailto:penghuang@chromium.org.
> 
> The reason for reverting is: chrome_public_test_apk failed.
> 
>
https://uberchromegw.corp.google.com/i/chromium.linux/builders/Android%20Test....

Looks like the Android Tests (dbg) bot throws up on calling assert
ThreadUtils.runningOnUiThread() in some tests. The other test bots seem fine
with this CL.

I'll remove those asserts from WebappRegistry.java and reland this tomorrow
morning AU time. Reviewers, if you have any objections, please let me know.

[gone, 2016-10-04 17:14:50]

On 2016/10/04 12:30:16, dominickn wrote:
> On 2016/10/04 12:16:15, Peng wrote:
> > A revert of this CL (patchset #11 id:200001) has been created in
> > https://codereview.chromium.org/2384413003/ by
mailto:penghuang@chromium.org.
> > 
> > The reason for reverting is: chrome_public_test_apk failed.
> > 
> >
>
https://uberchromegw.corp.google.com/i/chromium.linux/builders/Android%20Test....
> 
> Looks like the Android Tests (dbg) bot throws up on calling assert
> ThreadUtils.runningOnUiThread() in some tests. The other test bots seem fine
> with this CL.
> 
> I'll remove those asserts from WebappRegistry.java and reland this tomorrow
> morning AU time. Reviewers, if you have any objections, please let me know.

Wouldn't be surprised if it was because the other bots were running L+, which
don't do asserts.  I'm fine with it as long as we're OK with not running
functions on the UI thread.

[dominickn, 2016-10-04 22:41:47]

Description was changed from

==========
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}
==========

to

==========
[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
Cr-Commit-Position: refs/heads/master@{#422703}
==========

[dominickn, 2016-10-04 23:04:01]

The CQ bit was checked by dominickn@chromium.org

[dominickn, 2016-10-04 23:04:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
msramek@chromium.org, wnwen@chromium.org, dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2351113005/#ps220001
(title: "Remove asserts which fail on Android Tests (dbg)")

[commit-bot: I haz the power, 2016-10-04 23:04:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-04 23:15:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-04 23:15:18]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[dominickn, 2016-10-05 01:43:13]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-05 01:43:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-05 01:53:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-05 01:53:48]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[dominickn, 2016-10-05 02:22:55]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-05 02:23:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-05 04:37:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-05 04:37:21]

Dry run: This issue passed the CQ dry run.

[dominickn, 2016-10-05 04:47:03]

The CQ bit was checked by dominickn@chromium.org

[dominickn, 2016-10-05 04:47:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
msramek@chromium.org, wnwen@chromium.org, dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2351113005/#ps240001
(title: "Checkstyle import order has changed overnight argh")

[commit-bot: I haz the power, 2016-10-05 04:47:20]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-05 04:52:53]

Description was changed from

==========
[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
Cr-Commit-Position: refs/heads/master@{#422703}
==========

to

==========
[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
Cr-Commit-Position: refs/heads/master@{#422703}
==========

[commit-bot: I haz the power, 2016-10-05 04:52:57]

Committed patchset #13 (id:240001)

[commit-bot: I haz the power, 2016-10-05 04:55:57]

Description was changed from

==========
[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
Cr-Commit-Position: refs/heads/master@{#422703}
==========

to

==========
[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}
==========

[commit-bot: I haz the power, 2016-10-05 04:55:58]

Patchset 13 (id:??) landed as
https://crrev.com/6e08a0b7993fb106e3dea796950521cfd04a47cf
Cr-Commit-Position: refs/heads/master@{#423077}

[Peng, 2016-10-05 13:19:31]

A revert of this CL (patchset #13 id:240001) has been created in
https://codereview.chromium.org/2390753004/ by penghuang@chromium.org.

The reason for reverting is: chrome_public_test_apk failure
https://uberchromegw.corp.google.com/i/chromium.linux/builders/Android%20Test....

[dvadym, 2016-10-05 13:55:39]

A revert of this CL (patchset #13 id:240001) has been created in
https://codereview.chromium.org/2397623004/ by dvadym@chromium.org.

The reason for reverting is: This CL is a reason of failure of
org.chromium.chrome.browser.webapps.WebappSplashScreenIconTest#testShowSplashIcon
org.chromium.chrome.browser.webapps.WebappSplashScreenIconTest#testUmaCustomIcon

https://uberchromegw.corp.google.com/i/chromium.linux/builders/Android%20Test....

[dominickn, 2016-10-05 23:52:03]

Description was changed from

==========
[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}
==========

to

==========
[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}
==========

[dominickn, 2016-10-05 23:53:53]

dfalcantara: PTAL at WebappActivityTestBase.java (the only change).

Looks like more assert tripping on KK driven by flakiness from async'ness. I
think explicitly waiting in WebappActivityTestBase is the way around it. I've
tracked down a KK device to test on and it seems to work. But this is proving to
be pretty difficult to land....

[dominickn, 2016-10-05 23:54:13]

Description was changed from

==========
[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}
==========

to

==========
[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
==========

[gone, 2016-10-06 00:03:38]

Well, third try's the charm.  It's surprising this worked before.

[gone, 2016-10-06 00:03:59]

(Still lgtm)

[dominickn, 2016-10-06 00:06:05]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-06 00:07:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-06 01:22:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-06 01:22:42]

Dry run: This issue passed the CQ dry run.

[dominickn, 2016-10-06 01:44:32]

The CQ bit was checked by dominickn@chromium.org

[dominickn, 2016-10-06 01:44:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
msramek@chromium.org, wnwen@chromium.org
Link to the patchset: https://codereview.chromium.org/2351113005/#ps260001
(title: "More flaky tests")

[commit-bot: I haz the power, 2016-10-06 01:44:56]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-06 01:52:48]

Description was changed from

==========
[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
==========

to

==========
[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
==========

[commit-bot: I haz the power, 2016-10-06 01:52:59]

Committed patchset #14 (id:260001)

[commit-bot: I haz the power, 2016-10-06 01:56:38]

Description was changed from

==========
[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
==========

to

==========
[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}
==========

[commit-bot: I haz the power, 2016-10-06 01:56:39]

Patchset 14 (id:??) landed as
https://crrev.com/8d4f92312dc1b0cb0bfcfe8ffd981b3555bfd537
Cr-Commit-Position: refs/heads/master@{#423389}

[nasko, 2016-10-06 18:29:27]

A revert of this CL (patchset #14 id:260001) has been created in
https://codereview.chromium.org/2395233002/ by nasko@chromium.org.

The reason for reverting is: chrome_public_test_apk test failures in
WebappSplashScreenTest.

https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...

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

[dominickn, 2016-10-12 04:56:02]

Description was changed from

==========
[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}
==========

to

==========
[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}
==========

[dominickn, 2016-10-12 04:58:19]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-12 04:58:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-12 05:08:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-12 05:08:39]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[dominickn, 2016-10-12 06:25:23]

Patchset #15 (id:280001) has been deleted

[dominickn, 2016-10-12 06:32:57]

Description was changed from

==========
[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}
==========

to

==========
[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
==========

[dominickn, 2016-10-12 06:33:37]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-12 06:33:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-12 07:52:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-12 07:52:46]

Dry run: This issue passed the CQ dry run.

[dominickn, 2016-10-12 08:03:46]

And here we go again...

dfalcantara: PTAL. I've redone the way that
WebappDataStorage#updateSplashScreenImage works. It no longer runs an AsyncTask,
which I believe is the root source of the flakiness in splash screen related
tests. Instead, it takes the string-encoded images and immediately writes to
SharedPreferences. The encoding must be done prior to calling the method.

The wrapping method ShortcutHelper#storeWebappSplashImage is now what runs the
AsyncTask to encode the image into a string before saving. All tests now run
through the non-AsyncTask pathway.

I've tested this on KK and M as much as I can. I figure you're as sick of
staring at this CL as I am!

[gone, 2016-10-13 17:43:04]

Lol, indeed.

LGTM, yet again.

https://chromiumcodereview.appspot.com/2351113005/diff/300001/chrome/android/...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
(right):

https://chromiumcodereview.appspot.com/2351113005/diff/300001/chrome/android/...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java:165:
* must have been encoded using ShortcutHelper#encodeBitmapAsString.
{@link ShortcutHelper#encodeBitmapAsString}

[dominickn, 2016-10-13 23:04:43]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-13 23:05:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-14 00:06:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-14 00:06:32]

Dry run: This issue passed the CQ dry run.

[dominickn, 2016-10-14 00:20:24]

*Closes eyes in hope*

https://codereview.chromium.org/2351113005/diff/300001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
(right):

https://codereview.chromium.org/2351113005/diff/300001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java:165:
* must have been encoded using ShortcutHelper#encodeBitmapAsString.
On 2016/10/13 17:43:03, dfalcantara (OOO 10.17-10.23) wrote:
> {@link ShortcutHelper#encodeBitmapAsString}

Done.

[dominickn, 2016-10-14 00:20:34]

The CQ bit was checked by dominickn@chromium.org

[dominickn, 2016-10-14 00:20:34]

The patchset sent to the CQ was uploaded after l-g-t-m from
msramek@chromium.org, wnwen@chromium.org, dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2351113005/#ps320001
(title: "Comments")

[commit-bot: I haz the power, 2016-10-14 00:21:03]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-14 00:28:57]

Committed patchset #16 (id:320001)

[commit-bot: I haz the power, 2016-10-14 00:31:03]

Description was changed from

==========
[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
==========

to

==========
[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

Committed: https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1
Cr-Commit-Position: refs/heads/master@{#425214}
==========

[commit-bot: I haz the power, 2016-10-14 00:31:06]

Patchset 16 (id:??) landed as
https://crrev.com/beceaa5ebceead7104900401ab96fe2facc26da1
Cr-Commit-Position: refs/heads/master@{#425214}

[whywhat, 2016-10-19 02:37:03]

avayvod@chromium.org changed reviewers:
+ avayvod@chromium.org

[whywhat, 2016-10-19 02:37:05]

https://codereview.chromium.org/2351113005/diff/200001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationService.java
(right):

https://codereview.chromium.org/2351113005/diff/200001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationService.java:85:
StrictMode.ThreadPolicy oldPolicy = StrictMode.allowThreadDiskReads();
Shouldn't this rather be added in WebappRegistry?

I'm running the instrumentation tests locally and DeferredStartupHandler calls
WebappRegistry.getInstance() that initializes shared prefs which on N is a
violation of the policy:

10-18 22:20:10.771 11325 11325 D StrictMode: StrictMode policy violation;
~duration=3 ms: android.os.StrictMode$StrictModeDiskReadViolation:
policy=18153503 violation=2
10-18 22:20:10.771 11325 11325 D StrictMode: 	at
android.os.StrictMode$AndroidBlockGuardPolicy.onReadFromDisk(StrictMode.java:1293)
10-18 22:20:10.771 11325 11325 D StrictMode: 	at
java.io.UnixFileSystem.checkAccess(UnixFileSystem.java:249)
10-18 22:20:10.771 11325 11325 D StrictMode: 	at
java.io.File.exists(File.java:780)
10-18 22:20:10.771 11325 11325 D StrictMode: 	at
android.app.ContextImpl.ensurePrivateDirExists(ContextImpl.java:512)
10-18 22:20:10.771 11325 11325 D StrictMode: 	at
android.app.ContextImpl.getPreferencesDir(ContextImpl.java:468)
10-18 22:20:10.771 11325 11325 D StrictMode: 	at
android.app.ContextImpl.getSharedPreferencesPath(ContextImpl.java:627)
10-18 22:20:10.771 11325 11325 D StrictMode: 	at
android.app.ContextImpl.getSharedPreferences(ContextImpl.java:345)
10-18 22:20:10.771 11325 11325 D StrictMode: 	at
android.content.ContextWrapper.getSharedPreferences(ContextWrapper.java:164)
10-18 22:20:10.771 11325 11325 D StrictMode: 	at
org.chromium.chrome.browser.webapps.WebappRegistry.openSharedPreferences(WebappRegistry.java:263)
10-18 22:20:10.771 11325 11325 D StrictMode: 	at
org.chromium.chrome.browser.webapps.WebappRegistry.<init>(WebappRegistry.java:268)
10-18 22:20:10.771 11325 11325 D StrictMode: 	at
org.chromium.chrome.browser.webapps.WebappRegistry.getInstance(WebappRegistry.java:69)
10-18 22:20:10.771 11325 11325 D StrictMode: 	at
org.chromium.chrome.browser.DeferredStartupHandler$2.run(DeferredStartupHandler.java:179)
10-18 22:20:10.771 11325 11325 D StrictMode: 	at
org.chromium.chrome.browser.DeferredStartupHandler$1.queueIdle(DeferredStartupHandler.java:117)
10-18 22:20:10.771 11325 11325 D StrictMode: 	at
android.os.MessageQueue.next(MessageQueue.java:392)
10-18 22:20:10.771 11325 11325 D StrictMode: 	at
android.os.Looper.loop(Looper.java:136)
10-18 22:20:10.771 11325 11325 D StrictMode: 	at
android.app.ActivityThread.main(ActivityThread.java:6088)
10-18 22:20:10.771 11325 11325 D StrictMode: 	at
java.lang.reflect.Method.invoke(Native Method)
10-18 22:20:10.771 11325 11325 D StrictMode: 	at
com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:886)
10-18 22:20:10.771 11325 11325 D StrictMode: 	at
com.android.internal.os.ZygoteInit.main(ZygoteInit.java:776)
10-18 22:20:10.772 11325 11325 D AndroidRuntime: Shutting down VM
10-18 22:20:10.779 11325 11325 E AndroidRuntime: FATAL EXCEPTION: main
10-18 22:20:10.779 11325 11325 E AndroidRuntime: Process: org.chromium.chrome,
PID: 11325
10-18 22:20:10.779 11325 11325 E AndroidRuntime:
android.os.StrictMode$StrictModeViolation: policy=18153503 violation=2
10-18 22:20:10.779 11325 11325 E AndroidRuntime: 	at
android.os.StrictMode.executeDeathPenalty(StrictMode.java:1539)
10-18 22:20:10.779 11325 11325 E AndroidRuntime: 	at
android.os.StrictMode.-wrap3(StrictMode.java)
10-18 22:20:10.779 11325 11325 E AndroidRuntime: 	at
android.os.StrictMode$AndroidBlockGuardPolicy.handleViolation(StrictMode.java:1532)
10-18 22:20:10.779 11325 11325 E AndroidRuntime: 	at
android.os.StrictMode$AndroidBlockGuardPolicy$1.run(StrictMode.java:1410)
10-18 22:20:10.779 11325 11325 E AndroidRuntime: 	at
android.os.Handler.handleCallback(Handler.java:751)
10-18 22:20:10.779 11325 11325 E AndroidRuntime: 	at
android.os.Handler.dispatchMessage(Handler.java:95)
10-18 22:20:10.779 11325 11325 E AndroidRuntime: 	at
android.os.Looper.loop(Looper.java:154)
10-18 22:20:10.779 11325 11325 E AndroidRuntime: 	at
android.app.ActivityThread.main(ActivityThread.java:6088)
10-18 22:20:10.779 11325 11325 E AndroidRuntime: 	at
java.lang.reflect.Method.invoke(Native Method)
10-18 22:20:10.779 11325 11325 E AndroidRuntime: 	at
com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:886)
10-18 22:20:10.779 11325 11325 E AndroidRuntime: 	at
com.android.internal.os.ZygoteInit.main(ZygoteInit.java:776)

Not sure how it escapes the N bots (we have some, right?).

[dominickn, 2016-10-19 02:44:35]

This CL passed the trybots multiple times before being reverted, so we clearly
have some coverage issues. :)

wnwen@ has a CL to move the DeferredStartup warming to an AsyncTask:
crrev.com/2421833003

[whywhat, 2016-10-19 02:55:08]

On 2016/10/19 at 02:44:35, dominickn wrote:
> This CL passed the trybots multiple times before being reverted, so we clearly
have some coverage issues. :)

Not even that, it finally landed 5 days ago...

> 
> wnwen@ has a CL to move the DeferredStartup warming to an AsyncTask:
crrev.com/2421833003

Ok, looks better than what I was going to send :)

[Peter Wen, 2016-10-19 12:57:01]

On 2016/10/19 02:55:08, whywhat wrote:
> On 2016/10/19 at 02:44:35, dominickn wrote:
> > This CL passed the trybots multiple times before being reverted, so we
clearly
> have some coverage issues. :)
> 
> Not even that, it finally landed 5 days ago...

It's true, we have a downstream N bot, but it has StrictMode disabled due to
prior OS-wide issues that I've since fixed (should...hope so?). I'll look into
re-enabling it. :)

[Peter Wen, 2016-10-19 14:19:55]

On 2016/10/19 12:57:01, Peter Wen wrote:
> It's true, we have a downstream N bot, but it has StrictMode disabled due to
> prior OS-wide issues that I've since fixed (should...hope so?). I'll look into
> re-enabling it. :)

http://crbug.com/657387