diff options
13 files changed, 151 insertions, 122 deletions
diff --git a/chrome/browser/chrome_to_mobile_service.cc b/chrome/browser/chrome_to_mobile_service.cc index bbe3b93..e4b07f6 100644 --- a/chrome/browser/chrome_to_mobile_service.cc +++ b/chrome/browser/chrome_to_mobile_service.cc @@ -6,6 +6,7 @@ #include "base/bind.h" #include "base/command_line.h" +#include "base/file_util.h" #include "base/json/json_reader.h" #include "base/json/json_writer.h" #include "base/stringprintf.h" @@ -72,10 +73,6 @@ const char kRequestTypeURL[] = "url"; const char kRequestTypeDelayedSnapshot[] = "url_with_delayed_snapshot"; const char kRequestTypeSnapshot[] = "snapshot"; -// The snapshot path constants; used with a guid for each MHTML snapshot file. -const FilePath::CharType kSnapshotPath[] = - FILE_PATH_LITERAL("chrome_to_mobile_snapshot_.mht"); - // Get the "__c2dm__job_data" tag JSON data for the cloud print job submission. std::string GetJobString(const ChromeToMobileService::RequestData& data) { scoped_ptr<DictionaryValue> job(new DictionaryValue()); @@ -122,15 +119,23 @@ GURL GetSubmitURL(const GURL& service_url, return submit_url.ReplaceComponents(replacements); } -// Delete the specified file; called as a BlockingPoolTask. -void DeleteFilePath(const FilePath& file_path) { - bool success = file_util::Delete(file_path, false); - DCHECK(success); +// A callback to continue snapshot generation after creating the temp file. +typedef base::Callback<void(FilePath path, bool success)> + CreateSnapshotFileCallback; + +// Create a temp file and post the callback on the UI thread with the results. +// Call this as a BlockingPoolTask to avoid the FILE thread. +void CreateSnapshotFile(CreateSnapshotFileCallback callback) { + FilePath snapshot; + bool success = file_util::CreateTemporaryFile(&snapshot); + content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE, + base::Bind(callback, snapshot, success)); } -// Construct POST data and submit the MHTML snapshot file; deletes the snapshot. -void SubmitSnapshot(content::URLFetcher* request, - const ChromeToMobileService::RequestData& data) { +// Send snapshot file contents as POST data in a job submit request. +// Call this as a BlockingPoolSequencedTask (before posting DeleteSnapshotFile). +void SubmitSnapshotFile(content::URLFetcher* request, + const ChromeToMobileService::RequestData& data) { std::string file; if (file_util::ReadFileToString(data.snapshot_path, &file) && !file.empty()) { std::string post_data, mime_boundary; @@ -154,9 +159,13 @@ void SubmitSnapshot(content::URLFetcher* request, request->SetUploadData(content_type, post_data); request->Start(); } +} - content::BrowserThread::PostBlockingPoolTask(FROM_HERE, - base::Bind(&DeleteFilePath, data.snapshot_path)); +// Delete the snapshot file; DCHECK, but really ignore the result of the delete. +// Call this as a BlockingPoolSequencedTask [after posting SubmitSnapshotFile]. +void DeleteSnapshotFile(const FilePath snapshot) { + bool success = file_util::Delete(snapshot, false); + DCHECK(success); } } // namespace @@ -181,23 +190,12 @@ bool ChromeToMobileService::IsChromeToMobileEnabled() { } ChromeToMobileService::ChromeToMobileService(Profile* profile) - : profile_(profile), + : ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)), + profile_(profile), cloud_print_url_(new CloudPrintURL(profile)), - temp_dir_valid_(false), cloud_print_accessible_(false) { -} - -ChromeToMobileService::~ChromeToMobileService() {} - -void ChromeToMobileService::Init() { // Skip initialization if constructed without a profile. if (profile_) { - // Create a unique temporary directory for the page snapshots. - // Note that if this task is posted and run in the ctor, the service will be - // destroyed (since there are no other references to the service). - content::BrowserThread::PostBlockingPoolTask(FROM_HERE, - base::Bind(&ChromeToMobileService::CreateUniqueTempDir, this)); - // Get an access token as soon as the Gaia login refresh token is available. TokenService* service = TokenServiceFactory::GetForProfile(profile_); registrar_.Add(this, chrome::NOTIFICATION_TOKEN_AVAILABLE, @@ -207,6 +205,11 @@ void ChromeToMobileService::Init() { } } +ChromeToMobileService::~ChromeToMobileService() { + while (!snapshots_.empty()) + DeleteSnapshot(*snapshots_.begin()); +} + bool ChromeToMobileService::HasDevices() { return !mobiles().empty(); } @@ -223,19 +226,13 @@ void ChromeToMobileService::RequestMobileListUpdate() { } void ChromeToMobileService::GenerateSnapshot(base::WeakPtr<Observer> observer) { - // Signal snapshot generation failure and bail if the temp dir is invalid. - DCHECK(temp_dir_valid_); - if (!temp_dir_valid_) { - if (observer.get()) - observer->SnapshotGenerated(FilePath(), 0); - return; - } - - // Generate the snapshot and have the observer be called back on completion. - FilePath path(temp_dir_.path().Append(kSnapshotPath)); - BrowserList::GetLastActiveWithProfile(profile_)->GetSelectedWebContents()-> - GenerateMHTML(path.InsertBeforeExtensionASCII(guid::GenerateGUID()), - base::Bind(&Observer::SnapshotGenerated, observer)); + // Callback SnapshotFileCreated from CreateSnapshotFile to continue. + CreateSnapshotFileCallback callback = + base::Bind(&ChromeToMobileService::SnapshotFileCreated, + weak_ptr_factory_.GetWeakPtr(), observer); + // Create a temporary file via the blocking pool for snapshot storage. + content::BrowserThread::PostBlockingPoolTask(FROM_HERE, + base::Bind(&CreateSnapshotFile, callback)); } void ChromeToMobileService::SendToMobile(const string16& mobile_id, @@ -261,12 +258,21 @@ void ChromeToMobileService::SendToMobile(const string16& mobile_id, data.type = SNAPSHOT; content::URLFetcher* submit_snapshot = CreateRequest(data); request_observer_map_[submit_snapshot] = observer; - content::BrowserThread::PostBlockingPoolTask(FROM_HERE, - base::Bind(&SubmitSnapshot, submit_snapshot, data)); + content::BrowserThread::PostBlockingPoolSequencedTask( + data.snapshot_path.AsUTF8Unsafe(), FROM_HERE, + base::Bind(&SubmitSnapshotFile, submit_snapshot, data)); } } -void ChromeToMobileService::ShutdownOnUIThread() { +void ChromeToMobileService::DeleteSnapshot(const FilePath& snapshot) { + DCHECK(snapshot.empty() || snapshots_.find(snapshot) != snapshots_.end()); + if (snapshots_.find(snapshot) != snapshots_.end()) { + if (!snapshot.empty()) + content::BrowserThread::PostBlockingPoolSequencedTask( + snapshot.AsUTF8Unsafe(), FROM_HERE, + base::Bind(&DeleteSnapshotFile, snapshot)); + snapshots_.erase(snapshot); + } } void ChromeToMobileService::OnURLFetchComplete( @@ -283,7 +289,7 @@ void ChromeToMobileService::Observe( int type, const content::NotificationSource& source, const content::NotificationDetails& details) { - DCHECK(type == chrome::NOTIFICATION_TOKEN_AVAILABLE); + DCHECK_EQ(type, chrome::NOTIFICATION_TOKEN_AVAILABLE); TokenService::TokenAvailableDetails* token_details = content::Details<TokenService::TokenAvailableDetails>(details).ptr(); if (token_details->service() == GaiaConstants::kGaiaOAuth2LoginRefreshToken) @@ -308,10 +314,23 @@ void ChromeToMobileService::OnGetTokenFailure( this, &ChromeToMobileService::RefreshAccessToken); } -void ChromeToMobileService::CreateUniqueTempDir() { - temp_dir_valid_ = temp_dir_.CreateUniqueTempDir(); - DCHECK_EQ(temp_dir_valid_, temp_dir_.IsValid()); - DCHECK(temp_dir_valid_); +void ChromeToMobileService::SnapshotFileCreated( + base::WeakPtr<Observer> observer, + const FilePath path, + bool success) { + DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); + // Track the set of temporary files to be deleted later. + snapshots_.insert(path); + + Browser* browser = BrowserList::GetLastActiveWithProfile(profile_); + if (success && browser && browser->GetSelectedWebContents()) { + // Generate the snapshot and have the observer be called back on completion. + browser->GetSelectedWebContents()->GenerateMHTML(path, + base::Bind(&Observer::SnapshotGenerated, observer)); + } else if (observer.get()) { + // Signal snapshot generation failure. + observer->SnapshotGenerated(FilePath(), 0); + } } content::URLFetcher* ChromeToMobileService::CreateRequest( diff --git a/chrome/browser/chrome_to_mobile_service.h b/chrome/browser/chrome_to_mobile_service.h index e46847b..833282a 100644 --- a/chrome/browser/chrome_to_mobile_service.h +++ b/chrome/browser/chrome_to_mobile_service.h @@ -7,16 +7,16 @@ #pragma once #include <map> +#include <set> #include <string> #include <vector> -#include "base/file_util.h" +#include "base/file_path.h" #include "base/memory/scoped_vector.h" #include "base/memory/weak_ptr.h" -#include "base/scoped_temp_dir.h" #include "base/string16.h" #include "base/timer.h" -#include "chrome/browser/profiles/refcounted_profile_keyed_service.h" +#include "chrome/browser/profiles/profile_keyed_service.h" #include "chrome/common/net/gaia/oauth2_access_token_consumer.h" #include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_registrar.h" @@ -34,7 +34,7 @@ class DictionaryValue; // ChromeToMobileService connects to the cloud print service to enumerate // compatible mobiles owned by its profile and send URLs and MHTML snapshots. -class ChromeToMobileService : public RefcountedProfileKeyedService, +class ChromeToMobileService : public ProfileKeyedService, public content::URLFetcherDelegate, public content::NotificationObserver, public OAuth2AccessTokenConsumer { @@ -78,9 +78,6 @@ class ChromeToMobileService : public RefcountedProfileKeyedService, explicit ChromeToMobileService(Profile* profile); virtual ~ChromeToMobileService(); - // Initialize the service; performed on construction by the factory. - void Init(); - // Returns true if the service has found any registered mobile devices. bool HasDevices(); @@ -101,8 +98,9 @@ class ChromeToMobileService : public RefcountedProfileKeyedService, const FilePath& snapshot, base::WeakPtr<Observer> observer); - // RefcountedProfileKeyedService method. - virtual void ShutdownOnUIThread() OVERRIDE; + // Delete the snapshot file (should be called on observer destruction). + // Virtual for unit test mocking. + virtual void DeleteSnapshot(const FilePath& snapshot); // content::URLFetcherDelegate method. virtual void OnURLFetchComplete(const content::URLFetcher* source) OVERRIDE; @@ -119,8 +117,11 @@ class ChromeToMobileService : public RefcountedProfileKeyedService, private: friend class MockChromeToMobileService; - // Utility function to initialize the ScopedTempDir. - void CreateUniqueTempDir(); + // Handle the attempted creation of a temporary file for snapshot generation. + // Alert the observer of failure or generate MHTML with an observer callback. + void SnapshotFileCreated(base::WeakPtr<Observer> observer, + const FilePath path, + bool success); // Utility function to create URLFetcher requests. content::URLFetcher* CreateRequest(const RequestData& data); @@ -139,6 +140,8 @@ class ChromeToMobileService : public RefcountedProfileKeyedService, void HandleSearchResponse(); void HandleSubmitResponse(const content::URLFetcher* source); + base::WeakPtrFactory<ChromeToMobileService> weak_ptr_factory_; + Profile* profile_; // Used to recieve TokenService notifications for GaiaOAuth2LoginRefreshToken. @@ -151,9 +154,8 @@ class ChromeToMobileService : public RefcountedProfileKeyedService, // The list of mobile devices retrieved from the cloud print service. ScopedVector<base::DictionaryValue> mobiles_; - // The temporary directory for MHTML snapshot files and its validity flag. - ScopedTempDir temp_dir_; - bool temp_dir_valid_; + // The set of snapshots currently available. + std::set<FilePath> snapshots_; // Map URLFetchers to observers for reporting OnSendComplete. typedef std::map<const content::URLFetcher*, base::WeakPtr<Observer> > diff --git a/chrome/browser/chrome_to_mobile_service_factory.cc b/chrome/browser/chrome_to_mobile_service_factory.cc index 9cd082e..e669003 100644 --- a/chrome/browser/chrome_to_mobile_service_factory.cc +++ b/chrome/browser/chrome_to_mobile_service_factory.cc @@ -15,28 +15,24 @@ ChromeToMobileServiceFactory* ChromeToMobileServiceFactory::GetInstance() { } // static -scoped_refptr<ChromeToMobileService> -ChromeToMobileServiceFactory::GetForProfile(Profile* profile) { +ChromeToMobileService* ChromeToMobileServiceFactory::GetForProfile( + Profile* profile) { return static_cast<ChromeToMobileService*>( - GetInstance()->GetServiceForProfile(profile, true).get()); + GetInstance()->GetServiceForProfile(profile, true)); } -scoped_refptr<RefcountedProfileKeyedService> -ChromeToMobileServiceFactory::BuildServiceInstanceFor(Profile* profile) const { +ProfileKeyedService* ChromeToMobileServiceFactory::BuildServiceInstanceFor( + Profile* profile) const { // Ensure that the service is not instantiated or used if it is disabled. if (!ChromeToMobileService::IsChromeToMobileEnabled()) return NULL; - scoped_refptr<ChromeToMobileService> service = - new ChromeToMobileService(profile); - service->Init(); - return service; + return new ChromeToMobileService(profile); } ChromeToMobileServiceFactory::ChromeToMobileServiceFactory() - : RefcountedProfileKeyedServiceFactory( - "ChromeToMobileService", - ProfileDependencyManager::GetInstance()) { + : ProfileKeyedServiceFactory("ChromeToMobileService", + ProfileDependencyManager::GetInstance()) { DependsOn(TokenServiceFactory::GetInstance()); DependsOn(CookieSettings::Factory::GetInstance()); } diff --git a/chrome/browser/chrome_to_mobile_service_factory.h b/chrome/browser/chrome_to_mobile_service_factory.h index a3c8965..fa8f200 100644 --- a/chrome/browser/chrome_to_mobile_service_factory.h +++ b/chrome/browser/chrome_to_mobile_service_factory.h @@ -7,22 +7,21 @@ #pragma once #include "base/memory/singleton.h" -#include "chrome/browser/profiles/refcounted_profile_keyed_service_factory.h" +#include "chrome/browser/profiles/profile_keyed_service_factory.h" class ChromeToMobileService; -class ChromeToMobileServiceFactory - : public RefcountedProfileKeyedServiceFactory { +class ChromeToMobileServiceFactory : public ProfileKeyedServiceFactory { public: // Get the singleton ChromeToMobileServiceFactory instance. static ChromeToMobileServiceFactory* GetInstance(); // Get |profile|'s ChromeToMobileService, creating one if needed. - static scoped_refptr<ChromeToMobileService> GetForProfile(Profile* profile); + static ChromeToMobileService* GetForProfile(Profile* profile); protected: - // RefcountedProfileKeyedServiceFactory overrides: - virtual scoped_refptr<RefcountedProfileKeyedService> BuildServiceInstanceFor( + // ProfileKeyedServiceFactory overrides: + virtual ProfileKeyedService* BuildServiceInstanceFor( Profile* profile) const OVERRIDE; private: diff --git a/chrome/browser/chrome_to_mobile_service_unittest.cc b/chrome/browser/chrome_to_mobile_service_unittest.cc index a04937b..38c1058 100644 --- a/chrome/browser/chrome_to_mobile_service_unittest.cc +++ b/chrome/browser/chrome_to_mobile_service_unittest.cc @@ -39,12 +39,8 @@ class ChromeToMobileServiceTest : public testing::Test { ChromeToMobileServiceTest(); virtual ~ChromeToMobileServiceTest(); - virtual void SetUp() OVERRIDE { - service_ = new MockChromeToMobileService(); - } - protected: - scoped_refptr<MockChromeToMobileService> service_; + MockChromeToMobileService service_; private: DISALLOW_COPY_AND_ASSIGN(ChromeToMobileServiceTest); @@ -56,25 +52,25 @@ ChromeToMobileServiceTest::~ChromeToMobileServiceTest() {} // Ensure that RefreshAccessToken is not called for irrelevant notifications. TEST_F(ChromeToMobileServiceTest, IgnoreIrrelevantNotifications) { - EXPECT_CALL(*service_.get(), RefreshAccessToken()).Times(0); + EXPECT_CALL(service_, RefreshAccessToken()).Times(0); // Send dummy service/token details (should not refresh token). DummyNotificationSource dummy_source; TokenService::TokenAvailableDetails dummy_details(kDummyString, kDummyString); - service_->Observe(chrome::NOTIFICATION_TOKEN_AVAILABLE, + service_.Observe(chrome::NOTIFICATION_TOKEN_AVAILABLE, content::Source<DummyNotificationSource>(&dummy_source), content::Details<TokenService::TokenAvailableDetails>(&dummy_details)); } // Ensure that RefreshAccessToken is called on the proper notification. TEST_F(ChromeToMobileServiceTest, AuthenticateOnTokenAvailable) { - EXPECT_CALL(*service_.get(), RefreshAccessToken()).Times(1); + EXPECT_CALL(service_, RefreshAccessToken()).Times(1); // Send a Gaia OAuth2 Login service dummy token (should refresh token). DummyNotificationSource dummy_source; TokenService::TokenAvailableDetails login_details( GaiaConstants::kGaiaOAuth2LoginRefreshToken, kDummyString); - service_->Observe(chrome::NOTIFICATION_TOKEN_AVAILABLE, + service_.Observe(chrome::NOTIFICATION_TOKEN_AVAILABLE, content::Source<DummyNotificationSource>(&dummy_source), content::Details<TokenService::TokenAvailableDetails>(&login_details)); } diff --git a/chrome/browser/ui/cocoa/chrome_to_mobile_bubble_controller.h b/chrome/browser/ui/cocoa/chrome_to_mobile_bubble_controller.h index a8956c0..4ea73abd7 100644 --- a/chrome/browser/ui/cocoa/chrome_to_mobile_bubble_controller.h +++ b/chrome/browser/ui/cocoa/chrome_to_mobile_bubble_controller.h @@ -89,7 +89,7 @@ class ChromeToMobileBubbleNotificationBridge scoped_ptr<ChromeToMobileBubbleNotificationBridge> bridge_; // The Chrome To Mobile service associated with this bubble. - scoped_refptr<ChromeToMobileService> service_; + ChromeToMobileService* service_; // The file path for the MHTML page snapshot. FilePath snapshotPath_; @@ -122,7 +122,7 @@ class ChromeToMobileBubbleNotificationBridge @interface ChromeToMobileBubbleController (JustForTesting) - (id)initWithParentWindow:(NSWindow*)parentWindow - service:(scoped_refptr<ChromeToMobileService>)service; + service:(ChromeToMobileService*)service; - (void)setSendCopy:(bool)sendCopy; - (ChromeToMobileBubbleNotificationBridge*)bridge; diff --git a/chrome/browser/ui/cocoa/chrome_to_mobile_bubble_controller.mm b/chrome/browser/ui/cocoa/chrome_to_mobile_bubble_controller.mm index 4bce3eb..8ac3fb6 100644 --- a/chrome/browser/ui/cocoa/chrome_to_mobile_bubble_controller.mm +++ b/chrome/browser/ui/cocoa/chrome_to_mobile_bubble_controller.mm @@ -82,6 +82,9 @@ void ChromeToMobileBubbleNotificationBridge::OnSendComplete(bool success) { } - (void)windowWillClose:(NSNotification*)notification { + // Instruct the service to delete the snapshot file. + service_->DeleteSnapshot(snapshotPath_); + // We caught a close so we don't need to observe further notifications. bridge_.reset(NULL); [progressAnimation_ stopAnimation]; @@ -179,9 +182,9 @@ void ChromeToMobileBubbleNotificationBridge::OnSendComplete(bool success) { - (void)snapshotGenerated:(const FilePath&)path bytes:(int64)bytes { + snapshotPath_ = path; NSString* text = nil; if (bytes > 0) { - snapshotPath_ = path; [sendCopy_ setEnabled:YES]; text = l10n_util::GetNSStringF(IDS_CHROME_TO_MOBILE_BUBBLE_SEND_COPY, ui::FormatBytes(bytes)); @@ -243,7 +246,7 @@ void ChromeToMobileBubbleNotificationBridge::OnSendComplete(bool success) { @implementation ChromeToMobileBubbleController (JustForTesting) - (id)initWithParentWindow:(NSWindow*)parentWindow - service:(scoped_refptr<ChromeToMobileService>)service { + service:(ChromeToMobileService*)service { self = [super initWithWindowNibPath:@"ChromeToMobileBubble" parentWindow:parentWindow anchoredAt:NSZeroPoint]; diff --git a/chrome/browser/ui/cocoa/chrome_to_mobile_bubble_controller_unittest.mm b/chrome/browser/ui/cocoa/chrome_to_mobile_bubble_controller_unittest.mm index af14292..7eb1e97 100644 --- a/chrome/browser/ui/cocoa/chrome_to_mobile_bubble_controller_unittest.mm +++ b/chrome/browser/ui/cocoa/chrome_to_mobile_bubble_controller_unittest.mm @@ -21,6 +21,7 @@ class MockChromeToMobileService : public ChromeToMobileService { MOCK_METHOD3(SendToMobile, void(const string16& mobile_id, const FilePath& snapshot, base::WeakPtr<Observer> observer)); + MOCK_METHOD1(DeleteSnapshot, void(const FilePath& snapshot)); }; void MockChromeToMobileService::AddDevices(size_t count) { @@ -35,10 +36,8 @@ namespace { class ChromeToMobileBubbleControllerTest : public CocoaTest { public: - ChromeToMobileBubbleControllerTest() - : controller_(NULL), - service_(new MockChromeToMobileService()) { - } + ChromeToMobileBubbleControllerTest() : controller_(NULL) {} + virtual ~ChromeToMobileBubbleControllerTest() {} virtual void TearDown() { [controller_ close]; @@ -48,7 +47,7 @@ class ChromeToMobileBubbleControllerTest : public CocoaTest { void CreateBubble() { // The controller cleans up after itself when the window closes. controller_ = [[ChromeToMobileBubbleController alloc] - initWithParentWindow:test_window() service:service_]; + initWithParentWindow:test_window() service:&service_]; window_ = [controller_ window]; [controller_ showWindow:nil]; } @@ -92,58 +91,66 @@ class ChromeToMobileBubbleControllerTest : public CocoaTest { EXPECT_EQ([window_ delegate], controller_); } + protected: + MockChromeToMobileService service_; ChromeToMobileBubbleController* controller_; // Weak, owns self. + + private: NSWindow* window_; // Weak, owned by controller. - scoped_refptr<MockChromeToMobileService> service_; + + DISALLOW_COPY_AND_ASSIGN(ChromeToMobileBubbleControllerTest); }; TEST_F(ChromeToMobileBubbleControllerTest, OneDevice) { - EXPECT_CALL(*service_.get(), RequestMobileListUpdate()).Times(1); - EXPECT_CALL(*service_.get(), GenerateSnapshot(testing::_)).Times(1); + EXPECT_CALL(service_, RequestMobileListUpdate()).Times(1); + EXPECT_CALL(service_, GenerateSnapshot(testing::_)).Times(1); + EXPECT_CALL(service_, DeleteSnapshot(testing::_)).Times(1); - service_->AddDevices(1); + service_.AddDevices(1); CreateBubble(); CheckWindow(/*radio_buttons=*/0); } TEST_F(ChromeToMobileBubbleControllerTest, TwoDevices) { - EXPECT_CALL(*service_.get(), RequestMobileListUpdate()).Times(1); - EXPECT_CALL(*service_.get(), GenerateSnapshot(testing::_)).Times(1); + EXPECT_CALL(service_, RequestMobileListUpdate()).Times(1); + EXPECT_CALL(service_, GenerateSnapshot(testing::_)).Times(1); + EXPECT_CALL(service_, DeleteSnapshot(testing::_)).Times(1); - service_->AddDevices(2); + service_.AddDevices(2); CreateBubble(); CheckWindow(/*radio_buttons=*/2); } TEST_F(ChromeToMobileBubbleControllerTest, ThreeDevices) { - EXPECT_CALL(*service_.get(), RequestMobileListUpdate()).Times(1); - EXPECT_CALL(*service_.get(), GenerateSnapshot(testing::_)).Times(1); + EXPECT_CALL(service_, RequestMobileListUpdate()).Times(1); + EXPECT_CALL(service_, GenerateSnapshot(testing::_)).Times(1); + EXPECT_CALL(service_, DeleteSnapshot(testing::_)).Times(1); - service_->AddDevices(3); + service_.AddDevices(3); CreateBubble(); CheckWindow(/*radio_buttons=*/3); } TEST_F(ChromeToMobileBubbleControllerTest, SendWithoutSnapshot) { FilePath path; - EXPECT_CALL(*service_.get(), RequestMobileListUpdate()).Times(1); - EXPECT_CALL(*service_.get(), GenerateSnapshot(testing::_)).Times(1); - EXPECT_CALL(*service_.get(), - SendToMobile(testing::_, path, testing::_)).Times(1); + EXPECT_CALL(service_, RequestMobileListUpdate()).Times(1); + EXPECT_CALL(service_, GenerateSnapshot(testing::_)).Times(1); + EXPECT_CALL(service_, SendToMobile(testing::_, path, testing::_)).Times(1); + EXPECT_CALL(service_, DeleteSnapshot(testing::_)).Times(1); - service_->AddDevices(1); + service_.AddDevices(1); CreateBubble(); [controller_ send:nil]; } TEST_F(ChromeToMobileBubbleControllerTest, SendWithSnapshot) { FilePath path("path.mht"); - EXPECT_CALL(*service_.get(), RequestMobileListUpdate()).Times(1); - EXPECT_CALL(*service_.get(), GenerateSnapshot(testing::_)).Times(1); - EXPECT_CALL(*service_.get(), - SendToMobile(testing::_, path, testing::_)).Times(1); + EXPECT_CALL(service_, RequestMobileListUpdate()).Times(1); + EXPECT_CALL(service_, GenerateSnapshot(testing::_)).Times(1); + EXPECT_CALL(service_, SendToMobile(testing::_, path, testing::_)).Times(1); + EXPECT_CALL(service_, DeleteSnapshot(testing::_)).Times(1); - service_->AddDevices(1); + service_.AddDevices(1); CreateBubble(); ChromeToMobileBubbleNotificationBridge* bridge = [controller_ bridge]; bridge->SnapshotGenerated(path, 1); diff --git a/chrome/browser/ui/gtk/chrome_to_mobile_bubble_gtk.cc b/chrome/browser/ui/gtk/chrome_to_mobile_bubble_gtk.cc index 9e00dec..f4bc23f 100644 --- a/chrome/browser/ui/gtk/chrome_to_mobile_bubble_gtk.cc +++ b/chrome/browser/ui/gtk/chrome_to_mobile_bubble_gtk.cc @@ -63,6 +63,10 @@ void ChromeToMobileBubbleGtk::BubbleClosing(BubbleGtk* bubble, bool closed_by_escape) { DCHECK_EQ(bubble, bubble_); + // Instruct the service to delete the snapshot file. + service_->DeleteSnapshot(snapshot_path_); + + // Restore the resting state mobile device icon. gtk_image_set_from_pixbuf(GTK_IMAGE(anchor_image_), theme_service_->GetImageNamed(IDR_MOBILE)->ToGdkPixbuf()); @@ -107,8 +111,8 @@ void ChromeToMobileBubbleGtk::Observe( void ChromeToMobileBubbleGtk::SnapshotGenerated(const FilePath& path, int64 bytes) { + snapshot_path_ = path; if (bytes > 0) { - snapshot_path_ = path; gtk_button_set_label(GTK_BUTTON(send_copy_), l10n_util::GetStringFUTF8( IDS_CHROME_TO_MOBILE_BUBBLE_SEND_COPY, ui::FormatBytes(bytes)).c_str()); gtk_widget_set_sensitive(send_copy_, TRUE); diff --git a/chrome/browser/ui/gtk/chrome_to_mobile_bubble_gtk.h b/chrome/browser/ui/gtk/chrome_to_mobile_bubble_gtk.h index 8095774..d426919 100644 --- a/chrome/browser/ui/gtk/chrome_to_mobile_bubble_gtk.h +++ b/chrome/browser/ui/gtk/chrome_to_mobile_bubble_gtk.h @@ -76,7 +76,7 @@ class ChromeToMobileBubbleGtk : public BubbleDelegateGtk, base::WeakPtrFactory<ChromeToMobileBubbleGtk> weak_ptr_factory_; // The Chrome To Mobile service associated with this bubble. - scoped_refptr<ChromeToMobileService> service_; + ChromeToMobileService* service_; // Support members for getting theme colors and theme change notifications. ThemeServiceGtk* theme_service_; diff --git a/chrome/browser/ui/views/chrome_to_mobile_bubble_view.cc b/chrome/browser/ui/views/chrome_to_mobile_bubble_view.cc index 5bd583b..0eee02e 100644 --- a/chrome/browser/ui/views/chrome_to_mobile_bubble_view.cc +++ b/chrome/browser/ui/views/chrome_to_mobile_bubble_view.cc @@ -142,6 +142,9 @@ void ChromeToMobileBubbleView::WindowClosing() { DCHECK(bubble_ == this); bubble_ = NULL; + // Instruct the service to delete the snapshot file. + service_->DeleteSnapshot(snapshot_path_); + // Restore the resting state mobile device icon. SetImageViewToId(anchor_view(), IDR_MOBILE); } @@ -183,8 +186,8 @@ void ChromeToMobileBubbleView::ButtonPressed(views::Button* sender, void ChromeToMobileBubbleView::SnapshotGenerated(const FilePath& path, int64 bytes) { + snapshot_path_ = path; if (bytes > 0) { - snapshot_path_ = path; send_copy_->SetText(l10n_util::GetStringFUTF16( IDS_CHROME_TO_MOBILE_BUBBLE_SEND_COPY, ui::FormatBytes(bytes))); send_copy_->SetEnabled(true); diff --git a/chrome/browser/ui/views/chrome_to_mobile_bubble_view.h b/chrome/browser/ui/views/chrome_to_mobile_bubble_view.h index ae4602a..02c6ab9 100644 --- a/chrome/browser/ui/views/chrome_to_mobile_bubble_view.h +++ b/chrome/browser/ui/views/chrome_to_mobile_bubble_view.h @@ -77,7 +77,7 @@ class ChromeToMobileBubbleView : public views::BubbleDelegateView, base::WeakPtrFactory<ChromeToMobileBubbleView> weak_ptr_factory_; // The Chrome To Mobile service associated with this bubble. - scoped_refptr<ChromeToMobileService> service_; + ChromeToMobileService* service_; // A map of radio buttons for each mobile device to the device's information. typedef std::map<views::RadioButton*, base::DictionaryValue*> DeviceMap; diff --git a/chrome/browser/ui/views/location_bar/location_bar_view.cc b/chrome/browser/ui/views/location_bar/location_bar_view.cc index 5d2111d..8f27c6a 100644 --- a/chrome/browser/ui/views/location_bar/location_bar_view.cc +++ b/chrome/browser/ui/views/location_bar/location_bar_view.cc @@ -256,7 +256,7 @@ void LocationBarView::Init() { ChromeToMobileService::IsChromeToMobileEnabled()) { chrome_to_mobile_view_ = new ChromeToMobileView(this, command_updater_); AddChildView(chrome_to_mobile_view_); - scoped_refptr<ChromeToMobileService> service = + ChromeToMobileService* service = ChromeToMobileServiceFactory::GetForProfile(profile_); service->RequestMobileListUpdate(); chrome_to_mobile_view_->SetVisible(service->HasDevices()); |