Clean up automation provider side:

- remove unneeded code
- get rid of poorly written code like private inheritation
- simplify the thread-related code
- remove a few fatal assertions from test code
- minor style cleanups

BUG=none
TEST=none

Review URL: http://codereview.chromium.org/5510002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@68568 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 145 insertions, 215 deletions

diff --git a/chrome/browser/automation/automation_browser_tracker.h b/chrome/browser/automation/automation_browser_tracker.h
index 147b820..88d95b5 100644
--- a/chrome/browser/automation/automation_browser_tracker.h
+++ b/chrome/browser/automation/automation_browser_tracker.h
@@ -7,18 +7,16 @@
 #pragma once
 
 #include "chrome/browser/automation/automation_resource_tracker.h"
-#include "chrome/browser/ui/browser.h"
 #include "chrome/common/notification_source.h"
 
+class Browser;
+
 // Tracks Browser objects.
 class AutomationBrowserTracker : public AutomationResourceTracker<Browser*> {
  public:
   explicit AutomationBrowserTracker(IPC::Message::Sender* automation)
       : AutomationResourceTracker<Browser*>(automation) { }
 
-  virtual ~AutomationBrowserTracker() {
-  }
-
   virtual void AddObserver(Browser* resource) {
     registrar_.Add(this, NotificationType::BROWSER_CLOSED,
                    Source<Browser>(resource));
diff --git a/chrome/browser/automation/automation_provider_gtk.cc b/chrome/browser/automation/automation_provider_gtk.cc
index bf6a15c..ca021cd 100644
--- a/chrome/browser/automation/automation_provider_gtk.cc
+++ b/chrome/browser/automation/automation_provider_gtk.cc
@@ -13,6 +13,7 @@
 #include "chrome/browser/gtk/browser_window_gtk.h"
 #include "chrome/browser/gtk/gtk_util.h"
 #include "chrome/browser/gtk/view_id_util.h"
+#include "chrome/browser/ui/browser.h"
 #include "chrome/common/automation_messages.h"
 #include "gfx/point.h"
 #include "gfx/rect.h"
diff --git a/chrome/browser/automation/automation_resource_tracker.h b/chrome/browser/automation/automation_resource_tracker.h
index 2d6b0cd..871af66 100644
--- a/chrome/browser/automation/automation_resource_tracker.h
+++ b/chrome/browser/automation/automation_resource_tracker.h
@@ -11,11 +11,10 @@
 #include "base/basictypes.h"
 #include "chrome/common/notification_observer.h"
 #include "chrome/common/notification_registrar.h"
+#include "chrome/common/notification_source.h"
 #include "chrome/common/notification_type.h"
 #include "ipc/ipc_message.h"
 
-template <class T> class Source;
-
 // Template trick so that AutomationResourceTracker can be used with non-pointer
 // types.
 template <class T>
@@ -35,6 +34,7 @@ class AutomationResourceTrackerImpl {
   explicit AutomationResourceTrackerImpl(IPC::Message::Sender* sender);
   virtual ~AutomationResourceTrackerImpl();
 
+ protected:
   // These need to be implemented in AutomationResourceTracker,
   // since it needs to call the subclass's type-specific notification
   // registration functions.
@@ -50,16 +50,16 @@ class AutomationResourceTrackerImpl {
   int GetHandleImpl(const void* resource);
   void HandleCloseNotification(const void* resource);
 
- protected:
+ private:
   typedef std::map<const void*, int> ResourceToHandleMap;
   typedef std::map<int, const void*> HandleToResourceMap;
+
   ResourceToHandleMap resource_to_handle_;
   HandleToResourceMap handle_to_resource_;
 
- private:
-  DISALLOW_COPY_AND_ASSIGN(AutomationResourceTrackerImpl);
-
   IPC::Message::Sender* sender_;
+
+  DISALLOW_COPY_AND_ASSIGN(AutomationResourceTrackerImpl);
 };
 
 // This template defines a superclass for an object that wants to track
@@ -68,15 +68,12 @@ class AutomationResourceTrackerImpl {
 // define are AddObserver and RemoveObserver for the given resource's
 // close notifications.
 template <class T>
-class AutomationResourceTracker : public NotificationObserver,
-                                  private AutomationResourceTrackerImpl {
+class AutomationResourceTracker : public AutomationResourceTrackerImpl,
+                                  public NotificationObserver {
  public:
   explicit AutomationResourceTracker(IPC::Message::Sender* automation)
     : AutomationResourceTrackerImpl(automation) {}
 
-  virtual ~AutomationResourceTracker() {
-  }
-
   // The implementations for these should call the NotificationService
   // to add and remove this object as an observer for the appropriate
   // resource closing notification.
@@ -141,9 +138,6 @@ class AutomationResourceTracker : public NotificationObserver,
     HandleCloseNotification(resource);
   }
 
-  NotificationRegistrar registrar_;
-
- private:
   // These proxy calls from the base Impl class to the template's subclss.
   // The casts here allow this to compile with both T = Foo and T = const Foo.
   virtual void AddObserverTypeProxy(const void* resource) {
@@ -153,6 +147,9 @@ class AutomationResourceTracker : public NotificationObserver,
     RemoveObserver(static_cast<T>(const_cast<void*>(resource)));
   }
 
+  NotificationRegistrar registrar_;
+
+ private:
   DISALLOW_COPY_AND_ASSIGN(AutomationResourceTracker);
 };
 
diff --git a/chrome/browser/automation/testing_automation_provider.cc b/chrome/browser/automation/testing_automation_provider.cc
index 9098cf6..5aee9ee 100644
--- a/chrome/browser/automation/testing_automation_provider.cc
+++ b/chrome/browser/automation/testing_automation_provider.cc
@@ -78,136 +78,51 @@
 
 namespace {
 
-class GetCookiesTask : public Task {
- public:
-  GetCookiesTask(const GURL& url,
-                 URLRequestContextGetter* context_getter,
-                 base::WaitableEvent* event,
-                 std::string* cookies)
-      : url_(url),
-        context_getter_(context_getter),
-        event_(event),
-        cookies_(cookies) {}
-
-  virtual void Run() {
-    *cookies_ = context_getter_->GetCookieStore()->GetCookies(url_);
-    event_->Signal();
-  }
-
- private:
-  const GURL& url_;
-  URLRequestContextGetter* const context_getter_;
-  base::WaitableEvent* const event_;
-  std::string* const cookies_;
-
-  DISALLOW_COPY_AND_ASSIGN(GetCookiesTask);
-};
-
-std::string GetCookiesForURL(
+void GetCookiesOnIOThread(
     const GURL& url,
-    URLRequestContextGetter* context_getter) {
-  std::string cookies;
-  base::WaitableEvent event(true /* manual reset */,
-                            false /* not initially signaled */);
-  CHECK(BrowserThread::PostTask(
-      BrowserThread::IO, FROM_HERE,
-      new GetCookiesTask(url, context_getter, &event, &cookies)));
-  event.Wait();
-  return cookies;
+    const scoped_refptr<URLRequestContextGetter>& context_getter,
+    base::WaitableEvent* event,
+    std::string* cookies) {
+  *cookies = context_getter->GetCookieStore()->GetCookies(url);
+  event->Signal();
 }
 
-class SetCookieTask : public Task {
- public:
-  SetCookieTask(const GURL& url,
-                const std::string& value,
-                URLRequestContextGetter* context_getter,
-                base::WaitableEvent* event,
-                bool* rv)
-      : url_(url),
-        value_(value),
-        context_getter_(context_getter),
-        event_(event),
-        rv_(rv) {}
-
-  virtual void Run() {
-    *rv_ = context_getter_->GetCookieStore()->SetCookie(url_, value_);
-    event_->Signal();
-  }
-
- private:
-  const GURL& url_;
-  const std::string& value_;
-  URLRequestContextGetter* const context_getter_;
-  base::WaitableEvent* const event_;
-  bool* const rv_;
-
-  DISALLOW_COPY_AND_ASSIGN(SetCookieTask);
-};
-
-bool SetCookieForURL(
+void SetCookieOnIOThread(
     const GURL& url,
     const std::string& value,
-    URLRequestContextGetter* context_getter) {
-  base::WaitableEvent event(true /* manual reset */,
-                            false /* not initially signaled */);
-  bool rv = false;
-  CHECK(BrowserThread::PostTask(
-      BrowserThread::IO, FROM_HERE,
-      new SetCookieTask(url, value, context_getter, &event, &rv)));
-  event.Wait();
-  return rv;
+    const scoped_refptr<URLRequestContextGetter>& context_getter,
+    base::WaitableEvent* event,
+    bool* success) {
+  *success = context_getter->GetCookieStore()->SetCookie(url, value);
+  event->Signal();
 }
 
-class DeleteCookieTask : public Task {
- public:
-  DeleteCookieTask(const GURL& url,
-                   const std::string& name,
-                   const scoped_refptr<URLRequestContextGetter>& context_getter)
-      : url_(url),
-        name_(name),
-        context_getter_(context_getter) {}
-
-  virtual void Run() {
-    net::CookieStore* cookie_store = context_getter_->GetCookieStore();
-    cookie_store->DeleteCookie(url_, name_);
-  }
-
- private:
-  const GURL url_;
-  const std::string name_;
-  const scoped_refptr<URLRequestContextGetter> context_getter_;
-
-  DISALLOW_COPY_AND_ASSIGN(DeleteCookieTask);
-};
-
-class ClickTask : public Task {
- public:
-  explicit ClickTask(int flags) : flags_(flags) {}
-  virtual ~ClickTask() {}
-
-  virtual void Run() {
-    ui_controls::MouseButton button = ui_controls::LEFT;
-    if ((flags_ & views::Event::EF_LEFT_BUTTON_DOWN) ==
-        views::Event::EF_LEFT_BUTTON_DOWN) {
-      button = ui_controls::LEFT;
-    } else if ((flags_ & views::Event::EF_RIGHT_BUTTON_DOWN) ==
-        views::Event::EF_RIGHT_BUTTON_DOWN) {
-      button = ui_controls::RIGHT;
-    } else if ((flags_ & views::Event::EF_MIDDLE_BUTTON_DOWN) ==
-        views::Event::EF_MIDDLE_BUTTON_DOWN) {
-      button = ui_controls::MIDDLE;
-    } else {
-      NOTREACHED();
-    }
-
-    ui_controls::SendMouseClick(button);
+void DeleteCookieOnIOThread(
+    const GURL& url,
+    const std::string& name,
+    const scoped_refptr<URLRequestContextGetter>& context_getter,
+    base::WaitableEvent* event) {
+  context_getter->GetCookieStore()->DeleteCookie(url, name);
+  event->Signal();
+}
+
+void SendMouseClick(int flags) {
+  ui_controls::MouseButton button = ui_controls::LEFT;
+  if ((flags & views::Event::EF_LEFT_BUTTON_DOWN) ==
+      views::Event::EF_LEFT_BUTTON_DOWN) {
+    button = ui_controls::LEFT;
+  } else if ((flags & views::Event::EF_RIGHT_BUTTON_DOWN) ==
+             views::Event::EF_RIGHT_BUTTON_DOWN) {
+    button = ui_controls::RIGHT;
+  } else if ((flags & views::Event::EF_MIDDLE_BUTTON_DOWN) ==
+             views::Event::EF_MIDDLE_BUTTON_DOWN) {
+    button = ui_controls::MIDDLE;
+  } else {
+    NOTREACHED();
   }
 
- private:
-  int flags_;
-
-  DISALLOW_COPY_AND_ASSIGN(ClickTask);
-};
+  ui_controls::SendMouseClick(button);
+}
 
 class AutomationInterstitialPage : public InterstitialPage {
  public:
@@ -221,7 +136,7 @@ class AutomationInterstitialPage : public InterstitialPage {
   virtual std::string GetHTMLContents() { return contents_; }
 
  private:
-  std::string contents_;
+  const std::string contents_;
 
   DISALLOW_COPY_AND_ASSIGN(AutomationInterstitialPage);
 };
@@ -490,23 +405,20 @@ void TestingAutomationProvider::OnChannelError() {
 
 void TestingAutomationProvider::CloseBrowser(int browser_handle,
                                              IPC::Message* reply_message) {
-  if (browser_tracker_->ContainsHandle(browser_handle)) {
-    Browser* browser = browser_tracker_->GetResource(browser_handle);
-    new BrowserClosedNotificationObserver(browser, this,
-                                          reply_message);
-    browser->window()->Close();
-  } else {
-    NOTREACHED();
-  }
+  if (!browser_tracker_->ContainsHandle(browser_handle))
+    return;
+
+  Browser* browser = browser_tracker_->GetResource(browser_handle);
+  new BrowserClosedNotificationObserver(browser, this, reply_message);
+  browser->window()->Close();
 }
 
 void TestingAutomationProvider::CloseBrowserAsync(int browser_handle) {
-  if (browser_tracker_->ContainsHandle(browser_handle)) {
-    Browser* browser = browser_tracker_->GetResource(browser_handle);
-    browser->window()->Close();
-  } else {
-    NOTREACHED();
-  }
+  if (!browser_tracker_->ContainsHandle(browser_handle))
+    return;
+
+  Browser* browser = browser_tracker_->GetResource(browser_handle);
+  browser->window()->Close();
 }
 
 void TestingAutomationProvider::ActivateTab(int handle,
@@ -522,7 +434,8 @@ void TestingAutomationProvider::ActivateTab(int handle,
   }
 }
 
-void TestingAutomationProvider::AppendTab(int handle, const GURL& url,
+void TestingAutomationProvider::AppendTab(int handle,
+                                          const GURL& url,
                                           IPC::Message* reply_message) {
   int append_tab_response = -1;  // -1 is the error code
   NotificationObserver* observer = NULL;
@@ -582,10 +495,18 @@ void TestingAutomationProvider::GetCookies(const GURL& url, int handle,
                                            std::string* value) {
   *value_size = -1;
   if (url.is_valid() && tab_tracker_->ContainsHandle(handle)) {
-    NavigationController* tab = tab_tracker_->GetResource(handle);
-
     // Since we are running on the UI thread don't call GetURLRequestContext().
-    *value = GetCookiesForURL(url, tab->profile()->GetRequestContext());
+    scoped_refptr<URLRequestContextGetter> context_getter =
+        tab_tracker_->GetResource(handle)->profile()->GetRequestContext();
+
+    base::WaitableEvent event(true /* manual reset */,
+                              false /* not initially signaled */);
+    CHECK(BrowserThread::PostTask(
+              BrowserThread::IO, FROM_HERE,
+              NewRunnableFunction(&GetCookiesOnIOThread,
+                                  url, context_getter, &event, value)));
+    event.Wait();
+
     *value_size = static_cast<int>(value->size());
   }
 }
@@ -597,9 +518,20 @@ void TestingAutomationProvider::SetCookie(const GURL& url,
   *response_value = -1;
 
   if (url.is_valid() && tab_tracker_->ContainsHandle(handle)) {
-    NavigationController* tab = tab_tracker_->GetResource(handle);
-
-    if (SetCookieForURL(url, value, tab->profile()->GetRequestContext()))
+    // Since we are running on the UI thread don't call GetURLRequestContext().
+    scoped_refptr<URLRequestContextGetter> context_getter =
+        tab_tracker_->GetResource(handle)->profile()->GetRequestContext();
+
+    base::WaitableEvent event(true /* manual reset */,
+                              false /* not initially signaled */);
+    bool success = false;
+    CHECK(BrowserThread::PostTask(
+              BrowserThread::IO, FROM_HERE,
+              NewRunnableFunction(&SetCookieOnIOThread,
+                                  url, value, context_getter, &event,
+                                  &success)));
+    event.Wait();
+    if (success)
       *response_value = 1;
   }
 }
@@ -609,12 +541,17 @@ void TestingAutomationProvider::DeleteCookie(const GURL& url,
                                              int handle, bool* success) {
   *success = false;
   if (url.is_valid() && tab_tracker_->ContainsHandle(handle)) {
-    NavigationController* tab = tab_tracker_->GetResource(handle);
-    BrowserThread::PostTask(
-        BrowserThread::IO, FROM_HERE,
-        new DeleteCookieTask(
-            url, cookie_name,
-            make_scoped_refptr(tab->profile()->GetRequestContext())));
+    // Since we are running on the UI thread don't call GetURLRequestContext().
+    scoped_refptr<URLRequestContextGetter> context_getter =
+        tab_tracker_->GetResource(handle)->profile()->GetRequestContext();
+
+    base::WaitableEvent event(true /* manual reset */,
+                              false /* not initially signaled */);
+    CHECK(BrowserThread::PostTask(
+              BrowserThread::IO, FROM_HERE,
+              NewRunnableFunction(&DeleteCookieOnIOThread,
+                                  url, cookie_name, context_getter, &event)));
+    event.Wait();
     *success = true;
   }
 }
@@ -650,7 +587,7 @@ void TestingAutomationProvider::NavigateToURLBlockUntilNavigationsComplete(
       AddNavigationStatusListener(tab, reply_message, number_of_navigations,
                                   false);
 
-      // TODO(darin): avoid conversion to GURL
+      // TODO(darin): avoid conversion to GURL.
       browser->OpenURL(url, GURL(), CURRENT_TAB, PageTransition::TYPED);
       return;
     }
@@ -802,8 +739,9 @@ void TestingAutomationProvider::NeedsAuth(int tab_handle, bool* needs_auth) {
 void TestingAutomationProvider::GetRedirectsFrom(int tab_handle,
                                                  const GURL& source_url,
                                                  IPC::Message* reply_message) {
-  DCHECK(!redirect_query_) << "Can only handle one redirect query at once.";
-  if (tab_tracker_->ContainsHandle(tab_handle)) {
+  if (redirect_query_) {
+    LOG(ERROR) << "Can only handle one redirect query at once.";
+  } else if (tab_tracker_->ContainsHandle(tab_handle)) {
     NavigationController* tab = tab_tracker_->GetResource(tab_handle);
     HistoryService* history_service =
         tab->profile()->GetHistoryService(Profile::EXPLICIT_ACCESS);
@@ -927,7 +865,6 @@ void TestingAutomationProvider::ExecuteBrowserCommand(
 }
 
 void TestingAutomationProvider::GetBrowserLocale(string16* locale) {
-  DCHECK(g_browser_process);
   *locale = ASCIIToUTF16(g_browser_process->GetApplicationLocale());
 }
 
@@ -949,8 +886,9 @@ void TestingAutomationProvider::WindowSimulateClick(const IPC::Message& message,
                                                     const gfx::Point& click,
                                                     int flags) {
   if (window_tracker_->ContainsHandle(handle)) {
-    ui_controls::SendMouseMoveNotifyWhenDone(click.x(), click.y(),
-                                             new ClickTask(flags));
+    // TODO(phajdan.jr): This is flaky. We should wait for the final click.
+    ui_controls::SendMouseMoveNotifyWhenDone(
+        click.x(), click.y(), NewRunnableFunction(&SendMouseClick, flags));
   }
 }
 
@@ -973,14 +911,14 @@ void TestingAutomationProvider::WindowSimulateKeyPress(
   gfx::NativeWindow window = window_tracker_->GetResource(handle);
   // The key event is sent to whatever window is active.
   ui_controls::SendKeyPress(window, static_cast<app::KeyboardCode>(key),
-                           ((flags & views::Event::EF_CONTROL_DOWN) ==
-                              views::Event::EF_CONTROL_DOWN),
+                            ((flags & views::Event::EF_CONTROL_DOWN) ==
+                             views::Event::EF_CONTROL_DOWN),
                             ((flags & views::Event::EF_SHIFT_DOWN) ==
-                              views::Event::EF_SHIFT_DOWN),
+                             views::Event::EF_SHIFT_DOWN),
                             ((flags & views::Event::EF_ALT_DOWN) ==
-                              views::Event::EF_ALT_DOWN),
+                             views::Event::EF_ALT_DOWN),
                             ((flags & views::Event::EF_COMMAND_DOWN) ==
-                              views::Event::EF_COMMAND_DOWN));
+                             views::Event::EF_COMMAND_DOWN));
 }
 
 void TestingAutomationProvider::GetTabCount(int handle, int* tab_count) {
@@ -1218,43 +1156,39 @@ void TestingAutomationProvider::ExecuteJavascript(
     const std::wstring& frame_xpath,
     const std::wstring& script,
     IPC::Message* reply_message) {
-  bool succeeded = false;
   TabContents* tab_contents = GetTabContentsForHandle(handle, NULL);
-  if (tab_contents) {
-    // Set the routing id of this message with the controller.
-    // This routing id needs to be remembered for the reverse
-    // communication while sending back the response of
-    // this javascript execution.
-    std::wstring set_automation_id;
-    base::SStringPrintf(&set_automation_id,
-        L"window.domAutomationController.setAutomationId(%d);",
-        reply_message->routing_id());
-
-    DCHECK(reply_message_ == NULL);
-    reply_message_ = reply_message;
-
-    tab_contents->render_view_host()->ExecuteJavascriptInWebFrame(
-        frame_xpath, set_automation_id);
-    tab_contents->render_view_host()->ExecuteJavascriptInWebFrame(
-        frame_xpath, script);
-    succeeded = true;
-  }
-
-  if (!succeeded) {
+  if (!tab_contents) {
     AutomationMsg_DomOperation::WriteReplyParams(reply_message, std::string());
     Send(reply_message);
+    return;
   }
+
+  // Set the routing id of this message with the controller.
+  // This routing id needs to be remembered for the reverse
+  // communication while sending back the response of
+  // this javascript execution.
+  std::wstring set_automation_id;
+  base::SStringPrintf(&set_automation_id,
+                      L"window.domAutomationController.setAutomationId(%d);",
+                      reply_message->routing_id());
+
+  DCHECK(reply_message_ == NULL);
+  reply_message_ = reply_message;
+
+  tab_contents->render_view_host()->ExecuteJavascriptInWebFrame(
+      frame_xpath, set_automation_id);
+  tab_contents->render_view_host()->ExecuteJavascriptInWebFrame(
+      frame_xpath, script);
 }
 
 void TestingAutomationProvider::GetConstrainedWindowCount(int handle,
                                                           int* count) {
   *count = -1;  // -1 is the error code
   if (tab_tracker_->ContainsHandle(handle)) {
-      NavigationController* nav_controller = tab_tracker_->GetResource(handle);
-      TabContents* tab_contents = nav_controller->tab_contents();
-      if (tab_contents) {
-        *count = static_cast<int>(tab_contents->child_windows_.size());
-      }
+    NavigationController* nav_controller = tab_tracker_->GetResource(handle);
+    TabContents* tab_contents = nav_controller->tab_contents();
+    if (tab_contents)
+      *count = static_cast<int>(tab_contents->child_windows_.size());
   }
 }
 
@@ -1459,24 +1393,22 @@ void TestingAutomationProvider::ActionOnSSLBlockingPage(
 
 void TestingAutomationProvider::BringBrowserToFront(int browser_handle,
                                                     bool* success) {
+  *success = false;
   if (browser_tracker_->ContainsHandle(browser_handle)) {
     Browser* browser = browser_tracker_->GetResource(browser_handle);
     browser->window()->Activate();
     *success = true;
-  } else {
-    *success = false;
   }
 }
 
 void TestingAutomationProvider::IsMenuCommandEnabled(int browser_handle,
                                                      int message_num,
                                                      bool* menu_item_enabled) {
+  *menu_item_enabled = false;
   if (browser_tracker_->ContainsHandle(browser_handle)) {
     Browser* browser = browser_tracker_->GetResource(browser_handle);
     *menu_item_enabled =
         browser->command_updater()->IsCommandEnabled(message_num);
-  } else {
-    *menu_item_enabled = false;
   }
 }
 
@@ -1500,6 +1432,14 @@ void TestingAutomationProvider::SavePage(int tab_handle,
                                          const FilePath& dir_path,
                                          int type,
                                          bool* success) {
+  SavePackage::SavePackageType save_type =
+      static_cast<SavePackage::SavePackageType>(type);
+  if (save_type < SavePackage::SAVE_AS_ONLY_HTML ||
+      save_type > SavePackage::SAVE_AS_COMPLETE_HTML) {
+    *success = false;
+    return;
+  }
+
   if (!tab_tracker_->ContainsHandle(tab_handle)) {
     *success = false;
     return;
@@ -1507,18 +1447,12 @@ void TestingAutomationProvider::SavePage(int tab_handle,
 
   NavigationController* nav = tab_tracker_->GetResource(tab_handle);
   Browser* browser = FindAndActivateTab(nav);
-  DCHECK(browser);
   if (!browser->command_updater()->IsCommandEnabled(IDC_SAVE_PAGE)) {
     *success = false;
     return;
   }
 
-  SavePackage::SavePackageType save_type =
-      static_cast<SavePackage::SavePackageType>(type);
-  DCHECK(save_type >= SavePackage::SAVE_AS_ONLY_HTML &&
-         save_type <= SavePackage::SAVE_AS_COMPLETE_HTML);
   nav->tab_contents()->SavePage(file_name, dir_path, save_type);
-
   *success = true;
 }