Skip to content

Commit

Permalink
Revert "mac: Allow Chrome to hand off its active URL to other devices."
Browse files Browse the repository at this point in the history
This breaks Mac Memory tests. They're still building 32-bit, and this CL used
instance variables in a class extension, which is only supported in 64-bit
builds.

> This CL adds the class HandoffManager, which is responsible for interfacing
> with Apple's Handoff APIs. It takes a GURL, and exposes that GURL to Handoff.
>
> This CL adds the class ActiveWebContentsObserver, which is responsible for
> listening to changes to the active browser, the active tab, and the visible
> URL. It notifies its delegate when any of this state might have changed.
>
> AppControllerMac is the delegate of ActiveWebContentsObserver, as well as the
> owner of the HandoffManager. When it receives a delegate callback, it passes an
> updated GURL to the HandoffManager. There is some minimal logic in
> AppControllerMac that prevents URLs from incognito windows from being passed to
> the HandoffManager.
>
> BUG=431051, 438823
> Committed: https://crrev.com/708abc5b0abb5e0916d779bf6d1342fd472a2aa1
> Cr-Commit-Position: refs/heads/master@{#307846}

TBR=avi@chromium.org
NOTRY=true
BUG=431051, 438823

Review URL: https://codereview.chromium.org/799583002

Cr-Commit-Position: refs/heads/master@{#307947}
  • Loading branch information
erikchen authored and Commit bot committed Dec 11, 2014
1 parent 92a506d commit c429dd6
Show file tree
Hide file tree
Showing 13 changed files with 2 additions and 642 deletions.
4 changes: 0 additions & 4 deletions base/mac/sdk_forward_declarations.h
Original file line number Diff line number Diff line change
Expand Up @@ -334,10 +334,6 @@ BASE_EXPORT extern "C" NSString* const kCWSSIDDidChangeNotification;
@property (copy) NSDictionary* userInfo;
@property (copy) NSURL* webpageURL;

- (instancetype)initWithActivityType:(NSString*)activityType;
- (void)becomeCurrent;
- (void)invalidate;

@end

BASE_EXPORT extern "C" NSString* const NSUserActivityTypeBrowsingWeb;
Expand Down
4 changes: 0 additions & 4 deletions chrome/browser/app_controller_mac.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ class AppControllerProfileObserver;
class BookmarkMenuBridge;
class CommandUpdater;
class GURL;
@class HandoffManager;
class HistoryMenuBridge;
class Profile;
@class ProfileMenuController;
Expand Down Expand Up @@ -98,9 +97,6 @@ class WorkAreaWatcherObserver;

// Displays a notification when quitting while apps are running.
scoped_refptr<QuitWithAppsController> quitWithAppsController_;

// Responsible for maintaining all state related to the Handoff feature.
base::scoped_nsobject<HandoffManager> handoffManager_;
}

@property(readonly, nonatomic) BOOL startupComplete;
Expand Down
123 changes: 2 additions & 121 deletions chrome/browser/app_controller_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@
#import "chrome/browser/ui/cocoa/confirm_quit.h"
#import "chrome/browser/ui/cocoa/confirm_quit_panel_controller.h"
#import "chrome/browser/ui/cocoa/encoding_menu_controller_delegate_mac.h"
#include "chrome/browser/ui/cocoa/handoff_active_url_observer.h"
#include "chrome/browser/ui/cocoa/handoff_active_url_observer_delegate.h"
#import "chrome/browser/ui/cocoa/history_menu_bridge.h"
#include "chrome/browser/ui/cocoa/last_active_browser_cocoa.h"
#import "chrome/browser/ui/cocoa/profiles/profile_menu_controller.h"
Expand All @@ -85,7 +83,6 @@
#include "chrome/common/url_constants.h"
#include "chrome/grit/chromium_strings.h"
#include "chrome/grit/generated_resources.h"
#include "components/handoff/handoff_manager.h"
#include "components/handoff/handoff_utility.h"
#include "components/signin/core/browser/signin_manager.h"
#include "components/signin/core/common/profile_management_switches.h"
Expand Down Expand Up @@ -212,50 +209,9 @@ bool IsProfileSignedOut(Profile* profile) {
return cache.ProfileIsSigninRequiredAtIndex(profile_index);
}

} // namespace

// A protocol that allows ObjC objects to receive delegate callbacks from
// HandoffActiveURLObserver.
@protocol HandoffActiveURLObserverBridgeDelegate
- (void)handoffActiveURLChanged:(content::WebContents*)webContents;
@end

namespace {

// This class allows an ObjC object to receive the delegate callbacks from an
// HandoffActiveURLObserver.
class HandoffActiveURLObserverBridge : public HandoffActiveURLObserverDelegate {
public:
HandoffActiveURLObserverBridge(
NSObject<HandoffActiveURLObserverBridgeDelegate>* delegate)
: delegate_(delegate) {
DCHECK(delegate_);
observer_.reset(new HandoffActiveURLObserver(this));
}

~HandoffActiveURLObserverBridge() override{};

private:
void HandoffActiveURLChanged(content::WebContents* web_contents) override {
[delegate_ handoffActiveURLChanged:web_contents];
}

// Instances of this class should be owned by their |delegate_|.
NSObject<HandoffActiveURLObserverBridgeDelegate>* delegate_;

// The C++ object that this class acts as a bridge for.
scoped_ptr<HandoffActiveURLObserver> observer_;

DISALLOW_COPY_AND_ASSIGN(HandoffActiveURLObserverBridge);
};

} // namespace

@interface AppController () <HandoffActiveURLObserverBridgeDelegate> {
scoped_ptr<HandoffActiveURLObserverBridge>
active_web_contents_observer_bridge_;
}
} // anonymous namespace

@interface AppController (Private)
- (void)initMenuState;
- (void)initProfileMenu;
- (void)updateConfirmToQuitPrefMenuItem:(NSMenuItem*)item;
Expand Down Expand Up @@ -284,25 +240,6 @@ - (void)openStartupUrls;
// this method is called, and that tab is the NTP, then this method closes the
// NTP after all the |urls| have been opened.
- (void)openUrlsReplacingNTP:(const std::vector<GURL>&)urls;

// Whether instances of this class should use the Handoff feature.
- (BOOL)shouldUseHandoff;

// This method passes |handoffURL| to |handoffManager_|.
- (void)passURLToHandoffManager:(const GURL&)handoffURL;

// Lazily creates the Handoff Manager. Updates the state of the Handoff
// Manager. This method is idempotent. This should be called:
// - During initialization.
// - When the current tab navigates to a new URL.
// - When the active browser changes.
// - When the active browser's active tab switches.
// |webContents| should be the new, active WebContents.
- (void)updateHandoffManager:(content::WebContents*)webContents;

// Given |webContents|, extracts a GURL to be used for Handoff. This may return
// the empty GURL.
- (GURL)handoffURLFromWebContents:(content::WebContents*)webContents;
@end

class AppControllerProfileObserver : public ProfileInfoCacheObserver {
Expand Down Expand Up @@ -839,12 +776,6 @@ - (void)applicationDidFinishLaunching:(NSNotification*)notify {

startupComplete_ = YES;

Browser* browser =
FindLastActiveWithHostDesktopType(chrome::HOST_DESKTOP_TYPE_NATIVE);
content::WebContents* activeWebContents = nullptr;
if (browser)
activeWebContents = browser->tab_strip_model()->GetActiveWebContents();
[self updateHandoffManager:activeWebContents];
[self openStartupUrls];

PrefService* localState = g_browser_process->local_state();
Expand All @@ -855,9 +786,6 @@ - (void)applicationDidFinishLaunching:(NSNotification*)notify {
base::Bind(&chrome::BrowserCommandController::UpdateOpenFileState,
menuState_.get()));
}

active_web_contents_observer_bridge_.reset(
new HandoffActiveURLObserverBridge(self));
}

// This is called after profiles have been loaded and preferences registered.
Expand Down Expand Up @@ -1710,53 +1638,6 @@ - (void)application:(NSApplication*)application
error:(NSError*)error {
}

#pragma mark - Handoff Manager

- (BOOL)shouldUseHandoff {
return base::mac::IsOSYosemiteOrLater();
}

- (void)passURLToHandoffManager:(const GURL&)handoffURL {
[handoffManager_ updateActiveURL:handoffURL];
}

- (void)updateHandoffManager:(content::WebContents*)webContents {
if (![self shouldUseHandoff])
return;

if (!handoffManager_)
handoffManager_.reset([[HandoffManager alloc] init]);

GURL handoffURL = [self handoffURLFromWebContents:webContents];
[self passURLToHandoffManager:handoffURL];
}

- (GURL)handoffURLFromWebContents:(content::WebContents*)webContents {
if (!webContents)
return GURL();

Profile* profile =
Profile::FromBrowserContext(webContents->GetBrowserContext());
if (!profile)
return GURL();

// Handoff is not allowed from an incognito profile. To err on the safe side,
// also disallow Handoff from a guest profile.
if (profile->GetProfileType() != Profile::REGULAR_PROFILE)
return GURL();

if (!webContents)
return GURL();

return webContents->GetVisibleURL();
}

#pragma mark - HandoffActiveURLObserverBridgeDelegate

- (void)handoffActiveURLChanged:(content::WebContents*)webContents {
[self updateHandoffManager:webContents];
}

@end // @implementation AppController

//---------------------------------------------------------------------------
Expand Down
143 changes: 0 additions & 143 deletions chrome/browser/app_controller_mac_browsertest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,9 @@
#include "chrome/common/pref_names.h"
#include "chrome/common/url_constants.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "components/bookmarks/test/bookmark_test_helpers.h"
#include "components/signin/core/common/profile_management_switches.h"
#include "content/public/browser/web_contents.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/test_navigation_observer.h"
#include "extensions/browser/app_window/app_window_registry.h"
#include "extensions/common/extension.h"
Expand Down Expand Up @@ -483,144 +481,3 @@ void SetUpCommandLine(CommandLine* command_line) override {
}

} // namespace

//--------------------------AppControllerHandoffBrowserTest---------------------

static GURL g_handoff_url;

@interface AppController (BrowserTest)
- (BOOL)new_shouldUseHandoff;
- (void)new_passURLToHandoffManager:(const GURL&)handoffURL;
@end

@implementation AppController (BrowserTest)
- (BOOL)new_shouldUseHandoff {
return YES;
}

- (void)new_passURLToHandoffManager:(const GURL&)handoffURL {
g_handoff_url = handoffURL;
}
@end

namespace {

class AppControllerHandoffBrowserTest : public InProcessBrowserTest {
protected:
AppControllerHandoffBrowserTest() {}

// Exchanges the implementations of the two selectors on the class
// AppController.
void ExchangeSelectors(SEL originalMethod, SEL newMethod) {
Class appControllerClass = NSClassFromString(@"AppController");

ASSERT_TRUE(appControllerClass != nil);

Method original =
class_getInstanceMethod(appControllerClass, originalMethod);
Method destination = class_getInstanceMethod(appControllerClass, newMethod);

ASSERT_TRUE(original != NULL);
ASSERT_TRUE(destination != NULL);

method_exchangeImplementations(original, destination);
}

// Swizzle Handoff related implementations.
void SetUpInProcessBrowserTestFixture() override {
// Handoff is only available on OSX 10.10+. This swizzle makes the logic
// run on all OSX versions.
SEL originalMethod = @selector(shouldUseHandoff);
SEL newMethod = @selector(new_shouldUseHandoff);
ExchangeSelectors(originalMethod, newMethod);

// This swizzle intercepts the URL that would be sent to the Handoff
// Manager, and instead puts it into a variable accessible to this test.
originalMethod = @selector(passURLToHandoffManager:);
newMethod = @selector(new_passURLToHandoffManager:);
ExchangeSelectors(originalMethod, newMethod);
}

// Closes the tab, and waits for the close to finish.
void CloseTab(Browser* browser, int index) {
content::WebContentsDestroyedWatcher destroyed_watcher(
browser->tab_strip_model()->GetWebContentsAt(index));
browser->tab_strip_model()->CloseWebContentsAt(
index, TabStripModel::CLOSE_CREATE_HISTORICAL_TAB);
destroyed_watcher.Wait();
}
};

// Tests that as a user switches between tabs, navigates within a tab, and
// switches between browser windows, the correct URL is being passed to the
// Handoff.
IN_PROC_BROWSER_TEST_F(AppControllerHandoffBrowserTest, TestHandoffURLs) {
ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
EXPECT_EQ(g_handoff_url, GURL(url::kAboutBlankURL));

// Test that navigating to a URL updates the handoff URL.
GURL test_url1 = embedded_test_server()->GetURL("/title1.html");
ui_test_utils::NavigateToURL(browser(), test_url1);
EXPECT_EQ(g_handoff_url, test_url1);

// Test that opening a new tab updates the handoff URL.
GURL test_url2 = embedded_test_server()->GetURL("/title2.html");
chrome::NavigateParams params(browser(), test_url2, ui::PAGE_TRANSITION_LINK);
params.disposition = NEW_FOREGROUND_TAB;
ui_test_utils::NavigateToURL(&params);
EXPECT_EQ(g_handoff_url, test_url2);

// Test that switching tabs updates the handoff URL.
browser()->tab_strip_model()->ActivateTabAt(0, true);
EXPECT_EQ(g_handoff_url, test_url1);

// Test that closing the current tab updates the handoff URL.
CloseTab(browser(), 0);
EXPECT_EQ(g_handoff_url, test_url2);

// Test that opening a new browser window updates the handoff URL.
GURL test_url3 = embedded_test_server()->GetURL("/title3.html");
ui_test_utils::NavigateToURLWithDisposition(
browser(), GURL(test_url3), NEW_WINDOW,
ui_test_utils::BROWSER_TEST_WAIT_FOR_BROWSER);
EXPECT_EQ(g_handoff_url, test_url3);

// Check that there are exactly 2 browsers.
BrowserList* active_browser_list =
BrowserList::GetInstance(chrome::GetActiveDesktop());
EXPECT_EQ(2u, active_browser_list->size());

// Close the one and only tab for the second browser window.
Browser* browser2 = active_browser_list->get(1);
CloseTab(browser2, 0);
base::RunLoop().RunUntilIdle();
EXPECT_EQ(g_handoff_url, test_url2);

// The URLs of incognito windows should not be passed to Handoff.
GURL test_url4 = embedded_test_server()->GetURL("/simple.html");
ui_test_utils::NavigateToURLWithDisposition(
browser(), GURL(test_url4), OFF_THE_RECORD,
ui_test_utils::BROWSER_TEST_WAIT_FOR_BROWSER);
EXPECT_EQ(g_handoff_url, GURL());

// Open a new tab in the incognito window.
EXPECT_EQ(2u, active_browser_list->size());
Browser* browser3 = active_browser_list->get(1);
ui_test_utils::NavigateToURLWithDisposition(
browser3, test_url4, NEW_FOREGROUND_TAB,
ui_test_utils::BROWSER_TEST_WAIT_FOR_TAB);
EXPECT_EQ(g_handoff_url, GURL());

// Navigate the current tab in the incognito window.
ui_test_utils::NavigateToURLWithDisposition(
browser3, test_url1, CURRENT_TAB,
ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
EXPECT_EQ(g_handoff_url, GURL());

// Activate the original browser window.
Browser* browser1 = active_browser_list->get(0);
browser1->window()->Show();
EXPECT_EQ(g_handoff_url, test_url2);
}

} // namespace
Loading

0 comments on commit c429dd6

Please sign in to comment.