Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(20)

Side by Side Diff: chrome/browser/ui/cocoa/toolbar/toolbar_controller_unittest.mm

Issue 1415743011: Call -browserWillBeDestroyed before releasing ToolbarController in unit tests. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@bookmark-apps-location-bar
Patch Set: Created 5 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #import <Cocoa/Cocoa.h> 5 #import <Cocoa/Cocoa.h>
6 6
7 #import "base/mac/scoped_nsobject.h" 7 #import "base/mac/scoped_nsobject.h"
8 #include "base/prefs/pref_service.h" 8 #include "base/prefs/pref_service.h"
9 #include "base/run_loop.h" 9 #include "base/run_loop.h"
10 #include "chrome/app/chrome_command_ids.h" 10 #include "chrome/app/chrome_command_ids.h"
(...skipping 80 matching lines...) Expand 10 before | Expand all | Expand 10 after
91 initWithCommands:browser()->command_controller()->command_updater() 91 initWithCommands:browser()->command_controller()->command_updater()
92 profile:profile() 92 profile:profile()
93 browser:browser() 93 browser:browser()
94 resizeDelegate:resizeDelegate_.get()]); 94 resizeDelegate:resizeDelegate_.get()]);
95 EXPECT_TRUE([bar_ view]); 95 EXPECT_TRUE([bar_ view]);
96 NSView* parent = [test_window() contentView]; 96 NSView* parent = [test_window() contentView];
97 [parent addSubview:[bar_ view]]; 97 [parent addSubview:[bar_ view]];
98 } 98 }
99 99
100 void TearDown() override { 100 void TearDown() override {
101 bar_.reset(); // browser() must outlive the ToolbarController. 101 // Releasing ToolbarController doesn't actually free it at this point, since
102 // the NSViewController retains a reference to it from the nib loading.
103 // As browser() is released in the superclass TearDown, call
104 // -[ToolbarController browserWillBeDestroyed] to prevent a use after free
105 // issue on the |browser_| pointer in LocationBarViewMac when
106 // ToolbarController is actually freed (some time after this method is run).
107 [bar_ browserWillBeDestroyed];
108 bar_.reset();
102 CocoaProfileTest::TearDown(); 109 CocoaProfileTest::TearDown();
103 } 110 }
104 111
105 // Make sure the enabled state of the view is the same as the corresponding 112 // Make sure the enabled state of the view is the same as the corresponding
106 // command in the updater. The views are in the declaration order of outlets. 113 // command in the updater. The views are in the declaration order of outlets.
107 void CompareState(CommandUpdater* updater, NSArray* views) { 114 void CompareState(CommandUpdater* updater, NSArray* views) {
108 EXPECT_EQ(updater->IsCommandEnabled(IDC_BACK), 115 EXPECT_EQ(updater->IsCommandEnabled(IDC_BACK),
109 [[views objectAtIndex:kBackIndex] isEnabled] ? true : false); 116 [[views objectAtIndex:kBackIndex] isEnabled] ? true : false);
110 EXPECT_EQ(updater->IsCommandEnabled(IDC_FORWARD), 117 EXPECT_EQ(updater->IsCommandEnabled(IDC_FORWARD),
111 [[views objectAtIndex:kForwardIndex] isEnabled] ? true : false); 118 [[views objectAtIndex:kForwardIndex] isEnabled] ? true : false);
(...skipping 227 matching lines...) Expand 10 before | Expand all | Expand 10 after
339 TEST_F(ToolbarControllerTest, TranslateBubblePoint) { 346 TEST_F(ToolbarControllerTest, TranslateBubblePoint) {
340 const NSPoint translatePoint = [bar_ translateBubblePoint]; 347 const NSPoint translatePoint = [bar_ translateBubblePoint];
341 const NSRect barFrame = 348 const NSRect barFrame =
342 [[bar_ view] convertRect:[[bar_ view] bounds] toView:nil]; 349 [[bar_ view] convertRect:[[bar_ view] bounds] toView:nil];
343 EXPECT_TRUE(NSPointInRect(translatePoint, barFrame)); 350 EXPECT_TRUE(NSPointInRect(translatePoint, barFrame));
344 } 351 }
345 352
346 TEST_F(ToolbarControllerTest, HoverButtonForEvent) { 353 TEST_F(ToolbarControllerTest, HoverButtonForEvent) {
347 base::scoped_nsobject<HitView> view( 354 base::scoped_nsobject<HitView> view(
348 [[HitView alloc] initWithFrame:NSMakeRect(0, 0, 100, 100)]); 355 [[HitView alloc] initWithFrame:NSMakeRect(0, 0, 100, 100)]);
356 NSView* toolbarView = [bar_ view];
349 [bar_ setView:view]; 357 [bar_ setView:view];
350 NSEvent* event = [NSEvent mouseEventWithType:NSMouseMoved 358 NSEvent* event = [NSEvent mouseEventWithType:NSMouseMoved
351 location:NSMakePoint(10,10) 359 location:NSMakePoint(10,10)
352 modifierFlags:0 360 modifierFlags:0
353 timestamp:0 361 timestamp:0
354 windowNumber:0 362 windowNumber:0
355 context:nil 363 context:nil
356 eventNumber:0 364 eventNumber:0
357 clickCount:0 365 clickCount:0
358 pressure:0.0]; 366 pressure:0.0];
359 367
360 // NOT a match. 368 // NOT a match.
361 [view setHitTestReturn:bar_.get()]; 369 [view setHitTestReturn:bar_.get()];
362 EXPECT_FALSE([bar_ hoverButtonForEvent:event]); 370 EXPECT_FALSE([bar_ hoverButtonForEvent:event]);
363 371
364 // Not yet... 372 // Not yet...
365 base::scoped_nsobject<NSButton> button([[NSButton alloc] init]); 373 base::scoped_nsobject<NSButton> button([[NSButton alloc] init]);
366 [view setHitTestReturn:button]; 374 [view setHitTestReturn:button];
367 EXPECT_FALSE([bar_ hoverButtonForEvent:event]); 375 EXPECT_FALSE([bar_ hoverButtonForEvent:event]);
368 376
369 // Now! 377 // Now!
370 base::scoped_nsobject<ImageButtonCell> cell( 378 base::scoped_nsobject<ImageButtonCell> cell(
371 [[ImageButtonCell alloc] init]); 379 [[ImageButtonCell alloc] init]);
372 [button setCell:cell.get()]; 380 [button setCell:cell.get()];
373 EXPECT_TRUE([bar_ hoverButtonForEvent:nil]); 381 EXPECT_TRUE([bar_ hoverButtonForEvent:nil]);
382
383 // Restore the original view so that
384 // -[ToolbarController browserWillBeDestroyed] will run correctly.
385 [bar_ setView:toolbarView];
374 } 386 }
375 387
376 class BrowserRemovedObserver : public chrome::BrowserListObserver { 388 class BrowserRemovedObserver : public chrome::BrowserListObserver {
377 public: 389 public:
378 BrowserRemovedObserver() { BrowserList::AddObserver(this); } 390 BrowserRemovedObserver() { BrowserList::AddObserver(this); }
379 ~BrowserRemovedObserver() override { BrowserList::RemoveObserver(this); } 391 ~BrowserRemovedObserver() override { BrowserList::RemoveObserver(this); }
380 void WaitUntilBrowserRemoved() { run_loop_.Run(); } 392 void WaitUntilBrowserRemoved() { run_loop_.Run(); }
381 void OnBrowserRemoved(Browser* browser) override { run_loop_.Quit(); } 393 void OnBrowserRemoved(Browser* browser) override { run_loop_.Quit(); }
382 394
383 private: 395 private:
384 base::RunLoop run_loop_; 396 base::RunLoop run_loop_;
385 397
386 DISALLOW_COPY_AND_ASSIGN(BrowserRemovedObserver); 398 DISALLOW_COPY_AND_ASSIGN(BrowserRemovedObserver);
387 }; 399 };
388 400
389 // Test that ToolbarController can be destroyed after the Browser.
390 // This can happen because the ToolbarController is retained by both the
391 // BrowserWindowController and -[ToolbarController view], the latter of which is
392 // autoreleased.
393 TEST_F(ToolbarControllerTest, ToolbarDestroyedAfterBrowser) {
394 BrowserRemovedObserver observer;
395 // This is normally called by BrowserWindowController, but since |bar_| is not
396 // owned by one, call it here.
397 [bar_ browserWillBeDestroyed];
398 CloseBrowserWindow();
399 observer.WaitUntilBrowserRemoved();
400 // |bar_| is released in TearDown().
401 }
402
403 } // namespace 401 } // namespace
OLDNEW
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698