DataUse UI strings should be changeable via Finch

DataUse UI strings should be changeable via Finch

DataUse UI message strings shown in snackbars and interstitial dialogs,
should be changeable via Finch.

Since the UI string overrides in Finch are not accessible from java
localization API, a JNI wrapper is used to get the overrides from C++
to java.

BUG=585160
Committed: https://crrev.com/490ad2014d0e57a2f6fb6f0b43bd080b4f24647b
Cr-Commit-Position: refs/heads/master@{#374774}

[Raj, 2016-02-08 18:40:09]

Description was changed from

==========
DataUse UI strings should be changeable via Finch

DataUse UI message strings shown in snackbars and interstitial dialogs,
should be changeable via Finch.

Since the UI string overrides in Finch are not accessible from java
localization API, a JNI wrapper is used to get the overrides from C++
to java.

BUG=485160
==========

to

==========
DataUse UI strings should be changeable via Finch

DataUse UI message strings shown in snackbars and interstitial dialogs,
should be changeable via Finch.

Since the UI string overrides in Finch are not accessible from java
localization API, a JNI wrapper is used to get the overrides from C++
to java.

BUG=485160
==========

[Raj, 2016-02-08 18:40:10]

rajendrant@chromium.org changed reviewers:
+ megjablon@chromium.org, tbansal@chromium.org

[Raj, 2016-02-08 18:47:09]

ptal,

I will add more reviewers once the approach is ok with everyone.

https://codereview.chromium.org/1678943002/diff/1/chrome/browser/android/data...
File chrome/browser/android/data_usage/data_use_tab_ui_manager_android.cc
(right):

https://codereview.chromium.org/1678943002/diff/1/chrome/browser/android/data...
chrome/browser/android/data_usage/data_use_tab_ui_manager_android.cc:20:
namespace {
The resource IDs from generated_resources.grd have different unique number in
c++ and java side. Due to this problem we need to have this common enum, and a
map from this enum to actual resource ID.

This enum needs to be in sync with c++ and java.
An alternative is to autogenerate this enum, which requires changes in a couple
of GN, GYP files.
https://chromiumcodereview.appspot.com/1578433002/

[tbansal1, 2016-02-08 19:06:33]

Moving myself to cc-list since megjablon@ is more familiar with this code.

https://codereview.chromium.org/1678943002/diff/1/chrome/browser/android/data...
File chrome/browser/android/data_usage/data_use_tab_ui_manager_android.cc
(right):

https://codereview.chromium.org/1678943002/diff/1/chrome/browser/android/data...
chrome/browser/android/data_usage/data_use_tab_ui_manager_android.cc:39:
uint32_t data_use_ui_message_id_map[DATA_USE_UI_MESSAGE_MAX] = {
#include <stdint.h>

[tbansal1, 2016-02-08 19:17:14]

tbansal@chromium.org changed reviewers:
- tbansal@chromium.org

[megjablon, 2016-02-08 23:06:35]

Thanks so much for doing this! Out of curiosity are there any more permanent
solutions being discussed?

https://codereview.chromium.org/1678943002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/datausage/DataUseTabUIManager.java
(right):

https://codereview.chromium.org/1678943002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/datausage/DataUseTabUIManager.java:250:
public static String getUIMessage(int messageID) {
Add javadoc. Include why we need to do this, something like "Gets native strings
which may be overridden by Finch." Also, getUIMessage() is a bit ambiguous,
maybe getDataUseUIString()?

[tbansal1, 2016-02-08 23:16:01]

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

[tbansal1, 2016-02-08 23:16:02]

https://codereview.chromium.org/1678943002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/datausage/DataUseTabUIManager.java
(right):

https://codereview.chromium.org/1678943002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/datausage/DataUseTabUIManager.java:56:
public static final int DATA_USE_TRACKING_STARTED_SNACKBAR_MESSAGE = 0;
Is it possible to generate enums from c++ automatically?
(search for GENERATED_JAVA_ENUM_PACKAGE in src/net/ for relevant examples).

[Raj, 2016-02-09 01:10:35]

There were some discussions below. No possible solution yet. Permanent solution
may take more time to land as well.

https://groups.google.com/a/google.com/forum/#!topic/finch-users/1yobrlVbRhw

https://codereview.chromium.org/1678943002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/datausage/DataUseTabUIManager.java
(right):

https://codereview.chromium.org/1678943002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/datausage/DataUseTabUIManager.java:56:
public static final int DATA_USE_TRACKING_STARTED_SNACKBAR_MESSAGE = 0;
On 2016/02/08 23:16:02, tbansal1 wrote:
> Is it possible to generate enums from c++ automatically?
> (search for GENERATED_JAVA_ENUM_PACKAGE in src/net/ for relevant examples).

Implemented similar to
https://chromiumcodereview.appspot.com/1578433002/

Not sure if this is overkill for our purposes.

https://codereview.chromium.org/1678943002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/datausage/DataUseTabUIManager.java:250:
public static String getUIMessage(int messageID) {
On 2016/02/08 23:06:35, megjablon wrote:
> Add javadoc. Include why we need to do this, something like "Gets native
strings
> which may be overridden by Finch." Also, getUIMessage() is a bit ambiguous,
> maybe getDataUseUIString()? 

Done.

https://codereview.chromium.org/1678943002/diff/1/chrome/browser/android/data...
File chrome/browser/android/data_usage/data_use_tab_ui_manager_android.cc
(right):

https://codereview.chromium.org/1678943002/diff/1/chrome/browser/android/data...
chrome/browser/android/data_usage/data_use_tab_ui_manager_android.cc:39:
uint32_t data_use_ui_message_id_map[DATA_USE_UI_MESSAGE_MAX] = {
On 2016/02/08 19:06:33, tbansal1 wrote:
> #include <stdint.h>

Done.

https://codereview.chromium.org/1678943002/diff/20001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (left):

https://codereview.chromium.org/1678943002/diff/20001/chrome/app/generated_re...
chrome/app/generated_resources.grd:14573: <!-- Data Use -->
Removing the android_java attribute so that these strings will not be in java
resources and will have reduced space.

[tbansal1, 2016-02-09 01:19:31]

lgtm. Thanks for using the autogen enum. Please wait for megjablon@ before
submitting.

[Raj, 2016-02-09 01:42:36]

rajendrant@chromium.org changed reviewers:
+ sky@chromium.org, yusufo@chromium.org

[Raj, 2016-02-09 01:42:36]

Adding more reviewers. ptal.

Thanks

[sky, 2016-02-09 04:57:34]

When adding reviewers please indicate what files need to be reviewed.

Thanks,

  -Scott

On Mon, Feb 8, 2016 at 5:42 PM,  <rajendrant@chromium.org> wrote:
> Adding more reviewers. ptal.
>
> Thanks
>
> https://codereview.chromium.org/1678943002/

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Raj, 2016-02-09 17:58:26]

Sure Scott.

yusuf: ptal data_use_dialog.xml

sky: ptal chrome/BUILD.gn chrome/BUILD.gn chrome/chrome.gyp
EmbedContentViewActivity.java

https://codereview.chromium.org/1678943002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/EmbedContentViewActivity.java
(right):

https://codereview.chromium.org/1678943002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/EmbedContentViewActivity.java:63:
intent.putExtra(TITLE_INTENT_EXTRA, title);
I hope storing the string in intent extras, does not affect performance.

[sky, 2016-02-09 20:33:14]

https://codereview.chromium.org/1678943002/diff/20001/chrome/chrome.gyp
File chrome/chrome.gyp (right):

https://codereview.chromium.org/1678943002/diff/20001/chrome/chrome.gyp#newco...
chrome/chrome.gyp:692: 'source_file':
'../chrome/browser/android/data_usage/data_use_tab_ui_manager_android.cc',
Why do you have ../chrome here? Can't you just have browser/android/... ?

[Raj, 2016-02-09 20:58:20]

Addressed sky comments.

https://codereview.chromium.org/1678943002/diff/20001/chrome/chrome.gyp
File chrome/chrome.gyp (right):

https://codereview.chromium.org/1678943002/diff/20001/chrome/chrome.gyp#newco...
chrome/chrome.gyp:692: 'source_file':
'../chrome/browser/android/data_usage/data_use_tab_ui_manager_android.cc',
On 2016/02/09 20:33:14, sky wrote:
> Why do you have ../chrome here? Can't you just have browser/android/... ?

Done.

[sky, 2016-02-09 21:07:48]

LGTM

[Yusuf, 2016-02-09 21:22:30]

lgtm for resources

[megjablon, 2016-02-10 20:35:15]

lgtm

[Raj, 2016-02-10 20:41:06]

The CQ bit was checked by rajendrant@chromium.org

[Raj, 2016-02-10 20:41:06]

The patchset sent to the CQ was uploaded after l-g-t-m from tbansal@chromium.org
Link to the patchset: https://codereview.chromium.org/1678943002/#ps40001
(title: "Addressed sky comments")

[commit-bot: I haz the power, 2016-02-10 20:43:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1678943002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1678943002/40001

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

Description was changed from

==========
DataUse UI strings should be changeable via Finch

DataUse UI message strings shown in snackbars and interstitial dialogs,
should be changeable via Finch.

Since the UI string overrides in Finch are not accessible from java
localization API, a JNI wrapper is used to get the overrides from C++
to java.

BUG=485160
==========

to

==========
DataUse UI strings should be changeable via Finch

DataUse UI message strings shown in snackbars and interstitial dialogs,
should be changeable via Finch.

Since the UI string overrides in Finch are not accessible from java
localization API, a JNI wrapper is used to get the overrides from C++
to java.

BUG=485160
==========

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

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-02-16 22:32:20]

Description was changed from

==========
DataUse UI strings should be changeable via Finch

DataUse UI message strings shown in snackbars and interstitial dialogs,
should be changeable via Finch.

Since the UI string overrides in Finch are not accessible from java
localization API, a JNI wrapper is used to get the overrides from C++
to java.

BUG=485160
==========

to

==========
DataUse UI strings should be changeable via Finch

DataUse UI message strings shown in snackbars and interstitial dialogs,
should be changeable via Finch.

Since the UI string overrides in Finch are not accessible from java
localization API, a JNI wrapper is used to get the overrides from C++
to java.

BUG=485160

Committed: https://crrev.com/490ad2014d0e57a2f6fb6f0b43bd080b4f24647b
Cr-Commit-Position: refs/heads/master@{#374774}
==========

[commit-bot: I haz the power, 2016-02-16 22:32:21]

Patchset 3 (id:??) landed as
https://crrev.com/490ad2014d0e57a2f6fb6f0b43bd080b4f24647b
Cr-Commit-Position: refs/heads/master@{#374774}

[Raj, 2016-02-18 20:40:12]

Description was changed from

==========
DataUse UI strings should be changeable via Finch

DataUse UI message strings shown in snackbars and interstitial dialogs,
should be changeable via Finch.

Since the UI string overrides in Finch are not accessible from java
localization API, a JNI wrapper is used to get the overrides from C++
to java.

BUG=485160

Committed: https://crrev.com/490ad2014d0e57a2f6fb6f0b43bd080b4f24647b
Cr-Commit-Position: refs/heads/master@{#374774}
==========

to

==========
DataUse UI strings should be changeable via Finch

DataUse UI message strings shown in snackbars and interstitial dialogs,
should be changeable via Finch.

Since the UI string overrides in Finch are not accessible from java
localization API, a JNI wrapper is used to get the overrides from C++
to java.

BUG=585160

Committed: https://crrev.com/490ad2014d0e57a2f6fb6f0b43bd080b4f24647b
Cr-Commit-Position: refs/heads/master@{#374774}
==========