Make web intents picker work as constrained dialog instead of InfoBubble

Make web intents picker work as constrained dialog instead of InfoBubble

BUG=113333
TEST=WebIntentBubbleControllerTest.*; Bring up an intent picker and marvel at constrained dialog


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=125305

[Nico, 2012-03-03 06:13:49]

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/web_intent_bubble_controller.h (right):

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/web_intent_bubble_controller.h:5: #ifndef
CHROME_BROWSER_UI_COCOA_WEB_INTENT_BUBBLE_CONTROLLER_H_
Should you rename the file, too? (tools/git/mass-rename.sh if you haven't seen
it)

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/web_intent_bubble_controller.mm (right):

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/web_intent_bubble_controller.mm:83: window.reset(
Use scoped_ptr's constructor to set the pointer instead of empty constructor +
reset()

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/web_intent_bubble_controller.mm:99: // Also called when
user navigates to another page while the sheet is open.
No space in front of *

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/web_intent_bubble_controller.mm:288: windowFrame=[[self
window] frameRectForContentRect:windowFrame];
indent looks off

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/web_intent_bubble_controller_unittest.mm (right):

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/web_intent_bubble_controller_unittest.mm:63: Browser*
browser=(Browser*)0xdeadbeef;
Huh? If the browser object isn't needed, maybe WIPC shouldn't take it in its
constructor?

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/web_intent_bubble_controller_unittest.mm:65: &delegate_,
&model_));
indent is off

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm (right):

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm:31: // themselves, and the
WebIntentPicker needs to actually live longer than the
s/actually//

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm:32: // constrained window, we
need this forwarding class
.

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm:37:
WebIntentBubbleController* sheet_controller);
indent 1 more

[groby-ooo-7-16, 2012-03-03 23:41:48]

New version - slightly disturbed that the renamed files didn't get removed. Bug
in Rietveld? Or in my CL?

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/web_intent_bubble_controller.h (right):

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/web_intent_bubble_controller.h:5: #ifndef
CHROME_BROWSER_UI_COCOA_WEB_INTENT_BUBBLE_CONTROLLER_H_
On 2012/03/03 06:13:49, Nico wrote:
> Should you rename the file, too? (tools/git/mass-rename.sh if you haven't seen
> it)

Done.

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/web_intent_bubble_controller.mm (right):

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/web_intent_bubble_controller.mm:83: window.reset(
On 2012/03/03 06:13:49, Nico wrote:
> Use scoped_ptr's constructor to set the pointer instead of empty constructor +
> reset()

Done.

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/web_intent_bubble_controller.mm:99: // Also called when
user navigates to another page while the sheet is open.
On 2012/03/03 06:13:49, Nico wrote:
> No space in front of *

Done.

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/web_intent_bubble_controller.mm:288: windowFrame=[[self
window] frameRectForContentRect:windowFrame];
On 2012/03/03 06:13:49, Nico wrote:
> indent looks off

Done.

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/web_intent_bubble_controller_unittest.mm (right):

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/web_intent_bubble_controller_unittest.mm:63: Browser*
browser=(Browser*)0xdeadbeef;
On 2012/03/03 06:13:49, Nico wrote:
> Huh? If the browser object isn't needed, maybe WIPC shouldn't take it in its
> constructor?

It's needed _only_ if you invoke inline disposition, which none of the tests
here do. The problem is that at the point of that invokation, we don't have easy
access to the browser object.

I've allowed a NULL browser and added a DCHECK for inline disposition for now.

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/web_intent_bubble_controller_unittest.mm:65: &delegate_,
&model_));
On 2012/03/03 06:13:49, Nico wrote:
> indent is off

Done.

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm (right):

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm:31: // themselves, and the
WebIntentPicker needs to actually live longer than the
On 2012/03/03 06:13:49, Nico wrote:
> s/actually//

Done.

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm:32: // constrained window, we
need this forwarding class
On 2012/03/03 06:13:49, Nico wrote:
> .

Done.

https://chromiumcodereview.appspot.com/9581041/diff/1/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm:37:
WebIntentBubbleController* sheet_controller);
On 2012/03/03 06:13:49, Nico wrote:
> indent 1 more

Done.

[Nico, 2012-03-06 22:56:26]

LGTM

Sorry for the delay :-/

http://codereview.chromium.org/9581041/diff/4001/chrome/browser/ui/cocoa/web_...
File chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm (right):

http://codereview.chromium.org/9581041/diff/4001/chrome/browser/ui/cocoa/web_...
chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm:56:
set_sheet([sheet_controller window]);
init() already sets the sheet, so this line shouldn't be necessary.

[groby-ooo-7-16, 2012-03-07 00:50:42]

https://chromiumcodereview.appspot.com/9581041/diff/4001/chrome/browser/ui/co...
File chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm (right):

https://chromiumcodereview.appspot.com/9581041/diff/4001/chrome/browser/ui/co...
chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm:56:
set_sheet([sheet_controller window]);
On 2012/03/06 22:56:26, Nico wrote:
> init() already sets the sheet, so this line shouldn't be necessary.

Done.

[commit-bot: I haz the power, 2012-03-07 00:51:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/groby@chromium.org/9581041/7003

[commit-bot: I haz the power, 2012-03-07 02:59:30]

Change committed as 125305