A lice-sized code review (wangxianzhu localrev 1282)

[phni...@gmail.com]

Hello james.su,

I'd like you to do a code review. Please review the following patch:

----------------------------------------------------------------------
r1281: (no author) | 2009-12-02 14:15:16 +0800

Add ongotourl signal for gtkmoz browser element to allow gadgets to do more things about the browser.
Also fix a bug about alwaysOpenNewWindow.
----------------------------------------------------------------------
r1282: (no author) | 2009-12-02 14:16:54 +0800


----------------------------------------------------------------------

=== extensions/gtkmoz_browser_element/browser_element.cc
==================================================================
--- extensions/gtkmoz_browser_element/browser_element.cc (revision 1280)
+++ extensions/gtkmoz_browser_element/browser_element.cc (revision 1282)
@@ -1089,6 +1089,13 @@
} else {
result = OpenURL(params[2]) ? '1' : '0';
}
+ } else if (strcmp(type, kGoToURLFeedback) == 0) {
+ if (param_count != 3) {
+ LOG("%s feedback needs 3 parameters, but only %zu is given",
+ kGoToURLFeedback, param_count);
+ } else {
+ result = ongotourl_signal_(params[2]) ? '1' : '0';
+ }
} else if (strcmp(type, kNetErrorFeedback) == 0) {
if (param_count != 3) {
LOG("%s feedback needs 3 parameters, but only %zu is given",
@@ -1135,6 +1142,7 @@
Connection *minimized_connection_, *restored_connection_,
*popout_connection_, *popin_connection_,
*dock_connection_, *undock_connection_;
+ Signal1<bool, const char *> ongotourl_signal_;
Signal1<bool, const char *> onerror_signal_;
};

@@ -1192,6 +1200,8 @@
NewSlot(&BrowserElement::SetAlwaysOpenNewWindow));
RegisterClassSignal("onerror", &BrowserElementImpl::onerror_signal_,
&BrowserElement::impl_);
+ RegisterClassSignal("ongotourl", &BrowserElementImpl::ongotourl_signal_,
+ &BrowserElement::impl_);
}

BrowserElement::~BrowserElement() {
=== extensions/gtkmoz_browser_element/browser_child.cc
==================================================================
--- extensions/gtkmoz_browser_element/browser_child.cc (revision 1280)
+++ extensions/gtkmoz_browser_element/browser_child.cc (revision 1282)
@@ -627,8 +627,18 @@
return NS_OK;
}

- if (!browser_info->always_open_new_window &&
- (content_type == TYPE_DOCUMENT || content_type == TYPE_SUBDOCUMENT)) {
+ if (content_type == TYPE_DOCUMENT || content_type == TYPE_SUBDOCUMENT) {
+ if (!browser_info->always_open_new_window) {
+ std::string r = SendFeedback(browser_info->browser_id,
+ kGoToURLFeedback, url_spec.get(),
+ NULL);
+ // The controller should have opened the URL, so don't let the
+ // embedded browser open it.
+ if (r[0] != '0')
+ *retval = REJECT_OTHER;
+ return NS_OK;
+ }
+
if (request_origin)
request_origin->GetSpec(origin_spec);

=== extensions/gtkmoz_browser_element/browser_child.h
==================================================================
--- extensions/gtkmoz_browser_element/browser_child.h (revision 1280)
+++ extensions/gtkmoz_browser_element/browser_child.h (revision 1282)
@@ -245,7 +245,7 @@

/**
* The browser child tells the controller that the browser is about to open
- * an URL.
+ * a URL in a new window.
*
* Message format:
* <code>
@@ -262,6 +262,24 @@
const char kOpenURLFeedback[] = "OPEN";

/**
+ * The browser child tells the controller that the browser is about to go to
+ * a URL in the current window or some sub-frame.
+ *
+ * Message format:
+ * <code>
+ * GOTO\n
+ * Browser ID (size_t)\n
+ * URL (normal string)\n
+ * """EOM"""\n
+ * </code>
+ *
+ * The controller must immediately reply a message containing kReplyPrefix and
+ * the return value "1" or "0" indicating if the URL has been/will be opened or
+ * not opened by the controller respectively.
+ */
+const char kGoToURLFeedback[] = "GOTO";
+
+/**
* The browser child tells the controller that the browser has encountered a
* network error.
*

This is a semiautomated message from "svkmail". Complaints or suggestions?
Mail edy...@gmail.com.

[James Su]

LGTM.

2009/12/2 <phni...@gmail.com>