Handle multiple signin_internal_uis by using a linked_ptr

Handle multiple signin_internal_uis by using a linked_ptr.

Fixes an issue where chrome://signin-internals page is opened multiple times, and the scoped_ptr was getting de-allocated by the first object. Moving to a linked_ptr for more robust reference counting.

BUG=416802
Committed: https://crrev.com/a5dc279bb8398b6edfef7740622c47a113261753
Cr-Commit-Position: refs/heads/master@{#296932}

[Mike Lerman, 2014-09-24 16:09:55]

mlerman@chromium.org changed reviewers:
+ nkostylev@chromium.org, rogerta@chromium.org

[Mike Lerman, 2014-09-24 16:09:55]

Hi Roger and nikita,

Please take a look. Thanks!

Mike

[Nikita (slow), 2014-09-24 16:24:15]

lgtm

[Roger Tawa OOO till Jul 10th, 2014-09-25 01:14:06]

Hi Mike,

I think it would be better to have the same arg type for the two observer
methods.  Looks a little strange to have one as a scoped_ptr and another as a
linked_ptr.  See suggestions below.

Also, I noticed while working on https://codereview.chromium.org/601643002/ that
about-signin-internals makes a call to /ListAccounts on each profile startup. 
Can this be changed to request only when there is at least one ui displayed?

Thanks.

https://codereview.chromium.org/602773002/diff/1/chrome/browser/ui/webui/sign...
File chrome/browser/ui/webui/signin_internals_ui.h (right):

https://codereview.chromium.org/602773002/diff/1/chrome/browser/ui/webui/sign...
chrome/browser/ui/webui/signin_internals_ui.h:34:
linked_ptr<base::DictionaryValue> info) OVERRIDE;
Would be better to change this function and the one above to take arg:

   const base::Dictionary* info

https://codereview.chromium.org/602773002/diff/1/components/signin/core/brows...
File components/signin/core/browser/about_signin_internals.cc (right):

https://codereview.chromium.org/602773002/diff/1/components/signin/core/brows...
components/signin/core/browser/about_signin_internals.cc:226:
signin_status_.ToValue(client_->GetProductVersion())));
Above line 223 do:

   scoped_ptr<base::Dictionary> dict(
       signin_status_.ToValue(client_->GetProductVersion()));

the pass dict.get() at line 226.

https://codereview.chromium.org/602773002/diff/1/components/signin/core/brows...
components/signin/core/browser/about_signin_internals.cc:327:
linked_ptr<base::DictionaryValue> signin_status(new base::DictionaryValue());
Could declare this on the stack:

   base::Dictionary signin_status;

then pass |&signin_status| at line 343 below.

[Mike Lerman, 2014-09-25 17:28:34]

I changed the ListAccounts call so that we don't initiate that request if we're
sure there are no UIs to observe it. 

If a call's made and no one's around to observe the result, is it still a
success?

https://codereview.chromium.org/602773002/diff/1/chrome/browser/ui/webui/sign...
File chrome/browser/ui/webui/signin_internals_ui.h (right):

https://codereview.chromium.org/602773002/diff/1/chrome/browser/ui/webui/sign...
chrome/browser/ui/webui/signin_internals_ui.h:34:
linked_ptr<base::DictionaryValue> info) OVERRIDE;
On 2014/09/25 01:14:05, Roger Tawa wrote:
> Would be better to change this function and the one above to take arg:
> 
>    const base::Dictionary* info
Done.

https://codereview.chromium.org/602773002/diff/1/components/signin/core/brows...
File components/signin/core/browser/about_signin_internals.cc (right):

https://codereview.chromium.org/602773002/diff/1/components/signin/core/brows...
components/signin/core/browser/about_signin_internals.cc:226:
signin_status_.ToValue(client_->GetProductVersion())));
On 2014/09/25 01:14:05, Roger Tawa wrote:
> Above line 223 do:
> 
>    scoped_ptr<base::Dictionary> dict(
>        signin_status_.ToValue(client_->GetProductVersion()));
> 
> the pass dict.get() at line 226.

Done.

https://codereview.chromium.org/602773002/diff/1/components/signin/core/brows...
components/signin/core/browser/about_signin_internals.cc:327:
linked_ptr<base::DictionaryValue> signin_status(new base::DictionaryValue());
On 2014/09/25 01:14:05, Roger Tawa wrote:
> Could declare this on the stack:
> 
>    base::Dictionary signin_status;
> 
> then pass |&signin_status| at line 343 below.

Done.

[Roger Tawa OOO till Jul 10th, 2014-09-26 03:44:13]

lgtm

I guess v2 json form does the trick?

[Mike Lerman, 2014-09-26 12:50:22]

The CQ bit was checked by mlerman@chromium.org

[commit-bot: I haz the power, 2014-09-26 12:51:28]

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

[commit-bot: I haz the power, 2014-09-26 13:37:40]

Committed patchset #3 (id:40001) as 0c50a00f4efb2c74a612292dbe096e320cfd6dc5

[commit-bot: I haz the power, 2014-09-26 13:38:04]

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