[Android Site Settings] Disable fetching important sites when not needed

[Android Site Settings] Disable fetching important sites when not needed.

Important sites could be a performance hit, especially because it might
hit disk w/ content settings querying, enumerating all bookmarks, etc.
This change lets you disable that.

BUG=732907
Review-Url: https://codereview.chromium.org/2953853002
Cr-Commit-Position: refs/heads/master@{#481758}
Committed: https://chromium.googlesource.com/chromium/src/+/226c79833104ddfbceeb6e866a3f532c7c1fec77

[dmurph, 2017-06-22 18:28:18]

Description was changed from

==========
[Android Site Settings] Disable fetching important sites when not needed

BUG=732907
==========

to

==========
[Android Site Settings] Disable fetching important sites when not needed.

Important sites could be a performance hit, especially because it might
hit disk w/ content settings querying, enumerating all bookmarks, etc.
This change lets you disable that.

BUG=732907
==========

[dmurph, 2017-06-22 18:28:18]

dmurph@chromium.org changed reviewers:
+ tedchoc@chromium.org, twellington@chromium.org

[dmurph, 2017-06-22 18:29:19]

Hey Theresa & Ted,

Can you PTAL at this change? I want to be able to disable fetching important
site info if it's unneeded.

Daniel

[dmurph, 2017-06-22 18:29:28]

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

[commit-bot: I haz the power, 2017-06-22 18:29:52]

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, 2017-06-22 19:24:05]

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

[commit-bot: I haz the power, 2017-06-22 19:24:05]

Dry run: This issue passed the CQ dry run.

[Ted C, 2017-06-22 20:06:49]

Are there no places loading important info?  Looks like all callers pass false
now.

https://codereview.chromium.org/2953853002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/WebsitePermissionsFetcher.java
(right):

https://codereview.chromium.org/2953853002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/WebsitePermissionsFetcher.java:42:
public WebsitePermissionsFetcher(WebsitePermissionsCallback callback) {
replace the two lines with:

this(callback, false);

https://codereview.chromium.org/2953853002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/WebsitePermissionsFetcher.java:48:
* @param callback The callback to run when the fetch is complete.
add a @param for fetchSiteImportantInfo

https://codereview.chromium.org/2953853002/diff/1/chrome/browser/android/pref...
File chrome/browser/android/preferences/website_preference_bridge.cc (right):

https://codereview.chromium.org/2953853002/diff/1/chrome/browser/android/pref...
chrome/browser/android/preferences/website_preference_bridge.cc:737:
fetch_important == JNI_TRUE);
do you need the == JNI_TRUE here?  In general, I've seen them just call through
w/o this.

[dmurph, 2017-06-23 00:20:33]

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

[commit-bot: I haz the power, 2017-06-23 00:21:03]

Dry run: CQ is trying da patch.

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

[dmurph, 2017-06-23 00:22:48]

We need to fetch it in ManageSpaceActivity - change that call to be true.

https://codereview.chromium.org/2953853002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/WebsitePermissionsFetcher.java
(right):

https://codereview.chromium.org/2953853002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/WebsitePermissionsFetcher.java:42:
public WebsitePermissionsFetcher(WebsitePermissionsCallback callback) {
On 2017/06/22 20:06:49, Ted C wrote:
> replace the two lines with:
> 
> this(callback, false);

Done.

https://codereview.chromium.org/2953853002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/WebsitePermissionsFetcher.java:48:
* @param callback The callback to run when the fetch is complete.
On 2017/06/22 20:06:49, Ted C wrote:
> add a @param for fetchSiteImportantInfo

Done.

https://codereview.chromium.org/2953853002/diff/1/chrome/browser/android/pref...
File chrome/browser/android/preferences/website_preference_bridge.cc (right):

https://codereview.chromium.org/2953853002/diff/1/chrome/browser/android/pref...
chrome/browser/android/preferences/website_preference_bridge.cc:737:
fetch_important == JNI_TRUE);
On 2017/06/22 20:06:49, Ted C wrote:
> do you need the == JNI_TRUE here?  In general, I've seen them just call
through
> w/o this.

No - I can remove it.

[Ted C, 2017-06-23 00:25:58]

lgtm

[Theresa, 2017-06-23 00:31:17]

lgtm

[dmurph, 2017-06-23 00:38:38]

The CQ bit was unchecked by dmurph@chromium.org

[dmurph, 2017-06-23 00:38:41]

The CQ bit was checked by dmurph@chromium.org

[commit-bot: I haz the power, 2017-06-23 00:38:59]

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, 2017-06-23 01:08:14]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1498178321747330,
"parent_rev": "8c84bfe3673e65b0a912f0bff2f726073caface5", "commit_rev":
"c55bde7d59dac15bba649394d685b13a0ab23485"}

[commit-bot: I haz the power, 2017-06-23 01:08:34]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1498178321747330,
"parent_rev": "2dd0ea84298d45e0962c595c591529bc84dd0ebd", "commit_rev":
"226c79833104ddfbceeb6e866a3f532c7c1fec77"}

[commit-bot: I haz the power, 2017-06-23 01:08:48]

Description was changed from

==========
[Android Site Settings] Disable fetching important sites when not needed.

Important sites could be a performance hit, especially because it might
hit disk w/ content settings querying, enumerating all bookmarks, etc.
This change lets you disable that.

BUG=732907
==========

to

==========
[Android Site Settings] Disable fetching important sites when not needed.

Important sites could be a performance hit, especially because it might
hit disk w/ content settings querying, enumerating all bookmarks, etc.
This change lets you disable that.

BUG=732907

Review-Url: https://codereview.chromium.org/2953853002
Cr-Commit-Position: refs/heads/master@{#481758}
Committed:
https://chromium.googlesource.com/chromium/src/+/226c79833104ddfbceeb6e866a3f...
==========

[commit-bot: I haz the power, 2017-06-23 01:08:50]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/226c79833104ddfbceeb6e866a3f...