Bug 36489 - [DRT/Chromium] Add TestNavigationController and TestWebWorker
Summary: [DRT/Chromium] Add TestNavigationController and TestWebWorker
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 35902
  Show dependency treegraph
 
Reported: 2010-03-23 05:50 PDT by Kent Tamura
Modified: 2010-03-23 22:56 PDT (History)
2 users (show)

See Also:


Attachments
Patch (22.19 KB, patch)
2010-03-23 05:56 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Proposed patch (rev.2) (22.17 KB, patch)
2010-03-23 20:47 PDT, Kent Tamura
dglazkov: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 2010-03-23 05:50:40 PDT
[DRT/Chromium] Add TestNavigationController and TestWebWorker
Comment 1 Kent Tamura 2010-03-23 05:56:11 PDT
Created attachment 51415 [details]
Patch
Comment 2 Dimitri Glazkov (Google) 2010-03-23 09:46:02 PDT
Comment on attachment 51415 [details]
Patch

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.
Comment 3 Kent Tamura 2010-03-23 20:47:05 PDT
Created attachment 51476 [details]
Proposed patch (rev.2)
Comment 4 Kent Tamura 2010-03-23 20:50:39 PDT
(In reply to comment #2)
> (From update of attachment 51415 [details])
> 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.

We need it to produce the output of layoutTestControler.dumpBackForwardList.

> > +TestNavigationEntry::TestNavigationEntry(
> > +    int pageID, const GURL& url, const WebString& title, const WebString& targetFrame)
> 
> Can we use WebURL here -- or at least put a FIXME?

Replaced GURL with WebURL.

> > +#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.

Removed ref_counted.h, which was not used in this file.
I think no WTF alternative for linked_ptr.h.

> > +#define TestWebWorker_h
> > +
> > +#include "base/basictypes.h"
> 
> No need for this here.

Right. Removed.
Comment 5 Dimitri Glazkov (Google) 2010-03-23 21:04:41 PDT
Comment on attachment 51476 [details]
Proposed patch (rev.2)

ok. I don't think we'll need some of the methods TestNavigationController provides, but we can refactor this later.
Comment 6 Kent Tamura 2010-03-23 22:56:08 PDT
Landed as r56434.