Follow up CL for issue:541713002

Follow up CL for 541713002

As suggested by 'aurmias' fixed mentioned comments in issue 541713002
Fixed issues:
1.Using one button for stop and reload option.
2.Removed tabs from chrome_shell_acivity.xml
3.moved resource from chrome/android/java/res to chrome/android/shell/res

BUG=

Committed: https://crrev.com/569c527d3a9b859850c92e09d13c531c680c9cb8
Cr-Commit-Position: refs/heads/master@{#294164}

[ankit, 2014-09-08 07:27:37]

ankit2.kumar@samsung.com changed reviewers:
+ bauerb@chromium.org

[ankit, 2014-09-08 07:27:38]

@Bernhard
PTAL

[Bernhard Bauer, 2014-09-08 07:58:42]

bauerb@chromium.org changed reviewers:
+ aurimas@chromium.org

[Bernhard Bauer, 2014-09-08 07:58:42]

+Aurimas

https://codereview.chromium.org/546383002/diff/1/chrome/android/shell/java/sr...
File
chrome/android/shell/java/src/org/chromium/chrome/shell/ChromeShellToolbar.java
(right):

https://codereview.chromium.org/546383002/diff/1/chrome/android/shell/java/sr...
chrome/android/shell/java/src/org/chromium/chrome/shell/ChromeShellToolbar.java:94:
if (mLoading) mStopReloadButton.setImageResource(R.drawable.btn_stop_normal);
If one branch of if-else uses braces, the other one should too.

https://codereview.chromium.org/546383002/diff/1/chrome/android/shell/java/sr...
chrome/android/shell/java/src/org/chromium/chrome/shell/ChromeShellToolbar.java:191:
if (mLoading) mTab.getContentViewCore().stopLoading();
Putting the if-statement and the body on the same line is only allowed if
everything fits on one line. Here it doesn't, because there's also the
else-statement. You'll have to use braces and separate lines.

https://codereview.chromium.org/546383002/diff/1/chrome/android/shell/res/lay...
File chrome/android/shell/res/layout/chrome_shell_activity.xml (right):

https://codereview.chromium.org/546383002/diff/1/chrome/android/shell/res/lay...
chrome/android/shell/res/layout/chrome_shell_activity.xml:22:
android:src="@drawable/btn_stop_normal"
I'd rather initialize this with the reload button, as the initial state of
|mLoading| is going to be false.

[ankit, 2014-09-08 08:53:14]

@Bernhard
PTAL at comment and suggest

https://codereview.chromium.org/546383002/diff/1/chrome/android/shell/res/lay...
File chrome/android/shell/res/layout/chrome_shell_activity.xml (right):

https://codereview.chromium.org/546383002/diff/1/chrome/android/shell/res/lay...
chrome/android/shell/res/layout/chrome_shell_activity.xml:22:
android:src="@drawable/btn_stop_normal"
On 2014/09/08 07:58:42, Bernhard Bauer wrote:
> I'd rather initialize this with the reload button, as the initial state of
> |mLoading| is going to be false.

If initialized with reload button, on app launch transition of image
change(reload -> stop button) happens, which is required as page is loading. 
Initializing with stop button takes care of this scenario.
Please suggest.

[Bernhard Bauer, 2014-09-08 09:04:53]

https://codereview.chromium.org/546383002/diff/1/chrome/android/shell/res/lay...
File chrome/android/shell/res/layout/chrome_shell_activity.xml (right):

https://codereview.chromium.org/546383002/diff/1/chrome/android/shell/res/lay...
chrome/android/shell/res/layout/chrome_shell_activity.xml:22:
android:src="@drawable/btn_stop_normal"
On 2014/09/08 08:53:14, ankit wrote:
> On 2014/09/08 07:58:42, Bernhard Bauer wrote:
> > I'd rather initialize this with the reload button, as the initial state of
> > |mLoading| is going to be false.
> 
> If initialized with reload button, on app launch transition of image
> change(reload -> stop button) happens, which is required as page is loading. 
> Initializing with stop button takes care of this scenario.
> Please suggest.

Okay... in that case, can we initialize |mLoading| to true?

[ankit, 2014-09-08 09:23:08]

@Bernhard
PTAL
I have incorporated review comments.

https://codereview.chromium.org/546383002/diff/1/chrome/android/shell/java/sr...
File
chrome/android/shell/java/src/org/chromium/chrome/shell/ChromeShellToolbar.java
(right):

https://codereview.chromium.org/546383002/diff/1/chrome/android/shell/java/sr...
chrome/android/shell/java/src/org/chromium/chrome/shell/ChromeShellToolbar.java:94:
if (mLoading) mStopReloadButton.setImageResource(R.drawable.btn_stop_normal);
On 2014/09/08 07:58:42, Bernhard Bauer wrote:
> If one branch of if-else uses braces, the other one should too.

Done.

https://codereview.chromium.org/546383002/diff/1/chrome/android/shell/java/sr...
chrome/android/shell/java/src/org/chromium/chrome/shell/ChromeShellToolbar.java:191:
if (mLoading) mTab.getContentViewCore().stopLoading();
On 2014/09/08 07:58:42, Bernhard Bauer wrote:
> Putting the if-statement and the body on the same line is only allowed if
> everything fits on one line. Here it doesn't, because there's also the
> else-statement. You'll have to use braces and separate lines.

Done.

[Bernhard Bauer, 2014-09-08 09:24:42]

LG, let's wait for Aurimas for the final review.

[ankit, 2014-09-08 09:29:50]

On 2014/09/08 09:24:42, Bernhard Bauer wrote:
> LG, let's wait for Aurimas for the final review.

Thanks.

[ankit, 2014-09-09 13:44:15]

On 2014/09/08 09:29:50, ankit wrote:
> On 2014/09/08 09:24:42, Bernhard Bauer wrote:
> > LG, let's wait for Aurimas for the final review.
> 
> Thanks.

@aurimas
PTAL

[aurimas (slooooooooow), 2014-09-09 15:18:36]

lgtm

[ankit, 2014-09-10 11:56:48]

On 2014/09/09 15:18:36, aurimas wrote:
> lgtm

@Bernhard
PTAL.

[Bernhard Bauer, 2014-09-10 12:32:43]

lgtm

[ankit, 2014-09-10 12:42:05]

On 2014/09/10 12:32:43, Bernhard Bauer wrote:
> lgtm

Thanks.

[ankit, 2014-09-10 12:42:11]

The CQ bit was checked by ankit2.kumar@samsung.com

[commit-bot: I haz the power, 2014-09-10 12:42:23]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ankit2.kumar@samsung.com/546383002/20001

[commit-bot: I haz the power, 2014-09-10 13:41:49]

Committed patchset #2 (id:20001) as c31ae5d9482c68deded72254b2bb7fd1917be547

[commit-bot: I haz the power, 2014-09-10 13:46:23]

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