Added Version Info page to toolbar menu

Added Version Info page to toolbar menu

Added an option "Version Info" to the three dots menu in toolbar. This page would show blimp client version (same as version from manifest), operating System, engine IP, engine version, assigner info etc.

BUG=596737
Committed: https://crrev.com/2c8d461d1897b9ea842ffae9056a2530b3e33b5f
Cr-Commit-Position: refs/heads/master@{#387123}

[shaktisahu, 2016-03-31 19:25:33]

Description was changed from

==========
Added blimp engine ip, version to settings page

BUG=596737
==========

to

==========
Added Version Info page to toolbar menu

Added an option "Version Info" to the three dots menu in toolbar. This page
would show blimp client version (same as version from manifest), operating
System, engine IP, engine version, assigner info etc.
Assigner info is not added in this CL yet.

BUG=596737
==========

[shaktisahu, 2016-03-31 19:25:33]

shaktisahu@chromium.org changed reviewers:
+ marshallk@google.com

[Kevin M, 2016-03-31 19:41:48]

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

[Kevin M, 2016-03-31 19:41:49]

https://codereview.chromium.org/1842403003/diff/1/blimp/client/app/android/bl...
File blimp/client/app/android/blimp_client_session_android.h (right):

https://codereview.chromium.org/1842403003/diff/1/blimp/client/app/android/bl...
blimp/client/app/android/blimp_client_session_android.h:34:
base::android::ScopedJavaLocalRef<jstring> GetEngineIPAddress(
Do we need to break these out into separate fields? I think it might be
advantageous to use a single formatted string instead (e.g. GetEngineInfo()). It
could return a value like "ip:127.0.0.1 version:v51.0.123123" That way, if we
decide to add additional information like as locale info or production zone, we
don't have to poke more holes into JNI.

I can see how having separate fields might be nice for analytics KV pairs, but
if we're just driving a dialog box, then one string seems fine?

https://codereview.chromium.org/1842403003/diff/1/blimp/client/app/android/ja...
File blimp/client/app/android/java/src/org/chromium/blimp/toolbar/Toolbar.java
(right):

https://codereview.chromium.org/1842403003/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/toolbar/Toolbar.java:230:
// intent.putExtra("ASSIGNER_URL", mBlimpClientSession.GetDefaultAssignerURL());
Uncomment or remove this

https://codereview.chromium.org/1842403003/diff/1/blimp/client/app/android/ja...
File blimp/client/app/android/java/strings/android_blimp_strings.grd (right):

https://codereview.chromium.org/1842403003/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/strings/android_blimp_strings.grd:161: Operation
system
Operating system

https://codereview.chromium.org/1842403003/diff/1/blimp/client/session/blimp_...
File blimp/client/session/blimp_client_session.h (right):

https://codereview.chromium.org/1842403003/diff/1/blimp/client/session/blimp_...
blimp/client/session/blimp_client_session.h:85: std::string engineIPAddress_;
Make the fields private and add a public or protected const getter.

Can we just store the Assignment object directly?

[shaktisahu, 2016-04-01 01:46:46]

shaktisahu@chromium.org changed reviewers:
+ nyquist@chromium.org
- kmarshall@chromium.org

[Khushal, 2016-04-01 17:20:46]

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

[Khushal, 2016-04-01 17:20:46]

https://codereview.chromium.org/1842403003/diff/1/blimp/client/app/android/ja...
File blimp/client/app/android/java/src/org/chromium/blimp/toolbar/Toolbar.java
(right):

https://codereview.chromium.org/1842403003/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/toolbar/Toolbar.java:39:
private BlimpClientSession mBlimpClientSession;
Is it necessary to give the Toolbar a reference to the BlimpClientSession?

May be you could add an EngineConnectionObserver that is implemented by the
Toolbar. Let the native BlimpClientSession pass these values to the java layer
when a new assignment is received. The BlimpClientSession can keep track of the
observers and propagate these values to them. And the hook up for adding and
removing the observers can be done in the BlimpRendererActivity which creates
and destroys these objects. You will also avoid redundant JNI hops since here
each time the Preferences Activity needs to be started, a JNI call is made to
query these values.

[shaktisahu, 2016-04-06 06:47:08]

Description was changed from

==========
Added Version Info page to toolbar menu

Added an option "Version Info" to the three dots menu in toolbar. This page
would show blimp client version (same as version from manifest), operating
System, engine IP, engine version, assigner info etc.
Assigner info is not added in this CL yet.

BUG=596737
==========

to

==========
Added Version Info page to toolbar menu

Added an option "Version Info" to the three dots menu in toolbar. This page
would show blimp client version (same as version from manifest), operating
System, engine IP, engine version, assigner info etc.

BUG=596737
==========

[shaktisahu, 2016-04-06 06:50:25]

https://codereview.chromium.org/1842403003/diff/1/blimp/client/app/android/bl...
File blimp/client/app/android/blimp_client_session_android.h (right):

https://codereview.chromium.org/1842403003/diff/1/blimp/client/app/android/bl...
blimp/client/app/android/blimp_client_session_android.h:34:
base::android::ScopedJavaLocalRef<jstring> GetEngineIPAddress(
On 2016/03/31 19:41:48, Kevin M wrote:
> Do we need to break these out into separate fields? I think it might be
> advantageous to use a single formatted string instead (e.g. GetEngineInfo()).
It
> could return a value like "ip:127.0.0.1 version:v51.0.123123" That way, if we
> decide to add additional information like as locale info or production zone,
we
> don't have to poke more holes into JNI.
> 
> I can see how having separate fields might be nice for analytics KV pairs, but
> if we're just driving a dialog box, then one string seems fine?

Acknowledged.

https://codereview.chromium.org/1842403003/diff/1/blimp/client/app/android/ja...
File blimp/client/app/android/java/src/org/chromium/blimp/toolbar/Toolbar.java
(right):

https://codereview.chromium.org/1842403003/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/toolbar/Toolbar.java:39:
private BlimpClientSession mBlimpClientSession;
On 2016/04/01 17:20:46, Khushal wrote:
> Is it necessary to give the Toolbar a reference to the BlimpClientSession?
> 
> May be you could add an EngineConnectionObserver that is implemented by the
> Toolbar. Let the native BlimpClientSession pass these values to the java layer
> when a new assignment is received. The BlimpClientSession can keep track of
the
> observers and propagate these values to them. And the hook up for adding and
> removing the observers can be done in the BlimpRendererActivity which creates
> and destroys these objects. You will also avoid redundant JNI hops since here
> each time the Preferences Activity needs to be started, a JNI call is made to
> query these values.

Acknowledged.

https://codereview.chromium.org/1842403003/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/toolbar/Toolbar.java:230:
// intent.putExtra("ASSIGNER_URL", mBlimpClientSession.GetDefaultAssignerURL());
On 2016/03/31 19:41:49, Kevin M wrote:
> Uncomment or remove this

Done.

https://codereview.chromium.org/1842403003/diff/1/blimp/client/app/android/ja...
File blimp/client/app/android/java/strings/android_blimp_strings.grd (right):

https://codereview.chromium.org/1842403003/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/strings/android_blimp_strings.grd:161: Operation
system
On 2016/03/31 19:41:49, Kevin M wrote:
> Operating system

Done.

https://codereview.chromium.org/1842403003/diff/1/blimp/client/session/blimp_...
File blimp/client/session/blimp_client_session.h (right):

https://codereview.chromium.org/1842403003/diff/1/blimp/client/session/blimp_...
blimp/client/session/blimp_client_session.h:85: std::string engineIPAddress_;
On 2016/03/31 19:41:49, Kevin M wrote:
> Make the fields private and add a public or protected const getter.
> 
> Can we just store the Assignment object directly?

Acknowledged.
Moved these out of BlimpClientSession.

[Kevin M, 2016-04-06 18:29:59]

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

[Kevin M, 2016-04-06 18:30:00]

lgtm

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
File
blimp/client/app/android/java/src/org/chromium/blimp/session/BlimpClientSession.java
(right):

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/session/BlimpClientSession.java:60:
mNativeBlimpClientSessionAndroidPtr = nativeInit(DEFAULT_ASSIGNER_URL);
Save the actual assigner URL to a local variable (mAssignerUrl)?

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/session/BlimpClientSession.java:134:
new EngineInfo(engineIP, engineVersion, DEFAULT_ASSIGNER_URL));
Read from mAssignerUrl.

[nyquist, 2016-04-06 19:04:10]

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
File
blimp/client/app/android/java/src/org/chromium/blimp/BlimpRendererActivity.java
(right):

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/BlimpRendererActivity.java:81:
mToolbar.destroy();
Should this happen after you remove it as an observer?

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/BlimpRendererActivity.java:82:
mBlimpClientSession.removeObserver(mToolbar);
Also, it looks like mBlimpClientSession maybe null (at least we check for that
later).

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/BlimpRendererActivity.java:98:
mBlimpClientSession.destroy();
Should we remove ourselves as an observer as well before we do this?

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
File
blimp/client/app/android/java/src/org/chromium/blimp/session/BlimpClientSession.java
(right):

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/session/BlimpClientSession.java:55:
private List<ConnectionObserver> mObservers;
This should still be final.

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/session/BlimpClientSession.java:63:
public void addObserver(ConnectionObserver observer) {
I believe in Chromium we are supposed to document public method. I don't think
it helps for these methods, but we should still follow common practices.

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/session/BlimpClientSession.java:87:
mObservers.clear();
In case native publishes something from destroy in the future, should we clear
the list of observers first?

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
File
blimp/client/app/android/java/src/org/chromium/blimp/session/EngineInfo.java
(right):

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/session/EngineInfo.java:11:
private String mIPAddress;
All these fields except mConnected should be final.

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/session/EngineInfo.java:22:
public String getIPAddress() {
Did you consider just making the fields public (final) instead of these
accessors? They don't seem to gain you anything, at least for the final fields.

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
File blimp/client/app/android/java/src/org/chromium/blimp/toolbar/Toolbar.java
(right):

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/toolbar/Toolbar.java:249:
mEngineInfo.setConnected(true);
Can mEngineInfo be null? (here and below)

[shaktisahu, 2016-04-06 20:59:59]

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
File
blimp/client/app/android/java/src/org/chromium/blimp/BlimpRendererActivity.java
(right):

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/BlimpRendererActivity.java:81:
mToolbar.destroy();
On 2016/04/06 19:04:10, nyquist wrote:
> Should this happen after you remove it as an observer?

Done.

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/BlimpRendererActivity.java:82:
mBlimpClientSession.removeObserver(mToolbar);
On 2016/04/06 19:04:10, nyquist wrote:
> Also, it looks like mBlimpClientSession maybe null (at least we check for that
> later).

Done.

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/BlimpRendererActivity.java:98:
mBlimpClientSession.destroy();
On 2016/04/06 19:04:10, nyquist wrote:
> Should we remove ourselves as an observer as well before we do this?

Done.

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
File
blimp/client/app/android/java/src/org/chromium/blimp/session/BlimpClientSession.java
(right):

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/session/BlimpClientSession.java:55:
private List<ConnectionObserver> mObservers;
On 2016/04/06 19:04:10, nyquist wrote:
> This should still be final.

Done.

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/session/BlimpClientSession.java:60:
mNativeBlimpClientSessionAndroidPtr = nativeInit(DEFAULT_ASSIGNER_URL);
On 2016/04/06 18:30:00, Kevin M wrote:
> Save the actual assigner URL to a local variable (mAssignerUrl)?

Done.

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/session/BlimpClientSession.java:63:
public void addObserver(ConnectionObserver observer) {
On 2016/04/06 19:04:10, nyquist wrote:
> I believe in Chromium we are supposed to document public method. I don't think
> it helps for these methods, but we should still follow common practices.

Done.

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/session/BlimpClientSession.java:87:
mObservers.clear();
On 2016/04/06 19:04:10, nyquist wrote:
> In case native publishes something from destroy in the future, should we clear
> the list of observers first?

Done.

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/session/BlimpClientSession.java:134:
new EngineInfo(engineIP, engineVersion, DEFAULT_ASSIGNER_URL));
On 2016/04/06 18:30:00, Kevin M wrote:
> Read from mAssignerUrl.

Done.

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
File
blimp/client/app/android/java/src/org/chromium/blimp/session/EngineInfo.java
(right):

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/session/EngineInfo.java:11:
private String mIPAddress;
On 2016/04/06 19:04:10, nyquist wrote:
> All these fields except mConnected should be final.

Done.

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/session/EngineInfo.java:22:
public String getIPAddress() {
On 2016/04/06 19:04:10, nyquist wrote:
> Did you consider just making the fields public (final) instead of these
> accessors? They don't seem to gain you anything, at least for the final
fields.

Done.

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
File blimp/client/app/android/java/src/org/chromium/blimp/toolbar/Toolbar.java
(right):

https://codereview.chromium.org/1842403003/diff/40001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/toolbar/Toolbar.java:249:
mEngineInfo.setConnected(true);
On 2016/04/06 19:04:10, nyquist wrote:
> Can mEngineInfo be null? (here and below)

Done.

[Khushal, 2016-04-06 21:15:43]

one comment, other than that lgtm.

https://codereview.chromium.org/1842403003/diff/60001/blimp/client/app/androi...
File blimp/client/app/android/java/src/org/chromium/blimp/toolbar/Toolbar.java
(right):

https://codereview.chromium.org/1842403003/diff/60001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/toolbar/Toolbar.java:232:
if (mEngineInfo.isConnected()) {
One of the aims of this is for users to easily find the details of the engine to
report an error. Since the engine will get disconnected immediately after it
crashes, I'm wondering if we should show the details from the engine info, and
the connection status rather than not showing anything at all in case of a
disconnect.

[shaktisahu, 2016-04-07 23:21:18]

https://codereview.chromium.org/1842403003/diff/60001/blimp/client/app/androi...
File blimp/client/app/android/java/src/org/chromium/blimp/toolbar/Toolbar.java
(right):

https://codereview.chromium.org/1842403003/diff/60001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/toolbar/Toolbar.java:232:
if (mEngineInfo.isConnected()) {
On 2016/04/06 21:15:43, Khushal wrote:
> One of the aims of this is for users to easily find the details of the engine
to
> report an error. Since the engine will get disconnected immediately after it
> crashes, I'm wondering if we should show the details from the engine info, and
> the connection status rather than not showing anything at all in case of a
> disconnect.

Yeah, that sounds right. I am removing this condition. Later we might add
another field displaying the connection status.

[shaktisahu, 2016-04-08 18:58:32]

The CQ bit was checked by shaktisahu@chromium.org

[shaktisahu, 2016-04-08 18:58:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
kmarshall@chromium.org, khushalsagar@chromium.org
Link to the patchset: https://codereview.chromium.org/1842403003/#ps80001
(title: "Addressed Khushal's comments")

[commit-bot: I haz the power, 2016-04-08 18:59:07]

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

[commit-bot: I haz the power, 2016-04-08 19:40:09]

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

[commit-bot: I haz the power, 2016-04-08 19:40:10]

Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)

[shaktisahu, 2016-04-08 20:41:51]

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

[commit-bot: I haz the power, 2016-04-08 20:42:13]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1842403003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1842403003/80001

[shaktisahu, 2016-04-08 21:09:42]

The CQ bit was unchecked by shaktisahu@chromium.org

[nyquist, 2016-04-08 21:10:56]

lgtm

https://codereview.chromium.org/1842403003/diff/80001/blimp/client/app/androi...
File
blimp/client/app/android/java/src/org/chromium/blimp/session/BlimpClientSession.java
(right):

https://codereview.chromium.org/1842403003/diff/80001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/session/BlimpClientSession.java:60:
mAssignerUrl = DEFAULT_ASSIGNER_URL;
This looks a little bit odd since |mAssignerUrl| is final, and is assigned a
constant value. Is it really needed? Is this maybe for making changes in the
future? If it's to make future changes easier, that's fine with me.

[shaktisahu, 2016-04-09 00:07:51]

I had two untracked files that was not in the CL. Please review.

https://codereview.chromium.org/1842403003/diff/80001/blimp/client/app/androi...
File
blimp/client/app/android/java/src/org/chromium/blimp/session/BlimpClientSession.java
(right):

https://codereview.chromium.org/1842403003/diff/80001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/session/BlimpClientSession.java:60:
mAssignerUrl = DEFAULT_ASSIGNER_URL;
On 2016/04/08 21:10:56, nyquist wrote:
> This looks a little bit odd since |mAssignerUrl| is final, and is assigned a
> constant value. Is it really needed? Is this maybe for making changes in the
> future? If it's to make future changes easier, that's fine with me.

Yes, in future this will be populated from an list of assigners.

[nyquist, 2016-04-12 18:57:24]

https://codereview.chromium.org/1842403003/diff/100001/blimp/client/app/andro...
File
blimp/client/app/android/java/src/org/chromium/blimp/settings/AboutBlimpPreferences.java
(right):

https://codereview.chromium.org/1842403003/diff/100001/blimp/client/app/andro...
blimp/client/app/android/java/src/org/chromium/blimp/settings/AboutBlimpPreferences.java:73:
} catch (PackageManager.NameNotFoundException e) {
Could you stop the catch-statement after you set the version string?

[shaktisahu, 2016-04-12 20:51:16]

On 2016/04/12 18:57:24, nyquist wrote:
>
https://codereview.chromium.org/1842403003/diff/100001/blimp/client/app/andro...
> File
>
blimp/client/app/android/java/src/org/chromium/blimp/settings/AboutBlimpPreferences.java
> (right):
> 
>
https://codereview.chromium.org/1842403003/diff/100001/blimp/client/app/andro...
>
blimp/client/app/android/java/src/org/chromium/blimp/settings/AboutBlimpPreferences.java:73:
> } catch (PackageManager.NameNotFoundException e) {
> Could you stop the catch-statement after you set the version string?

Done

[nyquist, 2016-04-13 21:45:03]

still lgtm

[nyquist, 2016-04-13 21:45:52]

The CQ bit was checked by nyquist@chromium.org

[nyquist, 2016-04-13 21:45:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
kmarshall@chromium.org, khushalsagar@chromium.org
Link to the patchset: https://codereview.chromium.org/1842403003/#ps120001
(title: "Addressed review comments")

[commit-bot: I haz the power, 2016-04-13 21:46:17]

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

[commit-bot: I haz the power, 2016-04-13 22:24:50]

Description was changed from

==========
Added Version Info page to toolbar menu

Added an option "Version Info" to the three dots menu in toolbar. This page
would show blimp client version (same as version from manifest), operating
System, engine IP, engine version, assigner info etc.

BUG=596737
==========

to

==========
Added Version Info page to toolbar menu

Added an option "Version Info" to the three dots menu in toolbar. This page
would show blimp client version (same as version from manifest), operating
System, engine IP, engine version, assigner info etc.

BUG=596737
==========

[commit-bot: I haz the power, 2016-04-13 22:24:51]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-04-13 22:25:39]

Description was changed from

==========
Added Version Info page to toolbar menu

Added an option "Version Info" to the three dots menu in toolbar. This page
would show blimp client version (same as version from manifest), operating
System, engine IP, engine version, assigner info etc.

BUG=596737
==========

to

==========
Added Version Info page to toolbar menu

Added an option "Version Info" to the three dots menu in toolbar. This page
would show blimp client version (same as version from manifest), operating
System, engine IP, engine version, assigner info etc.

BUG=596737

Committed: https://crrev.com/2c8d461d1897b9ea842ffae9056a2530b3e33b5f
Cr-Commit-Position: refs/heads/master@{#387123}
==========

[commit-bot: I haz the power, 2016-04-13 22:25:40]

Patchset 7 (id:??) landed as
https://crrev.com/2c8d461d1897b9ea842ffae9056a2530b3e33b5f
Cr-Commit-Position: refs/heads/master@{#387123}