Refactor IntentPickerBubbleView::ShowBubble()

Refactor IntentPickerBubbleView::ShowBubble()

so that the function takes content::WebContents* instead of
content::NavigationHandle*. That allows us to use the bubble
view class outside the throttle.

BUG=b:29079617, b:31406784
TEST=try

Committed: https://crrev.com/f9779cbd50b960fec92014569fa3e395607dd75d
Cr-Commit-Position: refs/heads/master@{#418628}

[Yusuke Sato, 2016-09-14 03:40:20]

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

[commit-bot: I haz the power, 2016-09-14 03:40:30]

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, 2016-09-14 04:28:09]

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

[commit-bot: I haz the power, 2016-09-14 04:28:09]

Dry run: This issue passed the CQ dry run.

[Yusuke Sato, 2016-09-14 16:58:02]

Description was changed from

==========
wip

BUG=b:29079617, b:31406784
==========

to

==========
Refactor IntentPickerBubbleView::ShowBubble()

so that the function takes content::WebContents* instead of
content::NavigationHandle*. That allows us to use the bubble
view class outside the throttle.

BUG=b:29079617, b:31406784
TEST=try
==========

[Yusuke Sato, 2016-09-14 17:00:51]

yusukes@chromium.org changed reviewers:
+ djacobo@google.com, sky@chromium.org

[Yusuke Sato, 2016-09-14 17:00:52]

Scott, could you have a look at this?

David: FYI.

[djacobo_, 2016-09-14 17:17:02]

lgtm with a few nits

https://codereview.chromium.org/2335313003/diff/1/chrome/browser/chromeos/arc...
File chrome/browser/chromeos/arc/arc_navigation_throttle.h (right):

https://codereview.chromium.org/2335313003/diff/1/chrome/browser/chromeos/arc...
chrome/browser/chromeos/arc/arc_navigation_throttle.h:50:
base::Callback<void(content::WebContents* contents,
nit: replace contents with web_contents

https://codereview.chromium.org/2335313003/diff/1/chrome/browser/ui/views/int...
File chrome/browser/ui/views/intent_picker_bubble_view.cc (right):

https://codereview.chromium.org/2335313003/diff/1/chrome/browser/ui/views/int...
chrome/browser/ui/views/intent_picker_bubble_view.cc:53: content::WebContents*
contents,
nit: other parts of the class refer to |web_contents| instead of just
|contents|, I think it would be a bit cleaner to use |web_contents| in all the
class.

https://codereview.chromium.org/2335313003/diff/1/chrome/browser/ui/views/int...
File chrome/browser/ui/views/intent_picker_bubble_view.h (right):

https://codereview.chromium.org/2335313003/diff/1/chrome/browser/ui/views/int...
chrome/browser/ui/views/intent_picker_bubble_view.h:63: static void
ShowBubble(content::WebContents* contents,
nit: same here, let's go either |contents| or |web_contents| in all the class.

[sky, 2016-09-14 18:08:15]

LGTM

https://codereview.chromium.org/2335313003/diff/1/chrome/browser/ui/browser_d...
File chrome/browser/ui/browser_dialogs.h (right):

https://codereview.chromium.org/2335313003/diff/1/chrome/browser/ui/browser_d...
chrome/browser/ui/browser_dialogs.h:178: using BubbleShowPtr = void (*)(
If you didn't, please run git cl format.

[Yusuke Sato, 2016-09-14 18:15:55]

Thanks!

https://codereview.chromium.org/2335313003/diff/1/chrome/browser/chromeos/arc...
File chrome/browser/chromeos/arc/arc_navigation_throttle.h (right):

https://codereview.chromium.org/2335313003/diff/1/chrome/browser/chromeos/arc...
chrome/browser/chromeos/arc/arc_navigation_throttle.h:50:
base::Callback<void(content::WebContents* contents,
On 2016/09/14 17:16:45, djacobo wrote:
> nit: replace contents with web_contents

Done.

https://codereview.chromium.org/2335313003/diff/1/chrome/browser/ui/browser_d...
File chrome/browser/ui/browser_dialogs.h (right):

https://codereview.chromium.org/2335313003/diff/1/chrome/browser/ui/browser_d...
chrome/browser/ui/browser_dialogs.h:178: using BubbleShowPtr = void (*)(
On 2016/09/14 18:08:15, sky wrote:
> If you didn't, please run git cl format.

Actually, the current (my CL's) format is by git cl format.

https://codereview.chromium.org/2335313003/diff/1/chrome/browser/ui/views/int...
File chrome/browser/ui/views/intent_picker_bubble_view.cc (right):

https://codereview.chromium.org/2335313003/diff/1/chrome/browser/ui/views/int...
chrome/browser/ui/views/intent_picker_bubble_view.cc:53: content::WebContents*
contents,
On 2016/09/14 17:16:53, djacobo wrote:
> nit: other parts of the class refer to |web_contents| instead of just
> |contents|, I think it would be a bit cleaner to use |web_contents| in all the
> class.

Done.

https://codereview.chromium.org/2335313003/diff/1/chrome/browser/ui/views/int...
File chrome/browser/ui/views/intent_picker_bubble_view.h (right):

https://codereview.chromium.org/2335313003/diff/1/chrome/browser/ui/views/int...
chrome/browser/ui/views/intent_picker_bubble_view.h:63: static void
ShowBubble(content::WebContents* contents,
On 2016/09/14 17:16:56, djacobo wrote:
> nit: same here, let's go either |contents| or |web_contents| in all the class.

Done.

[Yusuke Sato, 2016-09-14 18:16:12]

The CQ bit was checked by yusukes@chromium.org

[Yusuke Sato, 2016-09-14 18:16:13]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org,
djacobo@google.com
Link to the patchset: https://codereview.chromium.org/2335313003/#ps20001
(title: "Address comments")

[commit-bot: I haz the power, 2016-09-14 18:16:52]

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, 2016-09-14 19:05:22]

Description was changed from

==========
Refactor IntentPickerBubbleView::ShowBubble()

so that the function takes content::WebContents* instead of
content::NavigationHandle*. That allows us to use the bubble
view class outside the throttle.

BUG=b:29079617, b:31406784
TEST=try
==========

to

==========
Refactor IntentPickerBubbleView::ShowBubble()

so that the function takes content::WebContents* instead of
content::NavigationHandle*. That allows us to use the bubble
view class outside the throttle.

BUG=b:29079617, b:31406784
TEST=try
==========

[commit-bot: I haz the power, 2016-09-14 19:05:23]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-14 19:09:24]

Description was changed from

==========
Refactor IntentPickerBubbleView::ShowBubble()

so that the function takes content::WebContents* instead of
content::NavigationHandle*. That allows us to use the bubble
view class outside the throttle.

BUG=b:29079617, b:31406784
TEST=try
==========

to

==========
Refactor IntentPickerBubbleView::ShowBubble()

so that the function takes content::WebContents* instead of
content::NavigationHandle*. That allows us to use the bubble
view class outside the throttle.

BUG=b:29079617, b:31406784
TEST=try

Committed: https://crrev.com/f9779cbd50b960fec92014569fa3e395607dd75d
Cr-Commit-Position: refs/heads/master@{#418628}
==========

[commit-bot: I haz the power, 2016-09-14 19:09:25]

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