[webkit-reviews] review denied: [Bug 36489] [DRT/Chromium] Add TestNavigationController and TestWebWorker : [Attachment 51415] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Mar 23 09:46:02 PDT 2010


Dimitri Glazkov (Google) <dglazkov at chromium.org> has denied TAMURA, Kent
<tkent at chromium.org>'s request for review:
Bug 36489: [DRT/Chromium] Add TestNavigationController and TestWebWorker
https://bugs.webkit.org/show_bug.cgi?id=36489

Attachment 51415: Patch
https://bugs.webkit.org/attachment.cgi?id=51415&action=review

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
Why do we need TestNavigationController in its full glory in DRT? Most of its
work is focused around providing a simple browser, which we don't need here.

> +
> +//
----------------------------------------------------------------------------
> +// TestNavigationEntry
> +
> +TestNavigationEntry::TestNavigationEntry()
> +    : m_pageID(-1) {}
> +
> +TestNavigationEntry::TestNavigationEntry(
> +    int pageID, const GURL& url, const WebString& title, const WebString&
targetFrame)

Can we use WebURL here -- or at least put a FIXME?

> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef TestNavigationController_h
> +#define TestNavigationController_h
> +
> +#include "base/basictypes.h"
> +#include "base/linked_ptr.h"
> +#include "base/ref_counted.h"

It would be good to use WTF types here, but a FIXME should suffice for now.

> +#define TestWebWorker_h
> +
> +#include "base/basictypes.h"

No need for this here.


More information about the webkit-reviews mailing list