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

Unified Diff: chrome/browser/ui/intents/web_intent_picker_controller.cc

Issue 9148032: [Web Intents] Refactor picker to use WebIntentPickerModel. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: groby's fix Created 8 years, 11 months 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 side-by-side diff with in-line comments
Download patch
Index: chrome/browser/ui/intents/web_intent_picker_controller.cc
diff --git a/chrome/browser/ui/intents/web_intent_picker_controller.cc b/chrome/browser/ui/intents/web_intent_picker_controller.cc
index bd0cbc83596a8aae35b0e492ea6719049c334241..0d1bf44fdd8d3652dc71819ba71550cfb7e05ff2 100644
--- a/chrome/browser/ui/intents/web_intent_picker_controller.cc
+++ b/chrome/browser/ui/intents/web_intent_picker_controller.cc
@@ -16,7 +16,7 @@
#include "chrome/browser/tabs/tab_strip_model.h"
#include "chrome/browser/ui/browser_navigator.h"
#include "chrome/browser/ui/intents/web_intent_picker.h"
-#include "chrome/browser/ui/intents/web_intent_picker_factory.h"
+#include "chrome/browser/ui/intents/web_intent_picker_model.h"
#include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
#include "chrome/browser/webdata/web_data_service.h"
#include "chrome/common/chrome_notification_types.h"
@@ -25,11 +25,9 @@
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_intents_dispatcher.h"
#include "ui/gfx/codec/png_codec.h"
+#include "ui/gfx/image/image.h"
#include "webkit/glue/web_intent_service_data.h"
-using content::NavigationController;
-using content::WebContents;
-
namespace {
// Gets the favicon service for the profile in |tab_contents|.
@@ -42,6 +40,19 @@ WebIntentsRegistry* GetWebIntentsRegistry(TabContentsWrapper* wrapper) {
return WebIntentsRegistryFactory::GetForProfile(wrapper->profile());
}
+WebIntentPickerModel::Disposition ConvertDisposition(
+ webkit_glue::WebIntentServiceData::Disposition disposition) {
+ switch (disposition) {
+ case webkit_glue::WebIntentServiceData::DISPOSITION_INLINE:
+ return WebIntentPickerModel::DISPOSITION_INLINE;
+ case webkit_glue::WebIntentServiceData::DISPOSITION_WINDOW:
+ return WebIntentPickerModel::DISPOSITION_WINDOW;
+ default:
+ NOTREACHED();
+ return WebIntentPickerModel::DISPOSITION_WINDOW;
+ }
+}
+
} // namespace
// A class that asynchronously fetches web intent data from the web intents
@@ -108,23 +119,22 @@ class WebIntentPickerController::FaviconFetcher {
};
WebIntentPickerController::WebIntentPickerController(
- TabContentsWrapper* wrapper,
- WebIntentPickerFactory* factory)
- : wrapper_(wrapper),
- picker_factory_(factory),
- web_intent_data_fetcher_(
- new WebIntentDataFetcher(this,
- GetWebIntentsRegistry(wrapper))),
- favicon_fetcher_(
- new FaviconFetcher(this, GetFaviconService(wrapper))),
- picker_(NULL),
- pending_async_count_(0),
- service_tab_(NULL) {
- NavigationController* controller = &wrapper->web_contents()->GetController();
+ TabContentsWrapper* wrapper)
+ : wrapper_(wrapper),
+ web_intent_data_fetcher_(
+ new WebIntentDataFetcher(this, GetWebIntentsRegistry(wrapper))),
+ favicon_fetcher_(new FaviconFetcher(this, GetFaviconService(wrapper))),
+ picker_(NULL),
+ picker_model_(new WebIntentPickerModel()),
+ pending_async_count_(0),
+ picker_shown_(false),
+ service_tab_(NULL) {
+ content::NavigationController* controller =
+ &wrapper->web_contents()->GetController();
registrar_.Add(this, content::NOTIFICATION_LOAD_START,
- content::Source<NavigationController>(controller));
+ content::Source<content::NavigationController>(controller));
registrar_.Add(this, content::NOTIFICATION_TAB_CLOSING,
- content::Source<NavigationController>(controller));
+ content::Source<content::NavigationController>(controller));
}
WebIntentPickerController::~WebIntentPickerController() {
@@ -141,16 +151,19 @@ void WebIntentPickerController::SetIntentsDispatcher(
void WebIntentPickerController::ShowDialog(Browser* browser,
const string16& action,
const string16& type) {
- if (picker_ != NULL)
+ // Only show a picker once.
+ if (picker_shown_)
return;
- picker_ = picker_factory_->Create(browser, wrapper_, this);
+ picker_model_->Clear();
- // TODO(binji) Remove this check when there are implementations of the picker
- // for windows and mac.
- if (picker_ == NULL)
- return;
+ // If picker is non-NULL, it was set by a test.
+ if (picker_ == NULL) {
+ picker_ = WebIntentPicker::Create(browser, wrapper_, this,
+ picker_model_.get());
+ }
+ picker_shown_ = true;
web_intent_data_fetcher_->Fetch(action, type);
}
@@ -163,34 +176,57 @@ void WebIntentPickerController::Observe(
ClosePicker();
}
-void WebIntentPickerController::OnServiceChosen(size_t index) {
- DCHECK(index < urls_.size());
-
- bool inline_disposition = service_data_[index].disposition ==
- webkit_glue::WebIntentServiceData::DISPOSITION_INLINE;
- WebContents* new_web_contents = NULL;
- if (inline_disposition)
- new_web_contents = picker_->SetInlineDisposition(urls_[index]);
-
- if (new_web_contents == NULL) {
- // TODO(gbillock): This really only handles the 'window' disposition in a
- // quite prototype way. We need to flesh out what happens to the picker
- // during the lifetime of the service url context, and that may mean we
- // need to pass more information into the injector to find the picker again
- // and close it. Also: the above conditional construction is just because
- // there isn't Mac/Win support yet. When that's there, it'll be an else.
- browser::NavigateParams params(NULL, urls_[index],
- content::PAGE_TRANSITION_AUTO_BOOKMARK);
- params.disposition = NEW_FOREGROUND_TAB;
- params.profile = wrapper_->profile();
- browser::Navigate(&params);
- new_web_contents = params.target_contents->web_contents();
- service_tab_ = new_web_contents;
-
- ClosePicker();
+void WebIntentPickerController::OnServiceChosen(size_t index,
+ Disposition disposition) {
+ switch (disposition) {
+ case WebIntentPickerModel::DISPOSITION_INLINE:
+ // Set the model to inline disposition. It will notify the picker which
+ // will respond (via OnInlineDispositionWebContentsCreated) with the
+ // WebContents to dispatch the intent to.
+ picker_model_->SetInlineDisposition(index);
+ break;
+
+ case WebIntentPickerModel::DISPOSITION_WINDOW: {
+ // TODO(gbillock): This really only handles the 'window' disposition in a
+ // quite prototype way. We need to flesh out what happens to the picker
+ // during the lifetime of the service url context, and that may mean we
+ // need to pass more information into the injector to find the picker
+ // again and close it.
+ const WebIntentPickerModel::Item& item = picker_model_->GetItemAt(index);
+
+ browser::NavigateParams params(NULL, item.url,
+ content::PAGE_TRANSITION_AUTO_BOOKMARK);
+ params.disposition = NEW_FOREGROUND_TAB;
+ params.profile = wrapper_->profile();
+ browser::Navigate(&params);
+ content::WebContents* web_contents =
+ params.target_contents->web_contents();
+ service_tab_ = web_contents;
+
+ ClosePicker();
+
+ intents_dispatcher_->DispatchIntent(web_contents);
+ break;
+ }
+
+ default:
+ NOTREACHED();
+ break;
}
+}
- intents_dispatcher_->DispatchIntent(new_web_contents);
+void WebIntentPickerController::OnInlineDispositionWebContentsCreated(
+ content::WebContents* web_contents) {
+ if (web_contents) {
+ intents_dispatcher_->DispatchIntent(web_contents);
+ } else {
+ // TODO(binji): web_contents should never be NULL; it is in this case
+ // because views doesn't have an implementation for inline disposition yet.
+ // Remove this else when it does. In the meantime, just reforward as if it
+ // were the window disposition.
+ OnServiceChosen(picker_model_->inline_disposition_index(),
+ WebIntentPickerModel::DISPOSITION_WINDOW);
+ }
}
void WebIntentPickerController::OnCancelled() {
@@ -209,6 +245,8 @@ void WebIntentPickerController::OnCancelled() {
}
void WebIntentPickerController::OnClosing() {
+ picker_shown_ = false;
+ picker_ = NULL;
}
void WebIntentPickerController::OnSendReturnMessage() {
@@ -228,34 +266,37 @@ void WebIntentPickerController::OnSendReturnMessage() {
void WebIntentPickerController::OnWebIntentDataAvailable(
const std::vector<webkit_glue::WebIntentServiceData>& services) {
- urls_.clear();
+ std::vector<GURL> urls;
for (size_t i = 0; i < services.size(); ++i) {
- urls_.push_back(services[i].service_url);
+ picker_model_->AddItem(services[i].title, services[i].service_url,
+ ConvertDisposition(services[i].disposition));
+ urls.push_back(services[i].service_url);
}
- service_data_ = services;
// Tell the picker to initialize N urls to the default favicon
- picker_->SetServiceURLs(urls_);
- favicon_fetcher_->Fetch(urls_);
- pending_async_count_--;
+ favicon_fetcher_->Fetch(urls);
+ AsyncOperationFinished();
}
-void WebIntentPickerController::OnFaviconDataAvailable(
- size_t index,
- const SkBitmap& icon_bitmap) {
- picker_->SetServiceIcon(index, icon_bitmap);
- pending_async_count_--;
+void WebIntentPickerController::OnFaviconDataAvailable(size_t index,
+ const gfx::Image& icon) {
+ picker_model_->UpdateFaviconAt(index, icon);
+ AsyncOperationFinished();
}
void WebIntentPickerController::OnFaviconDataUnavailable(size_t index) {
- picker_->SetDefaultServiceIcon(index);
- pending_async_count_--;
+ AsyncOperationFinished();
+}
+
+void WebIntentPickerController::AsyncOperationFinished() {
+ if (--pending_async_count_ == 0) {
+ picker_->OnPendingAsyncCompleted();
+ }
}
void WebIntentPickerController::ClosePicker() {
if (picker_) {
- picker_factory_->ClosePicker(picker_);
- picker_ = NULL;
+ picker_->Close();
}
}
@@ -327,7 +368,8 @@ void WebIntentPickerController::FaviconFetcher::OnFaviconDataAvailable(
if (gfx::PNGCodec::Decode(favicon_data.image_data->front(),
favicon_data.image_data->size(),
&icon_bitmap)) {
- controller_->OnFaviconDataAvailable(index, icon_bitmap);
+ gfx::Image icon_image(new SkBitmap(icon_bitmap));
+ controller_->OnFaviconDataAvailable(index, icon_image);
return;
}
}

Powered by Google App Engine
This is Rietveld 408576698